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

Make use of cargo metadata #18

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Make use of cargo metadata #18

merged 2 commits into from
Apr 28, 2020

Conversation

qryxip
Copy link
Contributor

@qryxip qryxip commented Mar 24, 2020

  1. To free from CWD.

    Unlike other popular Cargo commands, cargo-atcoder currently assumes that CWD equals $CARGO_MANIFEST_DIR. We will be able to run cargo-atcoder everywhere under the workspace directory.

  2. To support --manifest-path and -p|--package.

    Building dependencies takes long time in general. Even proconio takes about 30 seconds with the derive feature.

    Warming up release build for `abc152`...
       Compiling proc-macro2 v0.4.30
       Compiling unicode-xid v0.1.0
       Compiling syn v0.15.44
       Compiling lazy_static v1.4.0
       Compiling quote v0.6.13
       Compiling proconio-derive v0.1.6
       Compiling proconio v0.3.6
       Compiling abc152 v0.1.0 (/home/ryo/src/local/abc152)
    warning: unused variable: `n`
     --> src/bin/b.rs:5:9
      |
    5 |         n: usize,
      |         ^ help: consider prefixing with an underscore: `_n`
      |
      = note: `#[warn(unused_variables)]` on by default
    
        Finished release [optimized] target(s) in 41.81s

    I'd like to propose that we create a large workspace and add the packages to the workspace members. In this way, we can keep using one build cache with the same dependency graph.

    (edit) Then we would have to put profile.release on the root manifest, and to rename bins uniquely.

    .
    ├── abc157
    │   ├── Cargo.toml
    │   └── src
    ├── abc158
    │   ├── Cargo.toml
    │   └── src
    ├── abc159
    │   ├── Cargo.toml
    │   └── src
    ├── Cargo.lock
    ├── Cargo.toml
    └── target
        ├── debug
        └── release
    # <workspace-root>/Cargo.toml
    
    [workspace]
    members = ["abc157", "abc158", "abc159"]
    exclude = ["abc154", "abc155", "abc156"]
    
    [profile.release]
    lto = true
    panic = 'abort'
    # <workspace-root>/abc159/Cargo.toml
    
    [package]
    name = "abc159"
    version = "0.1.0"
    authors = ["Ryo Yamashita <qryxip@gmail.com>"]
    edition = "2018"
    
    [[bin]]
    name = "abc159-a"
    path = "./src/bin/a.rs"
    
    [[bin]]
    name = "abc159-b"
    path = "./src/bin/b.rs"
    
    [[bin]]
    name = "abc159-c"
    path = "./src/bin/c.rs"
    
    [[bin]]
    name = "abc159-d"
    path = "./src/bin/d.rs"
    
    [[bin]]
    name = "abc159-e"
    path = "./src/bin/e.rs"
    
    [[bin]]
    name = "abc159-f"
    path = "./src/bin/f.rs"
    
    # Not here
    #[profile.release]
    #lto = true
    #panic = 'abort'
    
    [dependencies]
    proconio = { version = "0.3.6", features = ["derive"] }

    Currently, cargo-atcoder does not work with such a workspace. With cargo metadata, we can use cargo-atcoder on large workspaces.

@qryxip qryxip marked this pull request as ready for review March 25, 2020 10:26
@qryxip qryxip force-pushed the cargo-metadata branch 3 times, most recently from 10f4fde to d85a9b0 Compare March 31, 2020 18:01
@qryxip
Copy link
Contributor Author

qryxip commented Apr 4, 2020

--no-deps for cargo metadata was not good. I'll rewrite.

@qryxip
Copy link
Contributor Author

qryxip commented Apr 28, 2020

I will resolve the conflict.

@tanakh tanakh merged commit cd20464 into tanakh:master Apr 28, 2020
@tanakh
Copy link
Owner

tanakh commented Apr 28, 2020

It seems to be good feature. Thank you for nice patch! Warming up time was also annoying for me.

@qryxip qryxip deleted the cargo-metadata branch April 28, 2020 15:32
This was referenced May 11, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants