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.

Sign up for GitHub

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

Jump to bottom

Stop commit of Cargo.lock #117

Merged
merged 1 commit into from
Feb 27, 2021
Merged

Stop commit of Cargo.lock #117

merged 1 commit into from
Feb 27, 2021

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Feb 27, 2021 •
edited
Loading

UPDATE: #117 (comment)

I noticed that cargo automatically publishes Cargo.lock when there is binary in a crate. Therefore, there are no users who may be badly affected by this change.

$ cargo install cargo-hack --locked
    Updating crates.io index
  Downloaded cargo-hack v0.5.8
  Downloaded 1 crate (46.1 KB) in 1.22s
  Installing cargo-hack v0.5.8
   Compiling autocfg v1.0.1
   Compiling libc v0.2.103
   Compiling serde v1.0.130
   Compiling ryu v1.0.5
   Compiling serde_json v1.0.68
   Compiling cfg-if v1.0.0
   Compiling anyhow v1.0.44
   Compiling bitflags v1.3.2
   Compiling itoa v0.4.8
   Compiling termcolor v1.1.2
   Compiling memoffset v0.6.4
   Compiling nix v0.23.0
   Compiling atty v0.2.14
   Compiling toml v0.5.8
   Compiling ctrlc v3.2.1
   Compiling cargo-hack v0.5.8
    Finished release [optimized] target(s) in 13.44s
  Installing /Users/taiki/.cargo/bin/cargo-hack
   Installed package `cargo-hack v0.5.8` (executable `cargo-hack`)
$ cargo install cargo-hack -f
    Updating crates.io index
  Installing cargo-hack v0.5.8
   Compiling libc v0.2.112
   Compiling autocfg v1.0.1
   Compiling serde v1.0.132
   Compiling bitflags v1.3.2
   Compiling serde_json v1.0.73
   Compiling cfg-if v1.0.0
   Compiling anyhow v1.0.52
   Compiling ryu v1.0.9
   Compiling itoa v1.0.1
   Compiling termcolor v1.1.2
   Compiling memoffset v0.6.5
   Compiling nix v0.23.1
   Compiling atty v0.2.14
   Compiling toml v0.5.8
   Compiling ctrlc v3.2.1
   Compiling cargo-hack v0.5.8
    Finished release [optimized] target(s) in 14.36s
   Replacing /Users/taiki/.cargo/bin/cargo-hack
    Replaced package `cargo-hack v0.5.8` with `cargo-hack v0.5.8` (executable `cargo-hack`)

So, I'll remove the mention of this change from the changelog.

@taiki-e
Stop commit of Cargo.lock
7e3d0fa 
If you want to use cargo-hack with versions of dependencies at the time
of release, please download the compiled binary from GitHub Releases.
Upload of the compiled binary is part of the automated release process
and installation from GitHub Releases is much faster than installation
from the source.
@taiki-e
Copy link
Owner Author

taiki-e commented Feb 27, 2021 •
edited
Loading

cargo install ignores Cargo.lock by default and uses the latest dependencies.
As we committing Cargo.lock, the current CI settings cannot guarantee that crate is working properly with the latest dependencies at the time of release. (Given that many people are installing cargo-hack with CI without using --locked, that can actually be a problem.)

Of course, we can guarantee this by making CI complicate, but given that the cargo-hack is small and no one seems to be installing cargo-hack with the --locked flag, I think it's okay to do this for now. EDIT: see #117 (comment)

If someone needs it in the future, please comment on this PR or open a new issue. I can revisit this decision. EDIT: see #117 (comment)

@taiki-e
Copy link
Owner Author

taiki-e commented Feb 27, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 27, 2021

Build succeeded:

@bors bors bot merged commit 64696b6 into master Feb 27, 2021
@bors bors bot deleted the cargo-lock branch February 27, 2021 12:58
@taiki-e
Copy link
Owner Author

taiki-e commented Dec 28, 2021 •
edited
Loading

I noticed that cargo automatically publishes Cargo.lock when there is binary in a crate.
Therefore, there are no users who may be badly affected by this change.

$ cargo install cargo-hack --locked
    Updating crates.io index
  Downloaded cargo-hack v0.5.8
  Downloaded 1 crate (46.1 KB) in 1.22s
  Installing cargo-hack v0.5.8
   Compiling autocfg v1.0.1
   Compiling libc v0.2.103
   Compiling serde v1.0.130
   Compiling ryu v1.0.5
   Compiling serde_json v1.0.68
   Compiling cfg-if v1.0.0
   Compiling anyhow v1.0.44
   Compiling bitflags v1.3.2
   Compiling itoa v0.4.8
   Compiling termcolor v1.1.2
   Compiling memoffset v0.6.4
   Compiling nix v0.23.0
   Compiling atty v0.2.14
   Compiling toml v0.5.8
   Compiling ctrlc v3.2.1
   Compiling cargo-hack v0.5.8
    Finished release [optimized] target(s) in 13.44s
  Installing /Users/taiki/.cargo/bin/cargo-hack
   Installed package `cargo-hack v0.5.8` (executable `cargo-hack`)
$ cargo install cargo-hack -f
    Updating crates.io index
  Installing cargo-hack v0.5.8
   Compiling libc v0.2.112
   Compiling autocfg v1.0.1
   Compiling serde v1.0.132
   Compiling bitflags v1.3.2
   Compiling serde_json v1.0.73
   Compiling cfg-if v1.0.0
   Compiling anyhow v1.0.52
   Compiling ryu v1.0.9
   Compiling itoa v1.0.1
   Compiling termcolor v1.1.2
   Compiling memoffset v0.6.5
   Compiling nix v0.23.1
   Compiling atty v0.2.14
   Compiling toml v0.5.8
   Compiling ctrlc v3.2.1
   Compiling cargo-hack v0.5.8
    Finished release [optimized] target(s) in 14.36s
   Replacing /Users/taiki/.cargo/bin/cargo-hack
    Replaced package `cargo-hack v0.5.8` with `cargo-hack v0.5.8` (executable `cargo-hack`)

So, I'll remove the mention of this change from the changelog.

@jirutka
Copy link

jirutka commented May 8, 2022

Therefore, there are no users who may be badly affected by this change.

That’s not true, (linux) distributions uses this file. We build rust crates from the source repository, not crates.io. Can you please revert this and put Cargo.lock back where it should be, into the repository?

@taiki-e
Copy link
Owner Author

taiki-e commented May 8, 2022

As discussed in taiki-e/cargo-llvm-cov#152, I'm open to accepting a PR that commits lockfile if non-cargo package managers require it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@taiki-e @jirutka