Re: Editing vendored crates take #2

[Boris Zbarsky]

On 5/2/17 2:54 PM, Josh Matthews wrote:

My cargo from April 19 claims that "cargo vendor" isn't a real command.
Did you mean `./mach vendor rust`


Er, yes, I did.

-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates take #2

[Ted Mielczarek]

On Tue, May 2, 2017, at 02:54 PM, Josh Matthews wrote:
> On 2017-04-28 3:07 PM, Boris Zbarsky wrote:
> > On 4/28/17 1:05 PM, Josh Matthews wrote:
> > 2)  Run "cargo vendor" and watch it fail because of something I never
> > figured out.
> 
> My cargo from April 19 claims that "cargo vendor" isn't a real command. 
> Did you mean `./mach vendor rust` (which did end up deleting the whole 
> directory for me)?

FYI you need to `cargo install cargo-vendor` for `cargo vendor` to work.
`./mach vendor rust` will do that for you if it's not installed.

-Ted
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Bobby Holley]

On Sun, Feb 26, 2017 at 2:10 PM, Bobby Holley  wrote:

> On Sun, Feb 26, 2017 at 9:51 AM, Henri Sivonen 
> wrote:
>
>> I tried to add some panics to a vendored to create (rust-encoding) to
>> see if the code in question runs. However, I didn't get to the running
>> part, because the edited code failed to build.
>>
>> It turns out that each vendored crate has a .cargo-checksum.json file
>> that contains hashes of all the files in the crate, and Cargo refuses
>> to build the crate if the hashes don't match or the
>> .cargo-checksum.json file is removed.
>>
>> This seems hostile not only to experimental local edits as in my case
>> but also to use cases such as uplifting fixes to branches and shipping
>> modified crates if the upstream is slow to accept patches or disagrees
>> with the patches.
>>
>> As far as I can tell, there doesn't seem to be a way to ask cargo to
>> regenerate the .cargo-checksum.json after edits. Also, adding a
>> [replace] section to Cargo.toml of libgkrust pointing to the edited
>> crate under third-party/rust or adding  paths = [ "third-party/rust" ]
>> to .cargo/config.in don't make Cargo happy.
>>
>
> Can you elaborate on what goes wrong here? This worked for ted's
> experiment mentioned upthread, and for me on at least one occasion in
> https://hg.mozilla.org/try/rev/18dc070e0308 (permalink:
> https://pastebin.mozilla.org/8980438 )
>
>
And because pastebin seems to lie about stuff being permanent:
https://gist.github.com/bholley/85dc2277aaa732bce04cc9e672915595



> You'll need to |cargo update| after adding the replace in Cargo.toml to
> update Cargo.lock.
>
>
>>
>> What's the right way to build with edited crates under
>> third-party/rust? (I think we should have an easy way to do so.)
>>
>> --
>> Henri Sivonen
>> hsivo...@hsivonen.fi
>> https://hsivonen.fi/
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates take #2

[Kartikaya Gupta]

At one point the steps at
https://wiki.mozilla.org/Platform/GFX/Quantum_Render#Testing_third-party_rust_library_changes
were known to work. I don't know if they have since been broken.

On Fri, Apr 28, 2017 at 3:07 PM, Boris Zbarsky  wrote:
> On 4/28/17 1:05 PM, Josh Matthews wrote:
>>
>> Has anybody been able to make this work?
>
>
> I _think_ I made it work recently-ish, like so:
>
> 1)  Modify toolkit/library/rust/Cargo.toml with the relevant [replace] bit.
>
> 2)  Run "cargo vendor" and watch it fail because of something I never
> figured out.
>
> 3)  revert the changes "cargo vendor" made to third-party/rust (i.e.
> deleting the whole dir)
>
> As it happens, "cargo vendor" updated the Cargo.lock files in the right way
> before it failed out, so things worked.  Again, I _think_ that was the
> procedure.
>
> I'm not claiming this is in any way sane, btw.
>
> -Boris
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates take #2

