src: Update dependencies and fix issue with usage message #917

HarryMichal · 2021-11-11T16:36:51Z

We haven't updated the dependencies of Toolbox since the rewrite in Go. There was no real need since our main target is Fedora that takes care of updating the different packages. But because of that we did not catch a regressions in Cobra (see the commit messages).

The dependency update has been done first for patch versions and then even for major versions. Both steps were filled with a call to go mod tidy.

Fixes #962

softwarefactory-project-zuul · 2021-11-11T16:59:29Z

Build failed.

unit-test : RETRY_LIMIT in 5m 46s
system-test-fedora-33 : SUCCESS in 13m 32s
system-test-fedora-34 : SUCCESS in 15m 18s
system-test-fedora-rawhide : TIMED_OUT in 20m 54s

HarryMichal · 2021-11-11T21:14:59Z

recheck

softwarefactory-project-zuul · 2021-11-11T21:37:16Z

Build failed.

unit-test : RETRY_LIMIT in 5m 45s
system-test-fedora-33 : SUCCESS in 14m 15s
system-test-fedora-34 : SUCCESS in 13m 23s
system-test-fedora-rawhide : TIMED_OUT in 20m 40s

debarshiray · 2021-11-12T02:11:16Z

But because of that we did not catch a regressions in Cobra (see the commit messages).

Umm... I don't quite understand much from the commit messages.

Is there an actual regression in Cobra that we are addressing? But since we haven't updated the dependencies, how can it be a recent regression? Or are these just a periodic update of the dependencies?

debarshiray

It will be easier to understand if the commit message included a clear example of the change in behaviour of Cobra. Like in spf13/cobra#1532

test/system/002-help.bats

debarshiray · 2021-11-12T02:25:47Z

But because of that we did not catch a regressions in Cobra (see the commit messages).

Umm... I don't quite understand much from the commit messages.

Is there an actual regression in Cobra that we are addressing? But since we
haven't updated the dependencies, how can it be a recent regression? Or are
these just a periodic update of the dependencies?

Ah, I see that it's a change in behaviour in newer Cobra versions that distributors might be using for their builds!

In that case, does the newer approach of using the templates also work with older versions of Cobra? I am asking because different versions of Fedora might have different versions of Cobra.

HarryMichal · 2021-11-12T11:31:11Z

In that case, does the newer approach of using the templates also work with older versions of Cobra? I am asking because different versions of Fedora might have different versions of Cobra.

The template has been available from the very first version of Cobra, so yes.

- github.com/HarryMichal/go-version v1.0.0 => v1.0.1 - github.com/fsnotify/fsnotify v1.4.7 => v1.4.9 - github.com/godbus/dbus/v5 v5.0.3 => v5.0.6 - github.com/konsorten/go-windows-terminal-sequences v1.0.1 => v1.0.3 - github.com/magiconair/properties v1.8.0 => v1.8.5 - github.com/mattn/go-colorable v0.1.2 => v0.1.11 - github.com/mattn/go-isatty v0.0.8 => v0.0.14 - github.com/spf13/cast v1.3.0 => v1.3.1 - github.com/spf13/cobra v0.0.5 => v0.0.7 - github.com/spf13/pflag v1.0.3 => v1.0.5 - github.com/spf13/viper v1.3.2 => v1.4.0 - golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 => v0.0.0-20211115234514-b4de73f9ece8 - golang.org/x/sys v0.0.0-20190422165155-953cdadca894 => v0.0.0-20211113001501-0c823b97ae02 - golang.org/x/text v0.3.0 => v0.3.7 - gopkg.in/yaml.v2 v2.2.2 => v2.2.8 containers#917

