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

Marking Cargo.lock as generated #6180

Closed
nipunn1313 opened this issue Oct 16, 2018 · 5 comments · Fixed by #6548
Closed

Marking Cargo.lock as generated #6180

nipunn1313 opened this issue Oct 16, 2018 · 5 comments · Fixed by #6548
Assignees
@dwijnand
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@nipunn1313
Copy link
Contributor

Our code review tool (phabricator) will automatically hide changes to files that are marked as generated (via detecting the text @generated in the file)

It would be convenient if Cargo.lock had // @generated in the file, or some customizable comment to this effect.

I think we can configure phabricator to avoid this issue, but it seems also reasonable to have Cargo's defaults play nicely with phabricator's defaults.
@alexcrichton
Copy link
Member

I agree! I do regret at this point at least not having a comment at the top saying it's automatically generated. FWIW historically changing the lock file encoding is somewhat difficult because we don't want to roll it out immediately causing stable/nightly cargos to thrash on what the lock file should look like. In that sense the implementation here would take awhile to stabilize, but would look like:

  • Implement support in Cargo to preserve the comment at the top of the file. That is, if Cargo sees a comment at the top of a lock file, it preserves it.
  • Let this support ride the trains and bake on stable for a release or two
  • Update Cargo to emit a comment by default at the top

I think we can definitely figure out how to craft a comment that covers popular systems like GitHub, Phabricator, GitLab, etc, to automatically flag the file as generated.

@dwijnand would you perhaps be interested in kicking off the implementation here?

@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Oct 17, 2018
@dwijnand
Copy link
Member

So I had a look and I think this will have to wait for the switch from the toml crate to the toml_edit crate, as only the latter will preserve comments when deserialising/parsing.

@alexcrichton
Copy link
Member

Oh FWIW I don't think we need to block on that per se, we already have custom serialization for the lock file (as we want it formatted a particular way) and we could relatively easily cache the first lines that appear with # as well to "preserve" the top-level comment

@dwijnand
Copy link
Member

Hmm, ok let me see if I can figure that one.

@dwijnand dwijnand self-assigned this Oct 17, 2018
@dwijnand dwijnand mentioned this issue Oct 17, 2018
Merged
bors added a commit that referenced this issue Oct 18, 2018
@bors
Auto merge of #6181 - dwijnand:preserve-lockfile-top-comment, r=alexc…
8522a88 
…richton

Preserve lockfile top comment

Refs #6180
@dwijnand
Copy link
Member

@bors: remind me in 12/18 weeks to finish this.

This was referenced Jan 14, 2019
Merged
Closed
bors added a commit that referenced this issue Jan 22, 2019
@bors
Auto merge of #6548 - dwijnand:mark-lockfile-as-generated, r=alexcric…
3de188f 
…hton

Marking Cargo.lock as generated

12 weeks have passed, let's mark lock files as generated!

Fixes #6180
@topecongiro topecongiro mentioned this issue Jan 31, 2019
Merged
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 3, 2019
@emilio
Bug 1524822 - Fix warnings with rust 1.34+. r=jrmuizel
3d4396c 
There's some new limited const fn support in stable, and this is the recommended
way to initialize atomics now.

If this for some reason doesn't compile in all platforms / versions we support
I'll just sprinkle some #[allow(deprecated)] instead.

Also, cargo changes the output of Cargo.lock, see
rust-lang/cargo#6180. So also update those comments.

Differential Revision: https://phabricator.services.mozilla.com/D18495

--HG--
extra : moz-landing-system : lando
moz-gfx pushed a commit to moz-gfx/webrender that referenced this issue Feb 3, 2019
@emilio
Bug 1524822 - Fix warnings with rust 1.34+. r=jrmuizel
c835024 
There's some new limited const fn support in stable, and this is the recommended
way to initialize atomics now.

If this for some reason doesn't compile in all platforms / versions we support
I'll just sprinkle some #[allow(deprecated)] instead.

Also, cargo changes the output of Cargo.lock, see
rust-lang/cargo#6180. So also update those comments.

Differential Revision: https://phabricator.services.mozilla.com/D18495

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/5f006a3c652c3a5def8800b9b0e65c4213b86785
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Feb 3, 2019
@emilio
Bug 1524822 - Fix warnings with rust 1.34+. r=jrmuizel
f688e07 
There's some new limited const fn support in stable, and this is the recommended
way to initialize atomics now.

If this for some reason doesn't compile in all platforms / versions we support
I'll just sprinkle some #[allow(deprecated)] instead.

Also, cargo changes the output of Cargo.lock, see
rust-lang/cargo#6180. So also update those comments.