[Boris Zbarsky]

On 4/28/17 1:05 PM, Josh Matthews wrote:

Has anybody been able to make this work?


I _think_ I made it work recently-ish, like so:

1)  Modify toolkit/library/rust/Cargo.toml with the relevant [replace] bit.

2)  Run "cargo vendor" and watch it fail because of something I never 
figured out.


3)  revert the changes "cargo vendor" made to third-party/rust (i.e. 
deleting the whole dir)


As it happens, "cargo vendor" updated the Cargo.lock files in the right 
way before it failed out, so things worked.  Again, I _think_ that was 
the procedure.


I'm not claiming this is in any way sane, btw.

-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates take #2

[Josh Matthews]

I just tried moving my changes into third_part/rust/rust-cssparser. I 
also needed to modify the Cargo.toml there to update the paths to 
cssparser-macros and procedural-masquerade because they are vendored as 
separate crates, rather than in-library dependencies. At this point I 
was still hitting the problem with `cargo update -p cssparser` always 
updating to the newest version on crates.io, so I tried changing the 
version in rust-cssparser/Cargo.toml to 0.12.4 to match. At this point 
I'm stuck at this error:


 0:05.90 error: checksum for `cssparser v0.12.4` changed between lock files
 0:05.90
 0:05.90 this could be indicative of a few possible errors:
 0:05.90
 0:05.90 * the lock file is corrupt
 0:05.90 * a replacement source in use (e.g. a mirror) returned a 
different checksum

 0:05.90 * the source itself may be corrupt in one way or another
 0:05.90
 0:05.90 unable to verify that `cssparser v0.12.4` is the same as when 
the lockfile was generated


On 2017-04-28 1:28 PM, Bobby Holley wrote:

I definitely made this work at some point:
https://hg.mozilla.org/try/rev/18dc070e0308

The main difference with what you seem to be doing is that my version
points directly into third-party/rust, which I think is preferable anyway.

On Fri, Apr 28, 2017 at 10:05 AM, Josh Matthews 
wrote:


Back in February there was a discussion about how hard it is to modify
vendored Rust crates for Stylo. I can't actually find any solution to that
problem - the general idea appears to be "add a [replace] section to a
Cargo.toml and run cargo update", but I have not been able to make that
work yet. Given that the version of mozilla-central that I'm using contains
a vendored copy of rust-cssparser 0.12.2, here's what I have tried:

* added the following to servo/Cargo.toml and toolkit/library/rust:
[replace]
"rust-cssparser:0.12.2" = { path = "/Users/jdm/src/rust-cssparser" }

* run `cd servo/Cargo.toml && cargo update -p cssparser`
* run `cd toolkit/library/rust && cargo update -p cssparser`

This ends up modifying the Cargo.lock files to use the most recent version
from crates.io (0.12.4) instead of my local copy. Has anybody been able
to make this work? Can we please document the exact steps to follow? There
have been multiple people asking in #servo how to do this recently and
nobody seems to have done it successfully.

Cheers,
Josh
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates take #2

[Jack Moffitt]

There is this bug which the Cargo team is planning to work on to help
with these problems, but it hasn't been implemented yet that I know
of.

https://github.com/rust-lang/cargo/issues/2649#issuecomment-279615299

jack.