- github.com/briandowns/spinner v1.10.0 => v1.16.0 - github.com/fatih/color v1.7.0 => v1.13.0 - github.com/fsnotify/fsnotify v1.4.9 => v1.5.1 - github.com/sirupsen/logrus v1.4.2 => v1.8.1 - github.com/spf13/cast v1.3.1 => v1.4.1 - github.com/spf13/cobra v0.0.7 => v1.2.1 - github.com/spf13/viper v1.4.0 => v1.9.0 - gopkg.in/ini.v1 v1.63.2 => v1.64.0 - gopkg.in/yaml.v2 v2.2.8 => v2.4.0 containers#917

In version 1.1.2 of Cobra has been included a change[0] that changes the behaviour of how custom usage functions are printed. The change prepends "Error: " to the text. This is not desired since the usage text is not an error text. Example of the wrong behaviour: $ toolbox --foo Error: unknown flag: --foo Error: Run 'toolbox --help' for usage. Desired behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage. A workaround is to define a template string for the usage instead. The template uses the templating language of Go[1]. See the default template string in version 1.2.1[2]. Because the template is set only once, the executableBase needs to be set before the template is applied. That required the move of setUpGlobals() into init() of the cmd package. This is a better place for the function call as init() is called earlier than Execute()[3]. Upstream issue: spf13/cobra#1532 [0] spf13/cobra#1044 [1] https://pkg.go.dev/text/template [2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491 [3] https://golang.org/doc/effective_go#init containers#917

softwarefactory-project-zuul · 2021-11-16T02:23:20Z

Build failed.

unit-test : RETRY_LIMIT in 5m 44s
system-test-fedora-33 : SUCCESS in 14m 47s
system-test-fedora-34 : SUCCESS in 13m 00s
system-test-fedora-rawhide : TIMED_OUT in 20m 47s

- github.com/HarryMichal/go-version v1.0.0 => v1.0.1 - github.com/fsnotify/fsnotify v1.4.7 => v1.4.9 - github.com/godbus/dbus/v5 v5.0.3 => v5.0.6 - github.com/konsorten/go-windows-terminal-sequences v1.0.1 => v1.0.3 - github.com/magiconair/properties v1.8.0 => v1.8.5 - github.com/mattn/go-colorable v0.1.2 => v0.1.11 - github.com/mattn/go-isatty v0.0.8 => v0.0.14 - github.com/spf13/cast v1.3.0 => v1.3.1 - github.com/spf13/cobra v0.0.5 => v0.0.7 - github.com/spf13/pflag v1.0.3 => v1.0.5 - github.com/spf13/viper v1.3.2 => v1.4.0 - golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 => v0.0.0-20211115234514-b4de73f9ece8 - golang.org/x/sys v0.0.0-20190422165155-953cdadca894 => v0.0.0-20211113001501-0c823b97ae02 - golang.org/x/text v0.3.0 => v0.3.7 - gopkg.in/yaml.v2 v2.2.2 => v2.2.8 containers#917

- github.com/briandowns/spinner v1.10.0 => v1.16.0 - github.com/fatih/color v1.7.0 => v1.13.0 - github.com/fsnotify/fsnotify v1.4.9 => v1.5.1 - github.com/sirupsen/logrus v1.4.2 => v1.8.1 - github.com/spf13/cast v1.3.1 => v1.4.1 - github.com/spf13/cobra v0.0.7 => v1.2.1 - github.com/spf13/viper v1.4.0 => v1.9.0 - gopkg.in/ini.v1 v1.63.2 => v1.64.0 - gopkg.in/yaml.v2 v2.2.8 => v2.4.0 containers#917

In version 1.1.2 of Cobra has been included a change[0] that changes the behaviour of how custom usage functions are printed. The change prepends "Error: " to the text. This is not desired since the usage text is not an error text. Example of the wrong behaviour: $ toolbox --foo Error: unknown flag: --foo Error: Run 'toolbox --help' for usage. Desired behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage. A workaround is to define a template string for the usage instead. The template uses the templating language of Go[1]. See the default template string in version 1.2.1[2]. Because the template is set only once, the executableBase needs to be set before the template is applied. That required the move of setUpGlobals() into init() of the cmd package. This is a better place for the function call as init() is called earlier than Execute()[3]. Upstream issue: spf13/cobra#1532 [0] spf13/cobra#1044 [1] https://pkg.go.dev/text/template [2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491 [3] https://golang.org/doc/effective_go#init containers#917

