Closed Bug 1336153 Opened 3 years ago Closed 3 years ago

Remove MOZ_RUST

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: rillian, Assigned: rillian)

References

Details

Attachments

(2 files)

Now that we're requiring rust to build we should remove the MOZ_RUST subst and define and build Rust code unconditionally.
Assignee: nobody → giles
Comment on attachment 8865997 [details]
Bug 1336153 - Remove MOZ_RUST.

https://reviewboard.mozilla.org/r/137594/#review140722

::: old-configure.in:2330
(Diff revision 1)
>  
>  if test -n "$MOZ_MULET"; then
>      AC_DEFINE(MOZ_MULET)
>  fi
>  
>  # Propagate feature switches for code written in rust from confvars.sh

Someone should really move these into moz.configure. :)

::: toolkit/library/moz.build:65
(Diff revision 1)
>  
>      if CONFIG['MOZ_NEEDS_LIBATOMIC']:
>          OS_LIBS += ['atomic']
>  
>      # This option should go away in bug 1290972, but we need to wait until
>      # Rust 1.12 has been released.

Can we remove this now?
Attachment #8865997 - Flags: review?(ted) → review+
Comment on attachment 8865997 [details]
Bug 1336153 - Remove MOZ_RUST.

https://reviewboard.mozilla.org/r/137594/#review140722

> Can we remove this now?

IIRC Nathan tried and there were still issues. I'll put it on the list to look at it.
This patch breaks --enable-artifact-builds. I think the issue is that we don't include rust.configure, so none of the RUST_* config keys are set, but the RustLibrary mozbuild class assumes they're available at initialization time (to find out where cargo will put things). At the same time, MOZ_RUST wasn't defined, so no RustLibrary objects were created in traversing the build files.

Based on IRC discussions, I'll try just not initializing those members, hoping we won't be called on to do anything that depends on them.
Comment on attachment 8866067 [details]
Bug 1336153 - Skip RustLibrary init on artifact builds.

https://reviewboard.mozilla.org/r/137660/#review140806

Fine by me, but I haven't a lot of context on how the `RustLibrary` moz.build pieces glue together.
Attachment #8866067 - Flags: review?(nalexander) → review+
Comment on attachment 8866067 [details]
Bug 1336153 - Skip RustLibrary init on artifact builds.

https://reviewboard.mozilla.org/r/137660/#review141138

Ideally, this would already be caught by the `COMPILE_ENVIRONMENT` check in emitter.py, but I don't see a good way to shoehorn this into that.

::: python/mozbuild/mozbuild/frontend/data.py:550
(Diff revision 1)
> +        # config keys won't be available, but we should be ignored
> +        # by the actual build.

"but we should be ignored..." should be phrased as "but the instance variables that we don't set should never be accessed by the actual build."  Is that a correct understanding?
Attachment #8866067 - Flags: review?(nfroyd) → review+
Comment on attachment 8866067 [details]
Bug 1336153 - Skip RustLibrary init on artifact builds.

https://reviewboard.mozilla.org/r/137660/#review141138

> "but we should be ignored..." should be phrased as "but the instance variables that we don't set should never be accessed by the actual build."  Is that a correct understanding?

That's the idea, yes. Thanks for the wording improvement.
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/875c06419125
Remove MOZ_RUST. r=ted
https://hg.mozilla.org/integration/autoland/rev/d0fccf681c20
Skip RustLibrary init on artifact builds. r=froydnj,nalexander
https://hg.mozilla.org/mozilla-central/rev/875c06419125
https://hg.mozilla.org/mozilla-central/rev/d0fccf681c20
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.