On Fri, Apr 28, 2017 at 11:28 AM, Bobby Holley  wrote:
> I definitely made this work at some point:
> https://hg.mozilla.org/try/rev/18dc070e0308
>
> The main difference with what you seem to be doing is that my version
> points directly into third-party/rust, which I think is preferable anyway.
>
> On Fri, Apr 28, 2017 at 10:05 AM, Josh Matthews 
> wrote:
>
>> Back in February there was a discussion about how hard it is to modify
>> vendored Rust crates for Stylo. I can't actually find any solution to that
>> problem - the general idea appears to be "add a [replace] section to a
>> Cargo.toml and run cargo update", but I have not been able to make that
>> work yet. Given that the version of mozilla-central that I'm using contains
>> a vendored copy of rust-cssparser 0.12.2, here's what I have tried:
>>
>> * added the following to servo/Cargo.toml and toolkit/library/rust:
>> [replace]
>> "rust-cssparser:0.12.2" = { path = "/Users/jdm/src/rust-cssparser" }
>>
>> * run `cd servo/Cargo.toml && cargo update -p cssparser`
>> * run `cd toolkit/library/rust && cargo update -p cssparser`
>>
>> This ends up modifying the Cargo.lock files to use the most recent version
>> from crates.io (0.12.4) instead of my local copy. Has anybody been able
>> to make this work? Can we please document the exact steps to follow? There
>> have been multiple people asking in #servo how to do this recently and
>> nobody seems to have done it successfully.
>>
>> Cheers,
>> Josh
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates take #2

[Bobby Holley]

I definitely made this work at some point:
https://hg.mozilla.org/try/rev/18dc070e0308

The main difference with what you seem to be doing is that my version
points directly into third-party/rust, which I think is preferable anyway.

On Fri, Apr 28, 2017 at 10:05 AM, Josh Matthews 
wrote:

> Back in February there was a discussion about how hard it is to modify
> vendored Rust crates for Stylo. I can't actually find any solution to that
> problem - the general idea appears to be "add a [replace] section to a
> Cargo.toml and run cargo update", but I have not been able to make that
> work yet. Given that the version of mozilla-central that I'm using contains
> a vendored copy of rust-cssparser 0.12.2, here's what I have tried:
>
> * added the following to servo/Cargo.toml and toolkit/library/rust:
> [replace]
> "rust-cssparser:0.12.2" = { path = "/Users/jdm/src/rust-cssparser" }
>
> * run `cd servo/Cargo.toml && cargo update -p cssparser`
> * run `cd toolkit/library/rust && cargo update -p cssparser`
>
> This ends up modifying the Cargo.lock files to use the most recent version
> from crates.io (0.12.4) instead of my local copy. Has anybody been able
> to make this work? Can we please document the exact steps to follow? There
> have been multiple people asking in #servo how to do this recently and
> nobody seems to have done it successfully.
>
> Cheers,
> Josh
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Editing vendored crates take #2

[Josh Matthews]

Back in February there was a discussion about how hard it is to modify 
vendored Rust crates for Stylo. I can't actually find any solution to 
that problem - the general idea appears to be "add a [replace] section 
to a Cargo.toml and run cargo update", but I have not been able to make 
that work yet. Given that the version of mozilla-central that I'm using 
contains a vendored copy of rust-cssparser 0.12.2, here's what I have tried:


* added the following to servo/Cargo.toml and toolkit/library/rust:
[replace]
"rust-cssparser:0.12.2" = { path = "/Users/jdm/src/rust-cssparser" }

* run `cd servo/Cargo.toml && cargo update -p cssparser`
* run `cd toolkit/library/rust && cargo update -p cssparser`

This ends up modifying the Cargo.lock files to use the most recent 
version from crates.io (0.12.4) instead of my local copy. Has anybody 
been able to make this work? Can we please document the exact steps to 
follow? There have been multiple people asking in #servo how to do this 
recently and nobody seems to have done it successfully.


Cheers,
Josh
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Simon Sapin]

On 27/02/17 19:47, Simon Sapin wrote:

On 27/02/17 19:30, Henri Sivonen wrote:

On Mon, Feb 27, 2017 at 8:00 PM, Bobby Holley  wrote:

FWIW, |cargo tree| is a very helpful tool to figure out who's pulling in a
crate.


Thanks, but what I'm trying to figure out isn't whose pulling it in
(the style crate is) but whether it is actually used beyond an
always-None Option in a way that would result in the data
tables actually getting included in the build as oppose to (hopefully)
getting discarded by LTO.

