Block publishing on dirty directories

[sbeckeriv]

Dearest reviewer,

This is part of #841 which is about the publishing and tagging flow.
This pull request just prevents publishing with uncommitted or untracked
files. I have included a flag for turning this off. There are two new
test and I have also updated the docs.

More details about the full flow at
#2443 . I plan on doing more
work on the flow, however, I felt this was useful enough to do stand
alone. When I tried the flow before I was doing to much at once.

closes #1597

Thanks!
Becker

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

This may actually be best configured via a command line flag perhaps? I suspect that this flag would be passed in one-off situations rather than blanket configured for the whole system

[alexcrichton]

Thanks @sbeckeriv! Looking good to me!

[sbeckeriv]

Updated with the changes. I think there is an edge case with open_ext. I believe you can run cargo publish from any sub level of directories. I think if you have something like this setup you will see an odd result

- level 1 (git)
-- project (hg)
--- sub project (still hg)

run cargo publish from sub project it will catch the level 1 git repo current. I don't know why someone would have this setup. Its just a thought. Trying to open the git repository is the best thing I can think to check for the version control system at this time. Also the flag will still allow you to publish.

Thanks again.

[alexcrichton]

Stylistically a space typically comes before the { here (also a few other places in this diff)

[alexcrichton]

This can probably just be:

if !files.is_empty() {
    bail!("...");
}
Ok(())

[bors]

☔ The latest upstream changes (presumably #2722) made this pull request unmergeable. Please resolve the merge conflicts.

[tomaka]

Cargo should be orthogonal to the version control system 😠

[alexcrichton]

@tomaka I'd personally at least find it a little more helpful if you could be more constructive in your criticism as well, as blanket stating "cargo should be orthogonal" isn't too actionable. Do you have specific concerns that you'd like to see addressed?

In general Cargo should indeed not worry about an underlying version control system, but that also doesn't mean that it should ignore it entirely. Cargo strives to recognize the underlying VCS to make workflows much more ergonomic in aspects like:

• Preventing duplication of .gitignore in Cargo.toml by inferring it.
• Enabling auto-tagging releases
• Preventing very likely bugs (like this PR is doing) where untracked changes may be included by accident

In each of these situations Cargo doesn't need to have a VCS or require something like git, it simply recognizes that git exists and attempts to infer as much from git as possible. This enables many more "zero configuration" use cases of Cargo where it all just works without having to do much. Cargo is explicitly designed to support more than one VCS as well, it just so happens that work has only been done so far to work with git, not others like mercurial.

[tomaka]

@alexcrichton I just brought up this point here without arguing since it has already been discussed in the workspace RFC.

The problem when you assume that the user does someone in a specific way is that it breaks horribly and in an unpredictable way when people use for example git differently than how it was supposed to be used. Tools that try to be smarter than the programmer tend to be annoying as soon as you try to do something slightly different than what's expected.

For example what if for some reason I write a script that applies a patch, then runs cargo publish, then reverts the change? Does that meant that the script will now have to create a commit, then publish, then revert the commit? What will the person who wrote the script think of this PR?

The follow-up is that at least the exact behavior of each command should be documented. If cargo publish checks the current git repo for untracked changes, then the documentation of cargo publish must say so in order to not surprise people.

[alexcrichton]

@tomaka yes it's possible for what we infer to be wrong, and that's why there's always an escape hatch. For example an --allow-untracked option being added in this PR is the escape hatch for something like that. The theory is that the number of situations where inference goes wrong is far fewer than the number of situations which need zero configuration which would otherwise require some.

[sbeckeriv]

I am closing this until I learn more about git2. Someone can pick it up it up if they like.

Sorry
Becker