HarryMichal · 2021-12-04T02:01:49Z

@debarshiray?

HarryMichal · 2021-12-04T15:43:16Z

This supersedes #775

- github.com/HarryMichal/go-version v1.0.0 => v1.0.1 - github.com/fsnotify/fsnotify v1.4.7 => v1.4.9 - github.com/godbus/dbus/v5 v5.0.3 => v5.0.6 - github.com/konsorten/go-windows-terminal-sequences v1.0.1 => v1.0.3 - github.com/magiconair/properties v1.8.0 => v1.8.5 - github.com/mattn/go-colorable v0.1.2 => v0.1.11 - github.com/mattn/go-isatty v0.0.8 => v0.0.14 - github.com/spf13/cast v1.3.0 => v1.3.1 - github.com/spf13/cobra v0.0.5 => v0.0.7 - github.com/spf13/pflag v1.0.3 => v1.0.5 - github.com/spf13/viper v1.3.2 => v1.4.0 - golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 => v0.0.0-20211115234514-b4de73f9ece8 - golang.org/x/sys v0.0.0-20190422165155-953cdadca894 => v0.0.0-20211113001501-0c823b97ae02 - golang.org/x/text v0.3.0 => v0.3.7 - gopkg.in/yaml.v2 v2.2.2 => v2.2.8 containers#917

- github.com/briandowns/spinner v1.10.0 => v1.16.0 - github.com/fatih/color v1.7.0 => v1.13.0 - github.com/fsnotify/fsnotify v1.4.9 => v1.5.1 - github.com/sirupsen/logrus v1.4.2 => v1.8.1 - github.com/spf13/cast v1.3.1 => v1.4.1 - github.com/spf13/cobra v0.0.7 => v1.2.1 - github.com/spf13/viper v1.4.0 => v1.9.0 - gopkg.in/ini.v1 v1.63.2 => v1.64.0 - gopkg.in/yaml.v2 v2.2.8 => v2.4.0 containers#917

In version 1.1.2 of Cobra has been included a change[0] that changes the behaviour of how custom usage functions are printed. The change prepends "Error: " to the text. This is not desired since the usage text is not an error text. Example of the wrong behaviour: $ toolbox --foo Error: unknown flag: --foo Error: Run 'toolbox --help' for usage. Desired behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage. A workaround is to define a template string for the usage instead. The template uses the templating language of Go[1]. See the default template string in version 1.2.1[2]. Because the template is set only once, the executableBase needs to be set before the template is applied. That required the move of setUpGlobals() into init() of the cmd package. This is a better place for the function call as init() is called earlier than Execute()[3]. Upstream issue: spf13/cobra#1532 [0] spf13/cobra#1044 [1] https://pkg.go.dev/text/template [2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491 [3] https://golang.org/doc/effective_go#init containers#917

softwarefactory-project-zuul · 2021-12-15T22:51:41Z

Build failed.

unit-test : RETRY_LIMIT in 5m 47s
system-test-fedora-34 : FAILURE in 16m 57s
system-test-fedora-35 : FAILURE in 13m 12s
system-test-fedora-rawhide : TIMED_OUT in 22m 53s

In version 1.1.2 of Cobra has been included a change[0] that changes the behaviour of how custom usage functions are printed. The change prepends "Error: " to the text. This is not desired since the usage text is not an error text. Example of the wrong behaviour: $ toolbox --foo Error: unknown flag: --foo Error: Run 'toolbox --help' for usage. Desired behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage. A workaround is to define a template string for the usage instead. The template uses the templating language of Go[1]. See the default template string in version 1.2.1[2]. Because the template is set only once, the executableBase needs to be set before the template is applied. That required the move of setUpGlobals() into init() of the cmd package. This is a better place for the function call as init() is called earlier than Execute()[3]. Upstream issue: spf13/cobra#1532 [0] spf13/cobra#1044 [1] https://pkg.go.dev/text/template [2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491 [3] https://golang.org/doc/effective_go#init containers#917

