-
Notifications
You must be signed in to change notification settings - Fork 6
fix: use CARGO_CFG_TARGET_OS and target_os android #81
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
=======================================
Coverage 86.71% 86.71%
=======================================
Files 5 5
Lines 557 557
Branches 557 557
=======================================
Hits 483 483
Misses 48 48
Partials 26 26 ☔ View full report in Codecov by Sentry. |
|
While I believe this is a step forward, this still needs more work. I will take another look tomorrow. I wonder, does the usage of Recent build with this pull request: https://treeherder.mozilla.org/jobs?repo=try&revision=d606720cbe371d272648f8d80702d50416962d54&selectedTaskRun=K6LyBVWgR1iof9RZmC43rQ.0 |
|
I think |
In a `build.rs` file `cfg(target_os = "linux")` refers to the target OS of the `build.rs` binary. The environment variable `CARGO_CFG_TARGET_OS` refers to the target OS of the final binary. When cross-compiling, the target OS of the `build.rs` binary and the target OS of the final binary are not the same. The `mtu`'s `build.rs` should use `CARGO_CFG_TARGET_OS`. In addition, `cfg(target_os = "linux")` does not include `android`. This commit `cfg`s `linux` and `android` everywhere.
c9c5691 to
84ca63e
Compare
mxinden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ready for review.
In a
build.rsfilecfg(target_os = "linux")refers to the target OS of thebuild.rsbinary. The environment variableCARGO_CFG_TARGET_OSrefers to the target OS of the final binary.When cross-compiling, the target OS of the
build.rsbinary and the target OS of the final binary are not the same. Themtu'sbuild.rsshould useCARGO_CFG_TARGET_OS.In addition,
cfg(target_os = "linux")does not includeandroid. This commitcfgslinuxandandroideverywhere.Fixes https://github.com/mozilla/mtu/issues/78.