Differential Revision: https://phabricator.services.mozilla.com/D18495
@dwijnand dwijnand mentioned this issue Feb 12, 2019
Closed
@est31 est31 mentioned this issue Jul 12, 2019
Merged
nickolay added a commit to nickolay/git-absorb that referenced this issue Jul 14, 2019
@nickolay
Update Cargo.lock with the autogenerated message added in newer cargo
88b59c4 
rust-lang/cargo#6180
@est31 est31 mentioned this issue Aug 16, 2019
Closed
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
@marco-c
Bug 1524822 - Fix warnings with rust 1.34+. r=jrmuizel
0f862e9 
There's some new limited const fn support in stable, and this is the recommended
way to initialize atomics now.

If this for some reason doesn't compile in all platforms / versions we support
I'll just sprinkle some #[allow(deprecated)] instead.

Also, cargo changes the output of Cargo.lock, see
rust-lang/cargo#6180. So also update those comments.

Differential Revision: https://phabricator.services.mozilla.com/D18495

UltraBlame original commit: 5f006a3c652c3a5def8800b9b0e65c4213b86785
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
@marco-c
Bug 1524822 - Fix warnings with rust 1.34+. r=jrmuizel
7d68c69 
There's some new limited const fn support in stable, and this is the recommended
way to initialize atomics now.

If this for some reason doesn't compile in all platforms / versions we support
I'll just sprinkle some #[allow(deprecated)] instead.

Also, cargo changes the output of Cargo.lock, see
rust-lang/cargo#6180. So also update those comments.

Differential Revision: https://phabricator.services.mozilla.com/D18495

UltraBlame original commit: 5f006a3c652c3a5def8800b9b0e65c4213b86785
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 1524822 - Fix warnings with rust 1.34+. r=jrmuizel
298a2c3 
There's some new limited const fn support in stable, and this is the recommended
way to initialize atomics now.

If this for some reason doesn't compile in all platforms / versions we support
I'll just sprinkle some #[allow(deprecated)] instead.

Also, cargo changes the output of Cargo.lock, see
rust-lang/cargo#6180. So also update those comments.

Differential Revision: https://phabricator.services.mozilla.com/D18495

UltraBlame original commit: 5f006a3c652c3a5def8800b9b0e65c4213b86785
@dwrensha dwrensha mentioned this issue Oct 24, 2019
Merged
@Timmmm Timmmm mentioned this issue Dec 9, 2019
Closed
@lovesegfault lovesegfault mentioned this issue Feb 6, 2020
Closed
ebkalderon pushed a commit to cargo2nix/cargo2nix that referenced this issue Feb 13, 2020
Mark Cargo.nix with @generated keyword
878de69 
It appears that some code review tools (e.g. Phabricator) will
automatically hide code changes made to files marked with the special
`@generated` keyword. Multiple package managers ([including Cargo]) have
added this keyword to their generated lockfiles to better accommodate
users of these tools and reduce unnecessary diff noise during code
reviews.

[including Cargo]: rust-lang/cargo#6180

This commit changes the comment at the top of all `Cargo.nix` files to
the following text:

```
This file is @generated by cargo2nix X.Y.Z.
It is not intended to be manually edited.
```
@ebkalderon ebkalderon mentioned this issue Feb 13, 2020
Merged
@lovesegfault lovesegfault mentioned this issue Feb 13, 2020
Closed
2 tasks
ebkalderon pushed a commit to cargo2nix/cargo2nix that referenced this issue Feb 14, 2020
Mark Cargo.nix with @generated keyword
4383f73 
It appears that some code review tools (e.g. Phabricator) will
automatically hide code changes made to files marked with the special
`@generated` keyword. Multiple package managers ([including Cargo]) have
added this keyword to their generated lockfiles to better accommodate
users of these tools and reduce unnecessary diff noise during code
reviews.

[including Cargo]: rust-lang/cargo#6180

This commit changes the comment at the top of all `Cargo.nix` files to
the following text:

```
This file is @generated by cargo2nix X.Y.Z.
It is not intended to be manually edited.
```
ebkalderon pushed a commit to cargo2nix/cargo2nix that referenced this issue Feb 14, 2020
Mark Cargo.nix with @generated keyword
0f27df7 
It appears that some code review tools (e.g. Phabricator) will
automatically hide code changes made to files marked with the special
`@generated` keyword. Multiple package managers ([including Cargo]) have
added this keyword to their generated lockfiles to better accommodate
users of these tools and reduce unnecessary diff noise during code
reviews.

[including Cargo]: rust-lang/cargo#6180

This commit changes the comment at the top of all `Cargo.nix` files to
the following text:

```
This file is @generated by cargo2nix X.Y.Z.
It is not intended to be manually edited.
```
@sfackler sfackler mentioned this issue Jun 1, 2020
Closed
@est31 est31 mentioned this issue Jul 22, 2020
Merged
@evaera evaera mentioned this issue Mar 23, 2021
Open
@BlueGlassBlock BlueGlassBlock mentioned this issue Jan 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dwijnand dwijnand

Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Marking Cargo.lock as generated dwijnand/cargo
3 participants
@alexcrichton @dwijnand @nipunn1313