On the graphics project branch we have added webrender as a dependency of libgkrust, and that pulls in all sorts of third-party dependencies. We are also doing regular merges from m-c to graphics. When somebody does |mach vendor rust| on m-c (e.g. to update the url library), that results in a merge conflict when merging m-c to graphics (because of all the extra libraries in the gkrust Cargo.lock files). Merging this by hand is difficult at best, and maybe impossible. So there are a couple of options to do this merge: 1) Before doing the merge, apply the same update (to the url library) on the graphics branch. Then merge. This should in theory result in no conflicts during the merge. However, this has the problem that there is no way to *just* update the url library when running |mach vendor rust|. Instead it pulls in updates to all the webrender-specific libraries, which may or may not be desirable[1]. This bug is requesting a way to just update the url library. 2) Do the merge, skipping conflict resolution. And then do another |mach vendor rust| on top of the merge to make everything work. Again this results in updates to webrender-specific libraries, which may or may not be desirable[1]. [1] Even though semantic versioning should mean that library updates are always ok, in practice there might be mistakes in version bumps that make it not true. For example right now on the graphics branch I seem to be getting a rust compile error after running an update and rebuilding, even though only minor version updates are happening.

After discussing with :ted on IRC, it seems that I can accomplish the desired effect by doing this: cd toolkit/library/rust/ cargo update -p url cd ../gtest/rust cargo update -p url hg commit -m "Bump url version" # comment out the line at [1] mach vendor rust [1] http://searchfox.org/mozilla-central/rev/a6a339991dbc650bab3aefe939cd0e5ac302274a/python/mozbuild/mozbuild/vendor_rust.py#82

I'm not sure what the right thing to do is here--I thought `cargo generate-lockfile` would simply resolve anything in Cargo.toml that was missing from Cargo.lock, but it sounds like it also will update anything in Cargo.lock that has a newer compatible version on crates.io? Alex: is there a better way to accomplish that? Also: I had heard it expressed elsewhere, but having a tool to merge Cargo.lock conflicts would be nice.

I think for the purposes of this bug the workflow in comment 1 is ok, we just need to add an option to |mach vendor rust| that allows skipping the generate-lockfile step.

Yes unfortunately `cargo generate-lockfile` blows away the lockfile and starts from scratch. (unsure if this should be considered a bug or not) As a workaround, you instead do something like: cargo update -p gecko where "gecko" is some crate that can't actually be updated because it's a path dependency. I typically just use the top-level project's crate as part of this. That'll ask Cargo to update *only* the `gecko` crate, but at the same time it will produce an entire resolution graph with minimal changes. Does that work for this purpose?

(In reply to Alex Crichton [:acrichto] from comment #4) > As a workaround, you instead do something like: > > cargo update -p gecko > > where "gecko" is some crate that can't actually be updated because it's a > path dependency. I typically just use the top-level project's crate as part > of this. That'll ask Cargo to update *only* the `gecko` crate, but at the > same time it will produce an entire resolution graph with minimal changes. Just to make sure I understand this: in the Gecko context, we wouldn't have to say |cargo update -p gkrust|, where gkrust is the top-level crate for Gecko's purposes, we could say |cargo update -p foo|, where foo is some crate that gkrust depends directly or transitively on? That should be very straightforward to add to |mach vendor rust|

Oh so I understood the problem is that you just want to sync Cargo.lock and Cargo.toml, not actually update anything, in that case I think the command you want is `cargo update -p gkrust`. If you execute `cargo update -p foo` where foo is a dependency then that'll actually attempt to update foo (e.g. check crates.io for a newer version). Note, though, that if foo is a path dependency (e.g. in tree, not on crates.io) then it will have the same effect as `cargo update -p gkrust`. Which is to say that updating a path dependency just syncs Cargo.lock and Cargo.toml.

^ Rebased on top of the patch at https://bugzilla.mozilla.org/show_bug.cgi?id=1277338#c26 - see the last few comments on that bug for discussion.

Comment on attachment 8820244 [details] Bug 1324462 - Don't automatically upgrade dependent libraries when running |mach vendor rust|. https://reviewboard.mozilla.org/r/99780/#review100320 LGTM; your call on implementing some of the below suggestions. ::: python/mozbuild/mozbuild/vendor_rust.py:79 (Diff revision 2) > - for crate_root in ('toolkit/library/rust/', > - 'toolkit/library/gtest/rust', > - 'js/src'): > + for (crate_root, lib) in (('toolkit/library/rust/', 'gkrust'), > + ('toolkit/library/gtest/rust', 'gkrust-gtest'), > + ('js/src', 'mozjs_sys')): As long as we're making this more complicated, WDYT about making it: ``` crates_and_roots = ( ('gkrust', 'toolkit/library/rust'), ... ('mozjs_sys', 'js/src'), ) for (lib, crate_root) in crates_and_roots: ... ``` so any additions are an actual single-line change, and the loop header is not quite so complex? ::: python/mozbuild/mozbuild/vendor_rust.py:83 (Diff revision 2) > # just do this once on the workspace root crate. > - for crate_root in ('toolkit/library/rust/', > - 'toolkit/library/gtest/rust', > - 'js/src'): > + for (crate_root, lib) in (('toolkit/library/rust/', 'gkrust'), > + ('toolkit/library/gtest/rust', 'gkrust-gtest'), > + ('js/src', 'mozjs_sys')): > path = mozpath.join(self.topsrcdir, crate_root) > - self._run_command_in_srcdir(args=[cargo, 'generate-lockfile', '--manifest-path', mozpath.join(path, 'Cargo.toml')]) > + self._run_command_in_srcdir(args=[cargo, 'update', '--manifest-path', mozpath.join(path, 'Cargo.toml'), '-p', lib]) Do you think it's worth providing a comment here about why we chose `update -p`?

Made both changes as suggested, landing updated patch.

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/bac03e62480e Don't automatically upgrade dependent libraries when running |mach vendor rust|. r=froydnj

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11) > Made both changes as suggested, landing updated patch. Thank you!