| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1117 +/- ##
=======================================
Coverage 63.06% 63.06%
=======================================
Files 50 50
Lines 8288 8288
=======================================
Hits 5227 5227
Misses 3061 3061 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
There was a problem hiding this comment.
Is this ready for review?
Sorry, something went wrong.
This is a breaking change, as Cargo is now a mandatory requirement for running a major update.
|
If you have any comments on the overall approach then fire away, but I'll mark as ready for review once the associated core PR is merged, so that people can do proper test runs. |
Sorry, something went wrong.
This wasn't actually much effort to implement, so I've added this in. New example run: nodejs/node@2c67a51 |
Sorry, something went wrong.
FWIW having the other PR merged is not necessary for doing test runs, one can apply the patch on nodejs/node already. I'd like this PR to land soon so we can release 7.0.0 which is long overdue at this point (but since this is marked semver major, it makes sense to wait for it) |
Sorry, something went wrong.
|
The upstream gyp PR has landed, so should be easy to test out on main. I've drafted an update PR at nodejs/node#64543 so that the output can be checked with a CI run on different platforms. |
Sorry, something went wrong.
|
Thank you! |
Sorry, something went wrong.
| task: async(ctx) => { | ||
| const tree = await forceRunAsync( | ||
| ctx.cargo, | ||
| ['tree', '-Z', 'bindeps', '--package', 'temporal_capi', '--prefix', 'none'], |
There was a problem hiding this comment.
| ['tree', '-Z', 'bindeps', '--package', 'temporal_capi', '--prefix', 'none'], | |
| ['+nightly', 'tree', '-Z', 'bindeps', '--package', 'temporal_capi', '--prefix', 'none'], |
Should we add this change? Without it, this command fails on my computer with:
➤ cargo tree -Z bindeps --package temporal_capi --prefix none error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks like Chromium sets RUSTC_BOOTSTRAP=1 in its toolchain, does this work with your setup?
The issue with +xyz is that it only works in a rustup environment.
Sorry, something went wrong.
There was a problem hiding this comment.
I tested with this patch and it works too:
Index: lib/update-v8/updateCrates.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/lib/update-v8/updateCrates.js b/lib/update-v8/updateCrates.js
--- a/lib/update-v8/updateCrates.js (revision c263c2ae7d9be82feb79766a89a7bdaf5527b86b)
+++ b/lib/update-v8/updateCrates.js (date 1785233230153)
@@ -37,7 +37,12 @@
{
ignoreFailure: false,
captureStdout: true,
- spawnArgs: { cwd: path.join(ctx.nodeDir, chromiumCratesDir) }
+ spawnArgs: {
+ cwd: path.join(ctx.nodeDir, chromiumCratesDir), env: {
+ ...process.env,
+ RUSTC_BOOTSTRAP: '1'
+ }
+ }
}
);
const crates = new Set();
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
An implementation of nodejs/node#64191 (comment). Depends on #1116 and nodejs/node#64482.
Broadly speaking, the process is as follows:
Example result: nodejs/node@f3bf260
Like Chromium, this approach doesn't add a rust-version to the crates manifest; Cargo will read this from the dependencies' manifests anyway. However, while it's not necessary for the build, I appreciate that it may be useful to see if our Rust requirement has changed, for updating BUILDING.md etc. If desired, we can either add this field or output the MSRV to terminal somehow; it'd just require interrogating each dependency's Cargo.toml while walking the tree.