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

Reconsider implicitly packaging all local files by default #2245

Closed
felixc opened this issue Dec 24, 2015 · 2 comments
Closed

Reconsider implicitly packaging all local files by default #2245

felixc opened this issue Dec 24, 2015 · 2 comments

Comments

@felixc
Copy link
Contributor

felixc commented Dec 24, 2015

I'm not sure whether to file this as a bug, a request for clearer documentation, or a suggestion for concrete changes in behaviour. Regardless, I wanted to bring up how cargo package and cargo publish implicitly grab all local files in the directory and include them in the crate. (Yes, because I was just caught out by this, though fortunately I don't believe I've published anything sensitive — but I'm about to go and double-check all my old versions).

What I'd like to argue is that implicitly grabbing all files in the local directory and including them in the published crate is a very dangerous feature. At least my mental model of "the contents of my crate" consists of just source files, docs, tests, and ancillary files like README and LICENSE; not of everything that happens to be in the same directory.

Like the Python folks say, "explicit is better than implicit". The safe default behaviour would be to require an explicit listing of what files to include (yes, I know the option exists, but it's not the default). If you'd like to encourage people to remember to keep it up to date, you could print a warning when you see e.g. a LICENSE or CONTRIBUTING file that isn't explicitly listed, or even any new file found for the first time. Having everything grabbed would work better as an opt-in for those who know about the feature and choose to accept the risk, but defaults should be chosen so as to fail in a safe manner.

If, on the other hand, this behaviour is really too desirable to give up, it should be very very explicitly called out, and loudly signalled (i.e. printed to stdout) when it does happen.

Currently the only mention of it that I can find is in the crates.io doc here, and even that isn't particularly explicit or clear. Firstly, I never though of my TODO list for the crate, my personal notes on my release process, etc. as "part of the crate", and thus publishable. Secondly, though it says "Cargo will automatically ignore files ignored by your version control system when packaging", I assumed that meant "files not tracked by your version control system", rather than specifically "files listed in .gitignore" (the only supported ignore mechanism, as far as I can tell).

But, most of all, it's a big scary footgun feature that's just mentioned in passing in one skimmable section (I admit I went straight to cargo upload the first time I read this, and skipped package altogether).

So, in sum, please reconsider the default behaviour of this feature, and please aim for safety over convenience, even if convenience is offered as an option for those who choose it.

@felixc
Copy link
Contributor Author

felixc commented Dec 24, 2015

This appears to have been partially discussed previously in #880 (which resulted in adding the option to use a whitelist), and in #1170 (where @huonw had similar concerns), but I'm not sure what the overall consensus is.

(Thanks to @steveklabnik for the pointers to these threads on #rust)

@alexcrichton
Copy link
Member

I think this is a dupe of #2063 so I'm gonna close in favor of that, but thanks for the report! (I'll copy over your comment over there)

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

No branches or pull requests

2 participants