softwarefactory-project-zuul · 2021-12-16T01:15:55Z

Build failed.

unit-test : RETRY_LIMIT in 6m 01s
system-test-fedora-34 : SUCCESS in 15m 39s
system-test-fedora-35 : SUCCESS in 12m 51s
system-test-fedora-rawhide : TIMED_OUT in 22m 46s

debarshiray

Thanks. One quick comment:

src/go.mod

- github.com/HarryMichal/go-version v1.0.0 => v1.0.1 - github.com/fsnotify/fsnotify v1.4.7 => v1.4.9 - github.com/godbus/dbus/v5 v5.0.3 => v5.0.6 - github.com/mattn/go-isatty v0.0.8 => v0.0.14 - github.com/spf13/cobra v0.0.5 => v0.0.7 - github.com/spf13/viper v1.3.2 => v1.4.0 - golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 => v0.0.0-20211115234514-b4de73f9ece8 - golang.org/x/sys v0.0.0-20190422165155-953cdadca894 => v0.0.0-20211113001501-0c823b97ae02 containers#917

debarshiray · 2021-12-17T14:52:55Z

Some more observations ...

As far as I understand, the wrong behaviour isn't this, as mentioned in the commit message:

$ toolbox --foo
Error: unknown flag: --foo
Error: Run 'toolbox --help' for usage.

It's actually this:

$ toolbox --foo
Error: unknown flag: --foo
Run 'toolbox --help' for usage.Error: Run 'toolbox --help' for usage.

ie., the usage string gets strangely duplicated, with an Error: prefix in the middle.

We could have avoided the duplication by not printing the text ourselves, if it wasn't for the extra prefix, etc.. These are details that we don't have to explain in detail, but the example is important. A wrong example will confuse people even more because it won't match their observation.

Secondly, we should bump the dependencies with go get -u after working around the Cobra issue. Otherwise it will break git bisect.

debarshiray · 2021-12-17T14:54:14Z

src/go.mod

-	github.com/mattn/go-isatty v0.0.8
+	github.com/fsnotify/fsnotify v1.4.9
+	github.com/godbus/dbus/v5 v5.0.6
+	github.com/konsorten/go-windows-terminal-sequences v1.0.3 // indirect


We don't need to highlight the changes in the indirect dependencies in the commit message. There are just too many of those and they keep coming and going. It's better to only highlight our direct dependencies in the commit message, unless there's one that we really need to be careful about.

Fair enough.

In version 1.1.2 of Cobra has been included a change[0] that changes how custom usage functions are handled. Example of the wrong behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage.Error: Run 'toolbox --help' for usage. Desired behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage. A workaround is to define a template string for the usage instead. The template uses the templating language of Go[1]. See the default template string in version 1.2.1[2]. Because the template is set only once, the executableBase needs to be set before the template is applied. That required the move of setUpGlobals() into init() of the cmd package. This is a better place for the function call as init() is called earlier than Execute()[3]. Upstream issue: spf13/cobra#1532 [0] spf13/cobra#1044 [1] https://pkg.go.dev/text/template [2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491 [3] https://golang.org/doc/effective_go#init containers#917

- github.com/acobaugh/osrelease v0.0.0-20181218015638-a93a0a55a249 => v0.1.0 - github.com/briandowns/spinner v1.10.0 => v1.17.0 - github.com/fsnotify/fsnotify v1.4.9 => v1.5.1 - github.com/sirupsen/logrus v1.4.2 => v1.8.1 - github.com/spf13/cobra v0.0.7 => v1.3.0 - github.com/spf13/viper v1.4.0 => v1.10.1 - golang.org/x/crypto v0.0.0-20211115234514-b4de73f9ece8 => v0.0.0-20211215153901-e495a2d5b3d3 - golang.org/x/sys v0.0.0-20211113001501-0c823b97ae02 => v0.0.0-20211216021012-1d35b9e2eb4e containers#917