(Motivation: If we are taking on the data tables, I want to argue that
we should include encoding_rs instead even before the "replace uconv
with encoding_rs" step is done.)


As an aside, I have a plan to remove rust-encoding entirely from Servo
(including Stylo) and use encoding_rs instead. But doing this the way I
want to has a number of prerequisites, and I’d prefer to switch
everything at once to avoid having both in the build. At the moment I’m
prioritizing other Stylo-related work, but I’m confident it’ll happen
before we start shipping Stylo.


Henri, you’re correct that the parts of the style crate using 
rust-encoding are never used in Stylo. Gecko always passes UTF-8 input 
(after decoding on the C++ side I assume) to Stylo for parsing 
stylesheets, which we unsafely view as  with 
std::str::from_utf8_unchecked.


I’ve submitted https://github.com/servo/servo/pull/15993 to remove the 
rust-encoding dependency from stylo. This doesn’t need to wait on 
switching the rest of Servo.


--
Simon Sapin
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Henri Sivonen]

On Mon, Feb 27, 2017 at 8:47 PM, Simon Sapin  wrote:
> As an aside, I have a plan to remove rust-encoding entirely from Servo
> (including Stylo) and use encoding_rs instead. But doing this the way I want
> to has a number of prerequisites, and I’d prefer to switch everything at
> once to avoid having both in the build. At the moment I’m prioritizing other
> Stylo-related work, but I’m confident it’ll happen before we start shipping
> Stylo.

Nice! Works for me. :-)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Simon Sapin]

On 27/02/17 19:30, Henri Sivonen wrote:

On Mon, Feb 27, 2017 at 8:00 PM, Bobby Holley  wrote:

FWIW, |cargo tree| is a very helpful tool to figure out who's pulling in a
crate.


Thanks, but what I'm trying to figure out isn't whose pulling it in
(the style crate is) but whether it is actually used beyond an
always-None Option in a way that would result in the data
tables actually getting included in the build as oppose to (hopefully)
getting discarded by LTO.

(Motivation: If we are taking on the data tables, I want to argue that
we should include encoding_rs instead even before the "replace uconv
with encoding_rs" step is done.)


As an aside, I have a plan to remove rust-encoding entirely from Servo 
(including Stylo) and use encoding_rs instead. But doing this the way I 
want to has a number of prerequisites, and I’d prefer to switch 
everything at once to avoid having both in the build. At the moment I’m 
prioritizing other Stylo-related work, but I’m confident it’ll happen 
before we start shipping Stylo.


--
Simon Sapin
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Henri Sivonen]

On Mon, Feb 27, 2017 at 8:00 PM, Bobby Holley  wrote:
> FWIW, |cargo tree| is a very helpful tool to figure out who's pulling in a
> crate.

Thanks, but what I'm trying to figure out isn't whose pulling it in
(the style crate is) but whether it is actually used beyond an
always-None Option in a way that would result in the data
tables actually getting included in the build as oppose to (hopefully)
getting discarded by LTO.

(Motivation: If we are taking on the data tables, I want to argue that
we should include encoding_rs instead even before the "replace uconv
with encoding_rs" step is done.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Bobby Holley]

On Mon, Feb 27, 2017 at 9:56 AM, Henri Sivonen  wrote:

> On Mon, Feb 27, 2017 at 7:47 PM, Ted Mielczarek 
> wrote:
> > On Mon, Feb 27, 2017, at 12:32 PM, Henri Sivonen wrote:
> >> We don't seem to need such change control beyond hg logs for e.g. the
> >> in-tree ICU or Skia, though.
> >
> > As someone who has maintained a vendored upstream C++ project (Breakpad)
> > for a decade, I can say that this causes us headaches *all the time*.
>
> OK.
>
> > I'm sorry this is causing you pain, and we should figure out a way to
> > make it less painful, but note that the intention is that things in
> > `third_party/rust` should be actual third-party code, not things under
> > active development by Mozilla. We don't currently have a great middle
> > ground between "mozilla-central is the repository of record for this
> > crate" and "this crate is vendored from crates.io". We're finding our
> > way there with Servo, so we might have a better story for things like
> > encoding-rs when we get that working well.
>
> Note that my problem at hand isn't with encoding_rs but with the
> currently-vendored rust-encoding. That is, I indeed would like to add
> a diagnostic panic!()/println!() to genuinely third-party code--not to
> code I've written.
>

To be clear, I totally agree that local debugging-oriented edits to
vendored crates should be painless. A 1-line mach command to add a cargo
[replace] and update Cargo.lock (with a helpful message if somebody forgets
to do that) seems like the right way to do that.

>
> That is, I'd like to experimentally understand what, if anything,
> rust-encoding is currently used for. (My best current hypothesis from
> reading things on SearchFox is that it's used in order to be able to
> compile one Option that's always None in Gecko.)
>

FWIW, |cargo tree| is a very helpful tool to figure out who's pulling in a
crate.


>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Henri Sivonen]

On Mon, Feb 27, 2017 at 7:47 PM, Ted Mielczarek  wrote:
> On Mon, Feb 27, 2017, at 12:32 PM, Henri Sivonen wrote:
>> We don't seem to need such change control beyond hg logs for e.g. the
>> in-tree ICU or Skia, though.
>
> As someone who has maintained a vendored upstream C++ project (Breakpad)
> for a decade, I can say that this causes us headaches *all the time*.

OK.

> I'm sorry this is causing you pain, and we should figure out a way to
> make it less painful, but note that the intention is that things in
> `third_party/rust` should be actual third-party code, not things under
> active development by Mozilla. We don't currently have a great middle
> ground between "mozilla-central is the repository of record for this
> crate" and "this crate is vendored from crates.io". We're finding our
> way there with Servo, so we might have a better story for things like
> encoding-rs when we get that working well.

Note that my problem at hand isn't with encoding_rs but with the
currently-vendored rust-encoding. That is, I indeed would like to add
a diagnostic panic!()/println!() to genuinely third-party code--not to
code I've written.

That is, I'd like to experimentally understand what, if anything,
rust-encoding is currently used for. (My best current hypothesis from
reading things on SearchFox is that it's used in order to be able to
compile one Option that's always None in Gecko.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Ted Mielczarek]

On Mon, Feb 27, 2017, at 12:32 PM, Henri Sivonen wrote:
> On Mon, Feb 27, 2017 at 7:04 PM, Ralph Giles  wrote:
> > On Mon, Feb 27, 2017 at 4:03 AM, Henri Sivonen  wrote:
> >
> >> I find this level of difficulty (self-inflicted quasi-Tivoization
> >> practically) an unreasonable impediment to practicing trivial Software
> >> Freedom with respect to the vendored crates.
> >
> > I agree we need to fix the ergonomics here, but I don't think you
> > should be so hard on cargo.
> 
> Sorry about the tone. I'm rather frustrated at how hard it is to do
> something that should be absolutely trivial (adding a local diagnostic
> panic!()/println!()).
> 
> > The hash checking is designed to make
> > builds more reproducible, so that unless we make an explicit diversion
> > we know we're building with the same source as every other use of that
> > same package version. This has benefits for security, debugging, and
> > change control.
> 
> We don't seem to need such change control beyond hg logs for e.g. the
> in-tree ICU or Skia, though.

As someone who has maintained a vendored upstream C++ project (Breakpad)
for a decade, I can say that this causes us headaches *all the time*. We
are constantly landing local changes to vendored projects and not
keeping track of them and then either losing patches or dealing with
conflicts or breakage when we update from upstream.

I'm sorry this is causing you pain, and we should figure out a way to
make it less painful, but note that the intention is that things in
`third_party/rust` should be actual third-party code, not things under
active development by Mozilla. We don't currently have a great middle
ground between "mozilla-central is the repository of record for this
crate" and "this crate is vendored from crates.io". We're finding our
way there with Servo, so we might have a better story for things like
encoding-rs when we get that working well. I understand that there are
lots of benefits to developing a crate in a standalone GitHub
repository, and you're certainly not the only one who wants to do that,
but it does make the integration story harder. It's very hard to support
code that has its repository of record somewhere other than
mozilla-central, but also have a simple workflow for making changes to
that code in mozilla-central along with other code.

-Ted
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Henri Sivonen]

On Mon, Feb 27, 2017 at 7:04 PM, Ralph Giles  wrote:
> On Mon, Feb 27, 2017 at 4:03 AM, Henri Sivonen  wrote:
>
>> I find this level of difficulty (self-inflicted quasi-Tivoization
>> practically) an unreasonable impediment to practicing trivial Software
>> Freedom with respect to the vendored crates.
>
> I agree we need to fix the ergonomics here, but I don't think you
> should be so hard on cargo.

Sorry about the tone. I'm rather frustrated at how hard it is to do
something that should be absolutely trivial (adding a local diagnostic
panic!()/println!()).

> The hash checking is designed to make
> builds more reproducible, so that unless we make an explicit diversion
> we know we're building with the same source as every other use of that
> same package version. This has benefits for security, debugging, and
> change control.

We don't seem to need such change control beyond hg logs for e.g. the
in-tree ICU or Skia, though.

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Ralph Giles]

