Closed Bug 1496296 Opened 6 years ago Closed 6 years ago

Stop broadly setting RUSTC_BOOTSTRAP

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(2 files)

      No description provided.
Corresponding PRs:
- https://github.com/hsivonen/simd/pull/27
- https://github.com/hsivonen/encoding_rs/pull/37
- https://github.com/hsivonen/encoding_rs/pull/36

That latter is only really required to allow vendor to work from the git
repository.

This revision is not meant to be landed as is, but to wait for releases
of simd and encoding_rs with the PRs above merged. When that happens,
We'd only update simd and encoding_rs without touching Cargo.toml.
Depends on D7679
(In reply to Mike Hommey [:glandium] from comment #1)
> Created attachment 9014246 [details]
> Bug 1496296 - Update encoding_rs and simd to versions that set
> RUSTC_BOOTSTRAP themselves
> 
> Corresponding PRs:
> - https://github.com/hsivonen/simd/pull/27
> - https://github.com/hsivonen/encoding_rs/pull/37
> - https://github.com/hsivonen/encoding_rs/pull/36
> 
> That latter is only really required to allow vendor to work from the git
> repository.
> 
> This revision is not meant to be landed as is, but to wait for releases
> of simd and encoding_rs with the PRs above merged. When that happens,
> We'd only update simd and encoding_rs without touching Cargo.toml.

As a matter of fact, using patches to point to external git repos like that fails every use of cargo on js/src/Cargo.toml and js/rust/Cargo.toml, in mozjs-crate, sm-rust and tup jobs. This may be a problem in cargo itself...
(In reply to Mike Hommey [:glandium] from comment #1)
> Created attachment 9014246 [details]
> Bug 1496296 - Update encoding_rs and simd to versions that set
> RUSTC_BOOTSTRAP themselves
> 
> Corresponding PRs:
> - https://github.com/hsivonen/simd/pull/27

I wonder how this kind of hack is going to affect the upgrade path to packed_simd. (I.e. whether packed_simd is going to accept the same hack.)

> - https://github.com/hsivonen/encoding_rs/pull/37
> - https://github.com/hsivonen/encoding_rs/pull/36
> 
> That latter is only really required to allow vendor to work from the git
> repository.

I think we shouldn't vendor the dependencies for the fuzzing code. Why did the fuzzing code itself get included in the vendoring from your git clone when it didn't get included when vendoring from crates.io?
Flags: needinfo?(mh+mozilla)
(In reply to Henri Sivonen (:hsivonen) from comment #4)
> I think we shouldn't vendor the dependencies for the fuzzing code. Why did
> the fuzzing code itself get included in the vendoring from your git clone
> when it didn't get included when vendoring from crates.io?

cargo publish probably doesn't embed it. cargo vendor does, for some reason.
Flags: needinfo?(mh+mozilla)
Attachment #9014246 - Attachment description: Bug 1496296 - Update encoding_rs and simd to versions that set RUSTC_BOOTSTRAP themselves → Bug 1496296 - Update encoding_rs to 0.8.9 and simd to 0.2.3
(In reply to Mike Hommey [:glandium] from comment #5)
> (In reply to Henri Sivonen (:hsivonen) from comment #4)
> > I think we shouldn't vendor the dependencies for the fuzzing code. Why did
> > the fuzzing code itself get included in the vendoring from your git clone
> > when it didn't get included when vendoring from crates.io?
> 
> cargo publish probably doesn't embed it. cargo vendor does, for some reason.

cargo vendor just copies whatever is in your local cargo registry, FWIW. Presumably for git dependencies cargo just clones the whole repo.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/b04cf2d0c1fd
Update encoding_rs to 0.8.9 and simd to 0.2.3 r=froydnj
https://hg.mozilla.org/integration/autoland/rev/b363fee0880d
Stop broadly setting RUSTC_BOOTSTRAP r=froydnj
https://hg.mozilla.org/mozilla-central/rev/b04cf2d0c1fd
https://hg.mozilla.org/mozilla-central/rev/b363fee0880d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.