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

cargo publish --package flag is ignored when cargo isn't running in a workspace(?) #10536

Closed
nagisa opened this issue Apr 2, 2022 · 8 comments
Labels
C-bug Category: bug Command-publish P-high Priority: High

Comments

@nagisa
Copy link
Member

nagisa commented Apr 2, 2022

Problem

The stacker and psm crates are both part of the same repository, but they do not form a workspace. I tried publishing psm but cargo went on to try and publish a wrong crate (stacker).

Steps

  1. Construct a setup similar to how stacker repository is structured. 1 crate at the top level (stacker) and another as a subdirectory (psm);
  2. cd wherever-stacker-is;
  3. cargo publish -p psm.

Possible Solution(s)

Cargo perhaps should return an error, or publish the specified crate.

Notes

Interestingly, cargo test -p psm does the right thing despite there being no workspace.

Version

cargo 1.58.0 (f01b232bc 2022-01-19)
release: 1.58.0
commit-hash: f01b232bc7f4d94f0c4603930a5a96277715eb8c
commit-date: 2022-01-19
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1l)
os: NixOS 22.5.0 [64-bit]
@nagisa nagisa added the C-bug Category: bug label Apr 2, 2022
@nagisa nagisa changed the title cargo publish --package flag is ignored when cargo isn't running in a workspace cargo publish --package flag is ignored when cargo isn't running in a workspace(?) Apr 2, 2022
@ehuss
Copy link
Contributor

ehuss commented Apr 4, 2022

It seems reasonable to allow publishing a dependency as long as it is a path-dependency. If that is too hard to implement, then it should at least produce an error.

@Rustin170506
Copy link
Member

@rustbot claim

@likzn
Copy link
Contributor

likzn commented May 6, 2022

It seems reasonable to allow publishing a dependency as long as it is a path-dependency. If that is too hard to implement, then it should at least produce an error.

@ehuss Hi.May i ask. When we run cargo publish -p [spec].Must the [spec] in workspace or path-dependency? Can we use it directly not to judge.

@Rustin170506
Copy link
Member

Sorry I haven't had time to fix it lately, so I'm unassign myself.

@Rustin170506 Rustin170506 removed their assignment May 17, 2022
@likzn
Copy link
Contributor

likzn commented May 17, 2022

@ehuss Can you explain more about it? I will complete it.And i find when we publish ,we will chech if the published cargo is in workspace in line 93.If not, it will publish itself(root cargo) . So i want to know if this check is need. Can we use the Spec directly?

pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
let specs = opts.to_publish.to_package_id_specs(ws)?;
let mut pkgs = ws.members_with_features(&specs, &opts.cli_features)?;
let (pkg, cli_features) = pkgs.pop().unwrap();
let mut publish_registry = opts.registry.clone();

@ehuss
Copy link
Contributor

ehuss commented May 17, 2022

From a behavior standpoint, here are some things to check:

  • In the root of a virtual workspace, it should be an error to run without -p.
  • It should be an error to pass -p for a non-workspace member.
  • It should be an error for -p to match multiple packages.
  • When using -p, it should publish that package, not the one in the current directory (which can be different).

Checking those rules will require some additions to the publish function. I think it might look something like iterating over ws.members() and checking which one matches opts.to_publish. If none of them do, error. If more than one does, error. That is, it should only select one member. I think the virtual case can be checked with ws.is_virtual().

@likzn
Copy link
Contributor

likzn commented May 18, 2022

Get. Its clear. I will work on it lately.

bors added a commit that referenced this issue May 27, 2022
fix(publish): add more check when use `publish -p <SPEC>`

### Main issue
As issue say #10536 , we need add more check when user use `cargo publish -p <SPEC>`

>`@ehuss` point outs:
>From a behavior standpoint, here are some things to check:
> - In the root of a virtual workspace, it should be an error to run without -p.
>- It should be an error to pass -p for a non-workspace member.
>- It should be an error for -p to match multiple packages.
>- When using -p, it should publish that package, not the one in the current directory (which can be different).
ehuss pushed a commit to ehuss/cargo that referenced this issue May 27, 2022
fix(publish): add more check when use `publish -p <SPEC>`

### Main issue
As issue say rust-lang#10536 , we need add more check when user use `cargo publish -p <SPEC>`

>`@ehuss` point outs:
>From a behavior standpoint, here are some things to check:
> - In the root of a virtual workspace, it should be an error to run without -p.
>- It should be an error to pass -p for a non-workspace member.
>- It should be an error for -p to match multiple packages.
>- When using -p, it should publish that package, not the one in the current directory (which can be different).
@weihanglo
Copy link
Member

Should be resolved with #10677. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-publish P-high Priority: High
Projects
None yet
Development

No branches or pull requests

6 participants
@ehuss @nagisa @weihanglo @Rustin170506 @likzn and others