Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hardcode standard package names #24

Closed
wants to merge 1 commit into from
Closed

hardcode standard package names #24

wants to merge 1 commit into from

Conversation

kwo
Copy link
Contributor

@kwo kwo commented Nov 20, 2020

This PR should fix issue #23 - instead of shelling out to go list std at program start to gather the list of stdlib package names, the list is hardcoded. Apparently, programmatic access to the list of stdlib package names does not work consistently across different machines as expected.

Pros:

  • removes dependency on external program to retrieve list of standard package name
  • removes performance penalty of shelling out to external program at program start

Cons

  • introduces minor maintenance task to add packages to the list when new ones are added to the language
  • hardcoding data not as elegant as an algorithmic approach (yet works much better)

moul-bot pushed a commit to berty/berty that referenced this pull request Nov 20, 2020
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul
Copy link

moul commented Nov 20, 2020

I just updated berty/berty#2771 with your PR as the target for gci, let's see what happens

@moul
Copy link

moul commented Nov 20, 2020

still the same issue I fear -> https://github.com/berty/berty/pull/2771/checks?check_run_id=1431070616#step:8:86

btw, is there any way to get more verbose output, especially when program exits

@moul
Copy link

moul commented Nov 20, 2020

same on my machine:

moul@fwrz:~/go/src/berty.tech/berty/go (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ go list -m -mod=mod -f {{.Dir}} github.com/daixiang0/gci
/home/moul/go/pkg/mod/github.com/kwo/gci@v0.2.6-0.20201120125248-316c2a0df03b
moul@fwrz:~/go/src/berty.tech/berty/go (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ make go.fmt
go run github.com/daixiang0/gci -w -local berty.tech .
skip file cmd/berty/doc.go since no import
skip file cmd/berty/mini/doc.go since no import
skip file cmd/berty/mini/example_test.go since no import
[...]
skip file pkg/username/ios.go since no import
skip file pkg/username/others.go since no import
skip file pkg/username/username.go since no import
exit status 1
Makefile:203: recipe for target 'go.fmt' failed
make: *** [go.fmt] Error 1

edit: and more context (not very useful)

moul@fwrz:~/go/src/berty.tech/berty/go (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ for pkg in $(go list ./... | sed 's@berty.tech/berty/v2/go@.@'); do echo $pkg; go run github.com/daixiang0/gci -w -local berty.tech $pkg; echo; done
./cmd/berty
skip file cmd/berty/doc.go since no import
skip file cmd/berty/mini/doc.go since no import
skip file cmd/berty/mini/example_test.go since no import
exit status 1

./cmd/berty/mini
skip file cmd/berty/mini/doc.go since no import
skip file cmd/berty/mini/example_test.go since no import
exit status 1

./cmd/berty-doctor
skip file cmd/berty-doctor/example_test.go since no import
exit status 1

./cmd/berty-integration
exit status 1

./cmd/betabot
skip file cmd/betabot/example_test.go since no import
exit status 1

./cmd/rdvp
skip file cmd/rdvp/doc.go since no import
skip file cmd/rdvp/example_test.go since no import
exit status 1

./cmd/testbot
skip file cmd/testbot/example_test.go since no import
exit status 1

./framework/bertybridge
skip file framework/bertybridge/config.go since no import
skip file framework/bertybridge/config_android.go since no import
skip file framework/bertybridge/doc.go since no import
exit status 1

./internal/ble-driver
skip file internal/ble-driver/const.go since no import
skip file internal/ble-driver/example_test.go since no import
exit status 1

./internal/config
exit status 1

./internal/cryptoutil
skip file internal/cryptoutil/doc.go since no import
exit status 1

./internal/discordlog
skip file internal/discordlog/doc.go since no import
exit status 1

./internal/grpcutil
skip file internal/grpcutil/doc.go since no import
exit status 1

./internal/handshake
skip file internal/handshake/doc.go since no import
exit status 1

./internal/initutil
skip file internal/initutil/tor_embedded.go since no import
skip file internal/initutil/tor_unembedded.go since no import
exit status 1

./internal/ipfsutil
skip file internal/ipfsutil/doc.go since no import
skip file internal/ipfsutil/metrics.go since no import
exit status 1

./internal/lifecycle
skip file internal/lifecycle/example_test.go since no import
exit status 1

./internal/logutil
exit status 1

./internal/multipeer-connectivity-driver
skip file internal/multipeer-connectivity-driver/const.go since no import
skip file internal/multipeer-connectivity-driver/example_test.go since no import
exit status 1

./internal/notification
skip file internal/notification/example_test.go since no import
skip file internal/notification/manager.go since no import
skip file internal/notification/manager_noop.go since no import
exit status 1

./internal/packingutil
skip file internal/packingutil/example_test.go since no import
exit status 1

./internal/proximity-transport
skip file internal/proximity-transport/addr.go since no import
skip file internal/proximity-transport/example_test.go since no import
skip file internal/proximity-transport/nativedriver.go since no import
exit status 1

./internal/sysutil
skip file internal/sysutil/sysutil_unsupported.go since no import
exit status 1

./internal/testutil
skip file internal/testutil/doc.go since no import
skip file internal/testutil/example_test.go since no import
exit status 1

./internal/tinder
skip file internal/tinder/doc.go since no import
exit status 1

./internal/tools
skip file internal/tools/example_test.go since no import
skip file internal/tools/tools_untool.go since no import
exit status 1

./internal/tracer
exit status 1

./pkg/assets
skip file pkg/assets/embed.go since no import
skip file pkg/assets/example_test.go since no import
exit status 1

./pkg/banner
skip file pkg/banner/doc.go since no import
exit status 1

./pkg/bertyaccount
exit status 1

./pkg/bertybot
skip file pkg/bertybot/commands.go since no import
exit status 1

./pkg/bertymessenger
skip file pkg/bertymessenger/doc.go since no import
exit status 1

./pkg/bertyprotocol
skip file pkg/bertyprotocol/doc.go since no import
skip file pkg/bertyprotocol/services_auth_templates.go since no import
exit status 1

./pkg/bertytypes
skip file pkg/bertytypes/config.go since no import
skip file pkg/bertytypes/doc.go since no import
skip file pkg/bertytypes/events_account.go since no import
skip file pkg/bertytypes/example_test.go since no import
exit status 1

./pkg/bertyversion
skip file pkg/bertyversion/example_test.go since no import
skip file pkg/bertyversion/version.go since no import
exit status 1

./pkg/errcode
skip file pkg/errcode/doc.go since no import
skip file pkg/errcode/stdproto.go since no import
exit status 1

./pkg/tempdir
skip file pkg/tempdir/tempdir.go since no import
skip file pkg/tempdir/tempdir_darwin.go since no import
skip file pkg/tempdir/tempdir_others.go since no import
exit status 1

./pkg/username
skip file pkg/username/android.go since no import
skip file pkg/username/example_test.go since no import
skip file pkg/username/ios.go since no import
skip file pkg/username/others.go since no import
skip file pkg/username/username.go since no import
exit status 1

@kwo
Copy link
Contributor Author

kwo commented Nov 20, 2020

I have no issues with my version of gci on that branch:

15:20:03 karl@kaos:~/projects/temp/berty$ git status
On branch dependabot/go_modules/github.com/daixiang0/gci-0.2.5
Your branch is up to date with 'origin/dependabot/go_modules/github.com/daixiang0/gci-0.2.5'.

nothing to commit, working tree clean
15:20:04 karl@kaos:~/projects/temp/berty$ gci -d -local berty.tech .
skip file go/cmd/berty/doc.go since no import
skip file go/cmd/berty/mini/doc.go since no import
skip file go/cmd/berty/mini/example_test.go since no import
skip file go/cmd/berty-doctor/example_test.go since no import
skip file go/cmd/betabot/example_test.go since no import
skip file go/cmd/rdvp/doc.go since no import
skip file go/cmd/rdvp/example_test.go since no import
skip file go/cmd/testbot/example_test.go since no import
skip file go/framework/bertybridge/config.go since no import
skip file go/framework/bertybridge/config_android.go since no import
skip file go/framework/bertybridge/doc.go since no import
skip file go/internal/ble-driver/const.go since no import
skip file go/internal/ble-driver/example_test.go since no import
skip file go/internal/cryptoutil/doc.go since no import
skip file go/internal/discordlog/doc.go since no import
skip file go/internal/grpcutil/doc.go since no import
skip file go/internal/handshake/doc.go since no import
skip file go/internal/initutil/tor_embedded.go since no import
skip file go/internal/initutil/tor_unembedded.go since no import
skip file go/internal/ipfsutil/doc.go since no import
skip file go/internal/ipfsutil/metrics.go since no import
skip file go/internal/lifecycle/example_test.go since no import
skip file go/internal/multipeer-connectivity-driver/const.go since no import
skip file go/internal/multipeer-connectivity-driver/example_test.go since no import
skip file go/internal/notification/example_test.go since no import
skip file go/internal/notification/manager.go since no import
skip file go/internal/notification/manager_noop.go since no import
skip file go/internal/packingutil/example_test.go since no import
skip file go/internal/proximity-transport/addr.go since no import
skip file go/internal/proximity-transport/example_test.go since no import
skip file go/internal/proximity-transport/nativedriver.go since no import
skip file go/internal/sysutil/sysutil_unsupported.go since no import
skip file go/internal/testutil/doc.go since no import
skip file go/internal/testutil/example_test.go since no import
skip file go/internal/tinder/doc.go since no import
skip file go/internal/tools/example_test.go since no import
skip file go/internal/tools/tools_untool.go since no import
skip file go/pkg/assets/embed.go since no import
skip file go/pkg/assets/example_test.go since no import
skip file go/pkg/banner/doc.go since no import
skip file go/pkg/bertybot/commands.go since no import
skip file go/pkg/bertymessenger/doc.go since no import
skip file go/pkg/bertyprotocol/doc.go since no import
skip file go/pkg/bertyprotocol/services_auth_templates.go since no import
skip file go/pkg/bertytypes/config.go since no import
skip file go/pkg/bertytypes/doc.go since no import
skip file go/pkg/bertytypes/events_account.go since no import
skip file go/pkg/bertytypes/example_test.go since no import
skip file go/pkg/bertyversion/example_test.go since no import
skip file go/pkg/bertyversion/version.go since no import
skip file go/pkg/errcode/doc.go since no import
skip file go/pkg/errcode/stdproto.go since no import
skip file go/pkg/tempdir/tempdir.go since no import
skip file go/pkg/tempdir/tempdir_darwin.go since no import
skip file go/pkg/tempdir/tempdir_others.go since no import
skip file go/pkg/username/android.go since no import
skip file go/pkg/username/example_test.go since no import
skip file go/pkg/username/ios.go since no import
skip file go/pkg/username/others.go since no import
skip file go/pkg/username/username.go since no import
skip file tool/test/experiment/dht/dht.go since no import
diff -u tool/test/experiment/dht/src/matrix/dht.go.orig tool/test/experiment/dht/src/matrix/dht.go
--- tool/test/experiment/dht/src/matrix/dht.go.orig	2020-11-20 15:20:11.000000000 +0100
+++ tool/test/experiment/dht/src/matrix/dht.go	2020-11-20 15:20:11.000000000 +0100
@@ -4,10 +4,10 @@
 	"encoding/json"
 	"fmt"

+	"github.com/matrix-org/gomatrix"
 	"github.com/pkg/errors"

 	"berty.tech/berty/experiment/dht"
-	"github.com/matrix-org/gomatrix"
 )

 var _ dht.DHT = (*DHT)(nil)
diff -u tool/test/experiment/dht/test/basic_test.go.orig tool/test/experiment/dht/test/basic_test.go
--- tool/test/experiment/dht/test/basic_test.go.orig	2020-11-20 15:20:11.000000000 +0100
+++ tool/test/experiment/dht/test/basic_test.go	2020-11-20 15:20:11.000000000 +0100
@@ -3,13 +3,14 @@
 import (
 	"testing"

+	. "github.com/smartystreets/goconvey/convey"
+
 	"berty.tech/berty/experiment/dht"
 	"berty.tech/berty/experiment/dht/src/bittorrent"
 	"berty.tech/berty/experiment/dht/src/chord"
 	"berty.tech/berty/experiment/dht/src/gnunet"
 	"berty.tech/berty/experiment/dht/src/libp2p"
 	"berty.tech/berty/experiment/dht/src/matrix"
-	. "github.com/smartystreets/goconvey/convey"
 )

 func TestBasic(t *testing.T) {

@kwo
Copy link
Contributor Author

kwo commented Nov 20, 2020

Can you run gci by itself - not from the Makefile?

@moul
Copy link

moul commented Nov 20, 2020

moul@fwrz:~/go/src/github.com/daixiang0/gci (cache-stdlib-package-names) $ rm -f ~/go/bin/gci
moul@fwrz:~/go/src/github.com/daixiang0/gci (cache-stdlib-package-names) $ gci
-bash: /home/moul/go/bin/gci: No such file or directory
moul@fwrz:~/go/src/github.com/daixiang0/gci (cache-stdlib-package-names) $ git log -1
commit 316c2a0df03b96976c688ee08e187b8ebaea4c1c (HEAD -> cache-stdlib-package-names)
Author: Karl Ostendorf <karl@ostendorf.com>
Date:   Fri Nov 20 13:52:48 2020 +0100

    hardcode standard package names
moul@fwrz:~/go/src/github.com/daixiang0/gci (cache-stdlib-package-names) $ go install
moul@fwrz:~/go/src/github.com/daixiang0/gci (cache-stdlib-package-names) $ gci
moul@fwrz:~/go/src/github.com/daixiang0/gci (cache-stdlib-package-names) $ cd ~/go/src/berty.tech/berty/go
moul@fwrz:~/go/src/berty.tech/berty/go (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ gci -w -local berty.tech .
skip file cmd/berty/doc.go since no import
skip file cmd/berty/mini/doc.go since no import
skip file cmd/berty/mini/example_test.go since no import
[...]
skip file pkg/username/ios.go since no import
skip file pkg/username/others.go since no import
skip file pkg/username/username.go since no import
moul@fwrz:~/go/src/berty.tech/berty/go (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ echo $?
1

@kwo
Copy link
Contributor Author

kwo commented Nov 20, 2020

I think the issue is that you need to execute the command from the project root directory, where go.mod is located - not from <PROJECT_ROOT>/go. When I run the command from the <PROJECT_ROOT>/go directory, I get the same result as you (exit status 1). But from the <PROJECT_ROOT>, the desired result.

@moul
Copy link

moul commented Nov 20, 2020

btw, did you check the exit code? because when I run your command I also get some diff, but if I add echo $? after I clearly see that it diffs, but also exits with 1

moul@fwrz:~/go/src/berty.tech/berty (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ gci -d .
[...]
diff -u tool/test/experiment/dht/test/basic_test.go.orig tool/test/experiment/dht/test/basic_test.go
--- tool/test/experiment/dht/test/basic_test.go.orig    2020-11-20 15:54:35.467088817 +0100
+++ tool/test/experiment/dht/test/basic_test.go 2020-11-20 15:54:35.467088817 +0100
@@ -1,16 +1,14 @@
 package test
 
 import (
-       "testing"
-
-       . "github.com/smartystreets/goconvey/convey"
-
        "berty.tech/berty/experiment/dht"
        "berty.tech/berty/experiment/dht/src/bittorrent"
        "berty.tech/berty/experiment/dht/src/chord"
        "berty.tech/berty/experiment/dht/src/gnunet"
        "berty.tech/berty/experiment/dht/src/libp2p"
        "berty.tech/berty/experiment/dht/src/matrix"
+       . "github.com/smartystreets/goconvey/convey"
+       "testing"
 )
 
 func TestBasic(t *testing.T) {
moul@fwrz:~/go/src/berty.tech/berty (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ echo $?
1

same result if running from root or from go/ btw

@kwo
Copy link
Contributor Author

kwo commented Nov 20, 2020

The exit code is out of scope for my PRs but logically, shouldn't the exit code be non-zero if there are diffs?

@moul
Copy link

moul commented Nov 20, 2020

👍

in the case of -d yes, in the case of -w I don't think so, most of the linter/formatting tools are just exiting with 0 even if there are changes, but they return 1 in case of real errors (i.e., syntax error)

moul@fwrz:~/go/src/berty.tech/berty (dependabot/go_modules/github.com/daixiang0/gci-0.2.5) $ gci -w .; echo $?
skip file go/cmd/berty/doc.go since no import
skip file go/cmd/berty/mini/doc.go since no import
skip file go/cmd/berty/mini/example_test.go since no import
[...]
skip file go/pkg/username/others.go since no import
skip file go/pkg/username/username.go since no import
skip file tool/test/experiment/dht/dht.go since no import
1

btw, the version 0.2.4 returned 0 for -w, the behavior changed with 0.2.5

@kwo
Copy link
Contributor Author

kwo commented Nov 20, 2020

@daixiang0 my PR does not address the exit code issue - nevertheless I think hardcoding the package names is a good idea for performance reasons.

The exit code issue - returning exit code 0 when in write mode and no error occurs - belongs in a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants