Prune should remove contents of root package directory when only subpackages are used

[kevinburke]

What version of dep are you using (dep version)?

v0.1.0-585-g34835a5

What dep command did you run?

dep init; dep ensure; dep prune.

I've set up a test repository with the results here: https://github.com/kevinburke/dep-test-bson

What did you expect to see?

I expected the vendor directory to have the following contents (and only the following contents)

vendor/gopkg.in/mgo.v2/bson/*
vendor/gopkg.in/mgo.v2/internal/json/*

What did you see instead?

I see all of the files and contents in the gopkg.in/mgo.v2 package as well:

$ ll vendor/gopkg.in/mgo.v2
total 1104
drwxr-xr-x  33 kevin  staff   1.1K Sep  1 13:27 .
drwxr-xr-x   3 kevin  staff   102B Sep  1 13:27 ..
-rw-r--r--   1 kevin  staff   1.0K Sep  1 13:27 .travis.yml
-rw-r--r--   1 kevin  staff   1.3K Sep  1 13:27 LICENSE
-rw-r--r--   1 kevin  staff    67B Sep  1 13:27 Makefile
-rw-r--r--   1 kevin  staff   136B Sep  1 13:27 README.md
-rw-r--r--   1 kevin  staff    12K Sep  1 13:27 auth.go
-rw-r--r--   1 kevin  staff    31K Sep  1 13:27 auth_test.go
drwxr-xr-x  12 kevin  staff   408B Sep  1 13:27 bson
-rw-r--r--   1 kevin  staff   9.8K Sep  1 13:27 bulk.go
-rw-r--r--   1 kevin  staff    14K Sep  1 13:27 bulk_test.go
-rw-r--r--   1 kevin  staff    19K Sep  1 13:27 cluster.go
-rw-r--r--   1 kevin  staff    54K Sep  1 13:27 cluster_test.go
-rw-r--r--   1 kevin  staff   1.2K Sep  1 13:27 doc.go
-rw-r--r--   1 kevin  staff   586B Sep  1 13:27 export_test.go
-rw-r--r--   1 kevin  staff    21K Sep  1 13:27 gridfs.go
-rw-r--r--   1 kevin  staff    16K Sep  1 13:27 gridfs_test.go
drwxr-xr-x   3 kevin  staff   102B Sep  1 13:27 internal
-rw-r--r--   1 kevin  staff   3.8K Sep  1 13:27 log.go
-rw-r--r--   1 kevin  staff   2.8K Sep  1 13:27 queue.go
-rw-r--r--   1 kevin  staff   2.7K Sep  1 13:27 queue_test.go
-rw-r--r--   1 kevin  staff    57B Sep  1 13:27 raceoff.go
-rw-r--r--   1 kevin  staff    55B Sep  1 13:27 raceon.go
-rw-r--r--   1 kevin  staff   224B Sep  1 13:27 saslimpl.go
-rw-r--r--   1 kevin  staff   194B Sep  1 13:27 saslstub.go
-rw-r--r--   1 kevin  staff    12K Sep  1 13:27 server.go
-rw-r--r--   1 kevin  staff   145K Sep  1 13:27 session.go
-rw-r--r--   1 kevin  staff   103K Sep  1 13:27 session_test.go
-rw-r--r--   1 kevin  staff    18K Sep  1 13:27 socket.go
-rw-r--r--   1 kevin  staff   3.4K Sep  1 13:27 stats.go
-rw-r--r--   1 kevin  staff   5.9K Sep  1 13:27 suite_test.go
-rw-r--r--   1 kevin  staff   218B Sep  1 13:27 syscall_test.go
-rw-r--r--   1 kevin  staff   181B Sep  1 13:27 syscall_windows_test.go

[kevinburke]

Previously, previously, previously

[carolynvs]

@kevinburke Am I summarizing correctly that you were expecting the root package contents (e.g. gopkg.in/mgo.v2/stats.go) to be pruned because your code is only importing a subpackage?

[sdboyer]

ohh interesting - yeah prune does need to empty out leading dirs that have to be there for structural purposes/because subdirs are required.

[kevinburke]

Carolyn - yes, that's correct

On Fri, Sep 1, 2017 at 13:41 sam boyer ***@***.***> wrote: ohh interesting - yeah prune does need to empty out leading dirs that have to be there for structural purposes/because subdirs are required. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1113 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOSI-aTv4LykxZh0Men1_mp9LclG0z_ks5seGvbgaJpZM4PKmZ4> .

--

…

-- Kevin Burke 925.271.7005 | kev.inburke.com

[ibrasho]

#1020 does this in the pruneUnusedPackages func.

[carolynvs]

@ibrasho Is there a test in that PR that demonstrates unused packages being pruned?

[ibrasho]

Not yet. It was in the original PR. I'm still working on completing #1020.

[ibrasho]

Since this is a separate issue... 😁

Which files should be deleted in an "usused" package?
Currently, #1020 deletes all files (including symlinks) except those matching some patterns (legal files and such). Is this the correct behavior?

Pruning options will be definable per-project, so this should pose a small issue anyway.

[ibrasho]

@sdboyer I'm not sure about deleting symlinks tbh. If a package relies on symlinks for package name this could cause unexpected issues.

[kevinburke]

This also causes a poor interaction with Bazel. For example, I only need golang.org/x/crypto/ssh/terminal. But because golang.org/x/crypto/ssh has all of the Go files in it, Bazel generates a BUILD.bazel file for it, which then means that Bazel expects all of the dependencies for golang.org/x/crypto/ssh to be in my vendor directory, even though they're unnecessary.

More discussion here: bazel-contrib/rules_go#725

[MarkusTeufelberger]

I would expect the following behavior for dep ensure:

After running this command, all files in vendor/... are there because they either have to be there because of

• legal reasons (LICENSE files)
• user choice (e.g. keep files named *.md)
• removing this file causes the actual project to either not compile at all or to compile differently compared to the case where this file is present in the vendor folder

Everything else (go code or not) that is in the vendor folder, but does NOT influence the compilation of the actual project and is not there for the first 2 reasons is useless and must be removed from the vendor folder.

This would solve issues deciding what should be kept: optional dependencies (library A depends on library B in some cases, but the actual project doesn't use these parts of library A anyways), symlinks (if they are not present and the project still compiles the same way, they are not necessary), random other files (.travis.yml really doesn't need to be vendored...) and so on could be evaluated against these criteria, even automatically by looking at hashes of compilation units.

[sdboyer]

oh, this is fixed now.

@MarkusTeufelberger pruning behavior is now established. fixes are possible, of course, but they need to be in a new issue. (and I'm not actually sure if you're trying to propose a change to the v0.4.1 behavior here)

[MarkusTeufelberger]

I'm not actually sure if you're trying to propose a change to the v0.4.1 behavior here

Neither am I, is it documented somewhere in detail?

#1580 for example sounds like it is not working as I would expect it to work (vendored testdata or vendored tests in general shouldn't be able to influence the outcome of a compilation).

"unused-packages indicates that files from directories that do not appear in the package import graph should be pruned." from https://golang.github.io/dep/docs/Gopkg.toml.html#prune sounds close, I guess I'm proposing an unused-files mode:

"unused-files indicates that all files that do not cause a change in the resulting package should be pruned."

I can of course open an issue, if that sounds like a reasonable proposal.

[sdboyer]

you linked to the docs we have so far. writing up a section to further explore pruning is a TODO.

that specific issue is a bit of an oddity, and is more of a bug than anything. however, it is not the case that we can necessarily assume testdata can be discarded - it is a terrible idea, but not disallowed to import from non-test Go code (and I've seen it in the wild in some protobuf library somewhere). so, we can't really treat the contents of those directories specially with respect to pruning rules.

how is what you're describing different from non-go?

(actually, yes, please open a new issue for this, and answer that question there 😊)

[MarkusTeufelberger]

See #1614