On Mon, Feb 27, 2017 at 4:03 AM, Henri Sivonen  wrote:

> I find this level of difficulty (self-inflicted quasi-Tivoization
> practically) an unreasonable impediment to practicing trivial Software
> Freedom with respect to the vendored crates.

I agree we need to fix the ergonomics here, but I don't think you
should be so hard on cargo. The hash checking is designed to make
builds more reproducible, so that unless we make an explicit diversion
we know we're building with the same source as every other use of that
same package version. This has benefits for security, debugging, and
change control.

 -r
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Henri Sivonen]

On Mon, Feb 27, 2017 at 12:10 AM, Bobby Holley  wrote:
> Can you elaborate on what goes wrong here? This worked for ted's experiment
> mentioned upthread, and for me on at least one occasion in
> https://hg.mozilla.org/try/rev/18dc070e0308 (permalink:
> https://pastebin.mozilla.org/8980438 )
>
> You'll need to |cargo update| after adding the replace in Cargo.toml to
> update Cargo.lock.

Thanks. I failed to do that yesterday.

When I do that, Cargo complains about not finding the
encoding-index-foo crates in subdirectories of encoding. Replacement
in gkrust's Cargo.toml doesn't work. So then I go and edit encoding's
Cargo.toml to point it to the right place. Then Cargo complains about
those crates not finding encoding_index_tests. So then I edit their
Cargo.tomls to point to the test crate.

Then cargo update passes.

But then I do ./mach build and Cargo complains about the checksums not
matching because I edited the Cargo.tomls under the crates that I
thought I was changing from "locally-stored crates.io crate" status to
"local replacement" status.

The I remove the checksum file.

The cargo complains about not finding the checksum file.

I find this level of difficulty (self-inflicted quasi-Tivoization
practically) an unreasonable impediment to practicing trivial Software
Freedom with respect to the vendored crates.

> This is basically the right way to do it, rather than editing the checksums. 
> [replace] tells the Cargo machinery that the vendored code is its own 
> snowflake, rather than just a cache of what's on crates.io. Doing otherwise 
> breaks cargo invariants.

What are the invariants? Why do we need the invariants, when can do
without impediments like these for e.g. the vendored copy of ICU?

What badness would arise from patching Cargo to ignore the
.cargo-checksum.json files?

On Mon, Feb 27, 2017 at 1:23 AM, Xidorn Quan  wrote:
> This workflow should really be automated via a mach command. Filed bug
> 1342815 [1] for that.
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1342815

Thank you. I think it should be possible to stick a diagnostic
println!() or panic!() in the vendored code with zero or minimal
ceremony.

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Xidorn Quan]