softwarefactory-project-zuul · 2021-12-17T15:49:07Z

Build failed.

unit-test : RETRY_LIMIT in 5m 42s
system-test-fedora-34 : SUCCESS in 13m 16s
system-test-fedora-35 : RETRY_LIMIT in 2m 40s
system-test-fedora-rawhide : RETRY_LIMIT in 15s

debarshiray · 2021-12-17T15:53:22Z

recheck

- github.com/HarryMichal/go-version v1.0.0 => v1.0.1 - github.com/fsnotify/fsnotify v1.4.7 => v1.4.9 - github.com/godbus/dbus/v5 v5.0.3 => v5.0.6 - github.com/mattn/go-isatty v0.0.8 => v0.0.14 - github.com/spf13/cobra v0.0.5 => v0.0.7 - github.com/spf13/viper v1.3.2 => v1.4.0 - golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 => v0.0.0-20211215153901-e495a2d5b3d3 - golang.org/x/sys v0.0.0-20190422165155-953cdadca894 => v0.0.0-20211216021012-1d35b9e2eb4e containers#917

In version 1.1.2 of Cobra has been included a change[0] that changes how custom usage functions are handled. Example of the wrong behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage.Error: Run 'toolbox --help' for usage. Desired behaviour: $ toolbox --foo Error: unknown flag: --foo Run 'toolbox --help' for usage. A workaround is to define a template string for the usage instead. The template uses the templating language of Go[1]. See the default template string in version 1.2.1[2]. Because the template is set only once, the executableBase needs to be set before the template is applied. That required the move of setUpGlobals() into init() of the cmd package. This is a better place for the function call as init() is called earlier than Execute()[3]. Upstream issue: spf13/cobra#1532 [0] spf13/cobra#1044 [1] https://pkg.go.dev/text/template [2] https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491 [3] https://golang.org/doc/effective_go#init containers#917

- github.com/acobaugh/osrelease v0.0.0-20181218015638-a93a0a55a249 => v0.1.0 - github.com/briandowns/spinner v1.10.0 => v1.17.0 - github.com/fsnotify/fsnotify v1.4.9 => v1.5.1 - github.com/sirupsen/logrus v1.4.2 => v1.8.1 - github.com/spf13/cobra v0.0.7 => v1.3.0 - github.com/spf13/viper v1.4.0 => v1.10.1 - golang.org/x/crypto v0.0.0-20211115234514-b4de73f9ece8 => v0.0.0-20211215153901-e495a2d5b3d3 - golang.org/x/sys v0.0.0-20211113001501-0c823b97ae02 => v0.0.0-20211216021012-1d35b9e2eb4e containers#917

debarshiray · 2021-12-17T16:18:39Z

Wow! Software Factory is really struggling today.

softwarefactory-project-zuul · 2021-12-17T16:24:23Z

Build failed.

unit-test : RETRY_LIMIT in 5m 49s
system-test-fedora-34 : SUCCESS in 17m 38s
system-test-fedora-35 : RETRY_LIMIT in 1m 40s
system-test-fedora-rawhide : RETRY_LIMIT in 13s

HarryMichal added 6. Minor Change Should not cause breakage 3. Bugfix Fixes a bug 2. CLI Issue is related to the command line interface 3. Refinement Reduces technical debt of the codebase 2. Under The Hood Multiple areas of the app are influenced by this ticket labels Nov 11, 2021

HarryMichal added this to the Release 0.1.0 milestone Nov 11, 2021

debarshiray requested changes Nov 12, 2021

View reviewed changes

test/system/002-help.bats Outdated Show resolved Hide resolved

HarryMichal force-pushed the update-deps branch from 6af7c77 to 5ee1518 Compare November 16, 2021 01:49

HarryMichal force-pushed the update-deps branch from 5ee1518 to 8bcb56a Compare November 16, 2021 02:01

HarryMichal mentioned this pull request Nov 21, 2021

cmd: Added shell completion command #840

Merged

HarryMichal force-pushed the update-deps branch from 8bcb56a to a1691f4 Compare December 15, 2021 22:27

HarryMichal force-pushed the update-deps branch from a1691f4 to 8c00233 Compare December 16, 2021 00:51

HarryMichal mentioned this pull request Dec 17, 2021

cmd/root: Replace the usage function with a template #775

Closed

debarshiray reviewed Dec 17, 2021

View reviewed changes

src/go.mod Outdated Show resolved Hide resolved

src/go.mod Show resolved Hide resolved

debarshiray reviewed Dec 17, 2021

View reviewed changes

src/go.mod Outdated Show resolved Hide resolved

debarshiray reviewed Dec 17, 2021

View reviewed changes

HarryMichal added 2 commits December 17, 2021 16:25

debarshiray force-pushed the update-deps branch from 8c00233 to 73c53a3 Compare December 17, 2021 15:28

HarryMichal force-pushed the update-deps branch from 73c53a3 to 979e6d8 Compare December 17, 2021 16:03

debarshiray force-pushed the update-deps branch from 979e6d8 to 73c53a3 Compare December 17, 2021 16:24

debarshiray merged commit 73c53a3 into containers:main Dec 17, 2021

HarryMichal deleted the update-deps branch December 18, 2021 11:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: Update dependencies and fix issue with usage message #917

src: Update dependencies and fix issue with usage message #917

HarryMichal commented Nov 11, 2021 •

edited

softwarefactory-project-zuul bot commented Nov 11, 2021

HarryMichal commented Nov 11, 2021

softwarefactory-project-zuul bot commented Nov 11, 2021

debarshiray commented Nov 12, 2021

debarshiray left a comment

debarshiray commented Nov 12, 2021

HarryMichal commented Nov 12, 2021

softwarefactory-project-zuul bot commented Nov 16, 2021

HarryMichal commented Dec 4, 2021

HarryMichal commented Dec 4, 2021

softwarefactory-project-zuul bot commented Dec 15, 2021

softwarefactory-project-zuul bot commented Dec 16, 2021

debarshiray left a comment

debarshiray commented Dec 17, 2021

debarshiray Dec 17, 2021

HarryMichal Dec 17, 2021

softwarefactory-project-zuul bot commented Dec 17, 2021

debarshiray commented Dec 17, 2021

debarshiray commented Dec 17, 2021

softwarefactory-project-zuul bot commented Dec 17, 2021

src: Update dependencies and fix issue with usage message #917

src: Update dependencies and fix issue with usage message #917

Conversation

HarryMichal commented Nov 11, 2021 • edited

softwarefactory-project-zuul bot commented Nov 11, 2021

HarryMichal commented Nov 11, 2021

softwarefactory-project-zuul bot commented Nov 11, 2021

debarshiray commented Nov 12, 2021

debarshiray left a comment

Choose a reason for hiding this comment

debarshiray commented Nov 12, 2021

HarryMichal commented Nov 12, 2021

softwarefactory-project-zuul bot commented Nov 16, 2021

HarryMichal commented Dec 4, 2021

HarryMichal commented Dec 4, 2021

softwarefactory-project-zuul bot commented Dec 15, 2021

softwarefactory-project-zuul bot commented Dec 16, 2021

debarshiray left a comment

Choose a reason for hiding this comment

debarshiray commented Dec 17, 2021

debarshiray Dec 17, 2021

Choose a reason for hiding this comment

HarryMichal Dec 17, 2021

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Dec 17, 2021

debarshiray commented Dec 17, 2021

debarshiray commented Dec 17, 2021

softwarefactory-project-zuul bot commented Dec 17, 2021

HarryMichal commented Nov 11, 2021 •

edited