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

add --dry-run to cargo publish #1332

Closed
shepmaster opened this issue Feb 22, 2015 · 5 comments
Closed

add --dry-run to cargo publish #1332

shepmaster opened this issue Feb 22, 2015 · 5 comments
Labels

Comments

@shepmaster
Copy link
Member

Running cargo package reported no errors, but running cargo publish warns:

all dependencies must come from the same source

The docs seems to indicate that cargo package is to be used as a "pre-flight" check of the package. I would have expected cargo publish to only be able to fail for reasons that involve communicating with crates.io proper.

@alexcrichton
Copy link
Member

I initially agreed with this, but I think I may be convincing myself of the opposite now. In terms of future expansion, we could have cargo package output anything from a tarball to an rpm, for example. We could also in theory provide fun options for cargo package like --include-dependencies or other minutia related to distributing source code.

Technically the restriction that all dependencies come from the same source is only imposed by the registry itself, not the packaging step. I suppose what I'm getting at is that if we only ever use cargo package to generate a .crate to upload then this makes sense, but if we expand beyond that it may not end up making sense.

@shepmaster
Copy link
Member Author

I think your explanation makes sense, so maybe this becomes a feature request for a --dry-run switch for cargo publish then. That way, we can check for specific issues for a repository (and a parallel cargo rpm --dry-run in the future, as you say).

@alexcrichton alexcrichton changed the title cargo package should warn about "all dependencies must come from the same source" add --dry-run to cargo publish Feb 26, 2015
@alexcrichton
Copy link
Member

Updated title as I think it'd be a nice piece to have as well!

@JustAPerson
Copy link
Contributor

So as tagged, this is a pretty simple thing to take care of. I've gone ahead and patched src/bin/publish.rs and src/cargo/ops/registry.rs, but I just want some input on what the output should look like.

Here's a sample of what it looks like currently
image

I'm not sure if Config::shell().status() is the correct way to output here. Additionally, should the --no-verify and --dry-run flags be mutually exclusive (i.e. error if both are supplied)? What should the messages look like?

With some feedback on what the cargo developers would like here, I'll fix things up and open a pull request.

@alexcrichton
Copy link
Member

@JustAPerson that looks pretty good to me! Perhaps it could actually print "Uploading"... as usual but just print a warning status right after saying "Aborting upload due to dry run" or something like that.

JustAPerson added a commit to JustAPerson/cargo that referenced this issue Jun 9, 2015
wezm added a commit to wezm/cargo that referenced this issue Jul 17, 2016
Squashed commit of the following:

commit deed1d7b99c1cd142f7782d3b3b782d949e1f71f
Author: Wesley Moore <[email protected]>
Date:   Fri Jul 15 13:35:01 2016 +1000

    Remove --dry-run and --no-verify mutual exclusion

commit 8a91fcf2a1aa3ba682fee67bb5b3e7c2c2cce8ef
Merge: 0c0d057 970535d
Author: Wesley Moore <[email protected]>
Date:   Fri Jul 15 13:30:38 2016 +1000

    Merge remote-tracking branch 'upstream/master' into publish_dry_run

commit 0c0d0572533599b3c0e42797a6014edf480f1dc2
Author: Wesley Moore <[email protected]>
Date:   Tue Jul 12 08:03:15 2016 +1000

    Improve grammar in --dry-run option

commit a17c1bf6f41f016cafdcb8cfc58ccbe34d54fbb8
Author: Wesley Moore <[email protected]>
Date:   Mon Jul 11 14:17:41 2016 +1000

    Add test for passing no-verify and dry-run to publish

commit 284810cca5df3268596f18700c0247de2f621c98
Author: Wesley Moore <[email protected]>
Date:   Mon Jul 11 14:51:38 2016 +1000

    Add test for publish --dry-run

commit 8514e47fbce61c20b227815887a377c25d17d004
Merge: 2b061c5 ef07b81
Author: Wesley Moore <[email protected]>
Date:   Mon Jul 11 08:27:10 2016 +1000

    Merge branch 'publish_dry_run' of github.com:JustAPerson/cargo into publish_dry_run

commit ef07b81
Author: Jason Priest <[email protected]>
Date:   Tue Jun 9 23:11:51 2015 -0500

    Improve publish `--dry-run`

    Catch a few more errors by aborting midway through transmit().

commit 0686fb0
Author: Jason Priest <[email protected]>
Date:   Tue Jun 9 14:38:58 2015 -0500

    Teach publish the `--dry-run` flag

    Closes rust-lang#1332
bors added a commit that referenced this issue Jul 18, 2016
Add --dry-run to cargo publish

This PR picks up where @JustAPerson left off in #1699. I've updated their changes to apply to current master and added tests, including (I think) a test that checks that the upload doesn't actually occur.

The output is as it was before:

![output](https://cloud.githubusercontent.com/assets/21787/16720362/f7ea710e-4778-11e6-8163-65f3978bb7ae.png)

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

No branches or pull requests

3 participants