Skip to content

Migrate 'docopt' to 'structopt'#295

Merged
bors[bot] merged 11 commits intokillercup:masterfrom
DCjanus:master
May 27, 2019
Merged

Migrate 'docopt' to 'structopt'#295
bors[bot] merged 11 commits intokillercup:masterfrom
DCjanus:master

Conversation

@DCjanus
Copy link
Copy Markdown
Contributor

@DCjanus DCjanus commented May 24, 2019 •
edited by ordian
Loading

Close #207

@DCjanus
Copy link
Copy Markdown
Contributor Author

DCjanus commented May 24, 2019

Beacuse of backtrace(>=0.3.16), we have to install MinGW for windows-gnu targets.

@DCjanus DCjanus changed the title [WIP]Migrate 'docopr' to 'structopt' Migrate 'docopr' to 'structopt' May 24, 2019
@DCjanus
Copy link
Copy Markdown
Contributor Author

DCjanus commented May 24, 2019 •
edited
Loading

Tried to upgrade the cargo-metadata version(0.6 -> 0.7), test failed and have to delete ~/.cargo/registry/src/xxx-your-registry/libc-*. I don't know how to fix that.

Output:

thread 'upgrade_as_expected' panicked at 'cargo-upgrade failed to execute', tests/utils.rs:62:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
test upgrade_as_expected ... FAILED
thread 'upgrade_prints_messages' panicked at 'Assertion failed for `target/debug/cargo-upgrade upgrade docopt --manifest-path=/tmp/cargo-edit-test.04q33Nv75XB6/Cargo.toml`
with: Unexpected return status: failure
stdout=``````
stderr=```Command failed due to unhandled error: Invalid manifest

Caused by: error during execution of `cargo metadata`:     Blocking waiting for file lock on the registry index
    Updating `git://mirrors.ustc.edu.cn/crates.io-index` index
    Blocking waiting for file lock on the git checkouts
    Updating git repository `https://github.com/serde-rs/serde.git`
 Downloading crates ...
error: failed to download `libc v0.2.55`

Caused by:
  unable to get packages from source

Caused by:
  failed to download replaced source registry `https://github.com/rust-lang/crates.io-index`

Caused by:
  failed to parse manifest at `/home/dcjanus/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/libc-0.2.55/Cargo.toml`

Caused by:
  failed to parse the version requirement `rustc-std-workspace-core--CURRENT_VERSION_TEST` for dependency `rustc-std-workspace-core`

Caused by:
  the given version requirement is invalid

```', /home/dcjanus/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/assert_cli-0.6.3/src/assert.rs:441:13
test upgrade_prints_messages ... FAILED

@ordian
Copy link
Copy Markdown
Collaborator

ordian commented May 24, 2019

@DCjanus thanks for the PR, I'll try to have a look this weekend.

@DCjanus DCjanus changed the title Migrate 'docopr' to 'structopt' Migrate 'docopt' to 'structopt' May 24, 2019
ordian
ordian reviewed May 26, 2019
View reviewed changes
Comment thread src/bin/add/args.rs Outdated
Comment thread src/bin/add/args.rs Outdated
Comment thread src/bin/add/args.rs Outdated
@DCjanus
Some mirror change: rename, comment location and arg conflict
186c6dd 
rename: 'ArgsWrap' -> 'Command'
comment location: put docs above attributes
arg conflict: 'path' and 'vers' are not conflicting
@DCjanus
Copy link
Copy Markdown
Contributor Author

DCjanus commented May 27, 2019

@ordian Thanks for your help, would you mind review again?

ordian
ordian approved these changes May 27, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ordian ordian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this PR closes #283. Anyway let's fix that issue in a separate PR. Other than that, LGTM.

@ordian
Copy link
Copy Markdown
Collaborator

ordian commented May 27, 2019

bors r+

bors Bot added a commit that referenced this pull request May 27, 2019
@bors @DCjanus
Merge #295
abed104 
295: Migrate 'docopt' to 'structopt' r=ordian a=DCjanus

Close #207

Co-authored-by: DCjanus <dcjanus@dcjanus.com>
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented May 27, 2019

Build succeeded

@bors bors Bot merged commit 186c6dd into killercup:master May 27, 2019
@DCjanus
Copy link
Copy Markdown
Contributor Author

DCjanus commented May 27, 2019

I don't think this PR closes #283

With these code, cargo-edit parse dependencies one by one, which means it could process multiple path dependencies. So I do believe this PR resolved #283

The reason that I changed those in this PR: It's hard to define some complex constraints in 'structopt'.

If I forgot to remind you to pay attention to these changes in the review, I'm so sorry.

@ordian

@ordian ordian mentioned this pull request May 27, 2019
Closed
@ordian
Copy link
Copy Markdown
Collaborator

ordian commented May 27, 2019

Ah, sorry, you're right.

@ordian
Copy link
Copy Markdown
Collaborator

ordian commented May 27, 2019

I've noticed this PR introduced a regression with cargo upgrade <crate> --all in a workspace. @DCjanus would you mind taking a look?

ordian
ordian reviewed May 27, 2019
View reviewed changes
Comment thread src/bin/upgrade/main.rs
/// `--version`
flag_version: bool,
/// Crates to be upgraded.
#[structopt(conflicts_with = "all")]
Copy link
Copy Markdown
Collaborator

@ordian ordian May 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I found the suspect.

@ordian ordian mentioned this pull request May 27, 2019
Merged
bors Bot added a commit that referenced this pull request May 27, 2019
@bors @ordian
Merge #296
579f949 
296: [upgrade] `all` doens't conflict with `dependency` r=ordian a=ordian

Follow up to #295.

Co-authored-by: Andronik Ordian <write@reusable.software>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ordian ordian ordian approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Switch to Clap

2 participants

@DCjanus @ordian