On Mon, Feb 27, 2017, at 09:10 AM, Bobby Holley wrote:
> On Sun, Feb 26, 2017 at 9:51 AM, Henri Sivonen 
> wrote:
> 
> > I tried to add some panics to a vendored to create (rust-encoding) to
> > see if the code in question runs. However, I didn't get to the running
> > part, because the edited code failed to build.
> >
> > It turns out that each vendored crate has a .cargo-checksum.json file
> > that contains hashes of all the files in the crate, and Cargo refuses
> > to build the crate if the hashes don't match or the
> > .cargo-checksum.json file is removed.
> >
> > This seems hostile not only to experimental local edits as in my case
> > but also to use cases such as uplifting fixes to branches and shipping
> > modified crates if the upstream is slow to accept patches or disagrees
> > with the patches.
> >
> > As far as I can tell, there doesn't seem to be a way to ask cargo to
> > regenerate the .cargo-checksum.json after edits. Also, adding a
> > [replace] section to Cargo.toml of libgkrust pointing to the edited
> > crate under third-party/rust or adding  paths = [ "third-party/rust" ]
> > to .cargo/config.in don't make Cargo happy.
> >
> 
> Can you elaborate on what goes wrong here? This worked for ted's
> experiment
> mentioned upthread, and for me on at least one occasion in
> https://hg.mozilla.org/try/rev/18dc070e0308 (permalink:
> https://pastebin.mozilla.org/8980438 )
> 
> You'll need to |cargo update| after adding the replace in Cargo.toml to
> update Cargo.lock.

This workflow should really be automated via a mach command. Filed bug
1342815 [1] for that.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1342815

- Xidorn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Bobby Holley]

On Sun, Feb 26, 2017 at 9:51 AM, Henri Sivonen  wrote:

> I tried to add some panics to a vendored to create (rust-encoding) to
> see if the code in question runs. However, I didn't get to the running
> part, because the edited code failed to build.
>
> It turns out that each vendored crate has a .cargo-checksum.json file
> that contains hashes of all the files in the crate, and Cargo refuses
> to build the crate if the hashes don't match or the
> .cargo-checksum.json file is removed.
>
> This seems hostile not only to experimental local edits as in my case
> but also to use cases such as uplifting fixes to branches and shipping
> modified crates if the upstream is slow to accept patches or disagrees
> with the patches.
>
> As far as I can tell, there doesn't seem to be a way to ask cargo to
> regenerate the .cargo-checksum.json after edits. Also, adding a
> [replace] section to Cargo.toml of libgkrust pointing to the edited
> crate under third-party/rust or adding  paths = [ "third-party/rust" ]
> to .cargo/config.in don't make Cargo happy.
>

Can you elaborate on what goes wrong here? This worked for ted's experiment
mentioned upthread, and for me on at least one occasion in
https://hg.mozilla.org/try/rev/18dc070e0308 (permalink:
https://pastebin.mozilla.org/8980438 )

You'll need to |cargo update| after adding the replace in Cargo.toml to
update Cargo.lock.


>
> What's the right way to build with edited crates under
> third-party/rust? (I think we should have an easy way to do so.)
>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Bobby Holley]

On Sun, Feb 26, 2017 at 2:02 PM, Manish Goregaokar 
wrote:

> > [replace] section to Cargo.toml of libgkrust pointing to the edited
> > crate under third-party/rust or adding  paths = [ "third-party/rust" ]
> > to .cargo/config.in don't make Cargo happy.
>
> [replace] is the way to do it, but it has to point to a crate outside of
> the vendoring directory.
>

That is not true - you can use [replace] to edit the vendored code:
https://bugzilla.mozilla.org/show_bug.cgi?id=1323557#c3

This is basically the right way to do it, rather than editing the
checksums. [replace] tells the Cargo machinery that the vendored code is
its own snowflake, rather than just a cache of what's on crates.io. Doing
otherwise breaks cargo invariants.

bholley


> The vendor directory is like ~/.cargo; it's not supposed to be edited by
> anything other than the tool.
>
> -Manish Goregaokar
>
> On Sun, Feb 26, 2017 at 9:51 AM, Henri Sivonen 
> wrote:
>
> > I tried to add some panics to a vendored to create (rust-encoding) to
> > see if the code in question runs. However, I didn't get to the running
> > part, because the edited code failed to build.
> >
> > It turns out that each vendored crate has a .cargo-checksum.json file
> > that contains hashes of all the files in the crate, and Cargo refuses
> > to build the crate if the hashes don't match or the
> > .cargo-checksum.json file is removed.
> >
> > This seems hostile not only to experimental local edits as in my case
> > but also to use cases such as uplifting fixes to branches and shipping
> > modified crates if the upstream is slow to accept patches or disagrees
> > with the patches.
> >
> > As far as I can tell, there doesn't seem to be a way to ask cargo to
> > regenerate the .cargo-checksum.json after edits. Also, adding a
> > [replace] section to Cargo.toml of libgkrust pointing to the edited
> > crate under third-party/rust or adding  paths = [ "third-party/rust" ]
> > to .cargo/config.in don't make Cargo happy.
> >
> > What's the right way to build with edited crates under
> > third-party/rust? (I think we should have an easy way to do so.)
> >
> > --
> > Henri Sivonen
> > hsivo...@hsivonen.fi
> > https://hsivonen.fi/
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editing vendored crates

[Manish Goregaokar]

> [replace] section to Cargo.toml of libgkrust pointing to the edited
> crate under third-party/rust or adding  paths = [ "third-party/rust" ]
> to .cargo/config.in don't make Cargo happy.

[replace] is the way to do it, but it has to point to a crate outside of
the vendoring directory.

The vendor directory is like ~/.cargo; it's not supposed to be edited by
anything other than the tool.

-Manish Goregaokar

On Sun, Feb 26, 2017 at 9:51 AM, Henri Sivonen  wrote:

> I tried to add some panics to a vendored to create (rust-encoding) to
> see if the code in question runs. However, I didn't get to the running
> part, because the edited code failed to build.
>
> It turns out that each vendored crate has a .cargo-checksum.json file
> that contains hashes of all the files in the crate, and Cargo refuses
> to build the crate if the hashes don't match or the
> .cargo-checksum.json file is removed.
>
> This seems hostile not only to experimental local edits as in my case
> but also to use cases such as uplifting fixes to branches and shipping
> modified crates if the upstream is slow to accept patches or disagrees
> with the patches.
>
> As far as I can tell, there doesn't seem to be a way to ask cargo to
> regenerate the .cargo-checksum.json after edits. Also, adding a
> [replace] section to Cargo.toml of libgkrust pointing to the edited
> crate under third-party/rust or adding  paths = [ "third-party/rust" ]
> to .cargo/config.in don't make Cargo happy.
>
> What's the right way to build with edited crates under
> third-party/rust? (I think we should have an easy way to do so.)
>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Editing vendored crates

[Henri Sivonen]

I tried to add some panics to a vendored to create (rust-encoding) to
see if the code in question runs. However, I didn't get to the running
part, because the edited code failed to build.

It turns out that each vendored crate has a .cargo-checksum.json file
that contains hashes of all the files in the crate, and Cargo refuses
to build the crate if the hashes don't match or the
.cargo-checksum.json file is removed.

This seems hostile not only to experimental local edits as in my case
but also to use cases such as uplifting fixes to branches and shipping
modified crates if the upstream is slow to accept patches or disagrees
with the patches.

As far as I can tell, there doesn't seem to be a way to ask cargo to
regenerate the .cargo-checksum.json after edits. Also, adding a
[replace] section to Cargo.toml of libgkrust pointing to the edited
crate under third-party/rust or adding  paths = [ "third-party/rust" ]
to .cargo/config.in don't make Cargo happy.

What's the right way to build with edited crates under
third-party/rust? (I think we should have an easy way to do so.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform