-
Notifications
You must be signed in to change notification settings - Fork 92
Update the non-toolchain examples to bzlmod #684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
a8ce95a to
164b2a0
Compare
164b2a0 to
eb822fa
Compare
layus
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.
Such an impressive rework. Not much code, but so many subtle edits. Thanks !
I am a bit concerned about the use of deprecated constructs, and the reason why we diverge from common conventions. Not a big worry, but an explanation in some readme would help, or next to the changes.
A bigger issue is that we impose all our users or rules_nixpkgs_cc to configure their own module extension. Is that by design ? Or a shortcut taken in these examples ?
| # Toolchains | ||
| # | ||
|
|
||
| cc_configure = use_extension("//:extension.bzl", "cc_configure") |
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.
Do we absolutely have to define our own module extension to use cc toolchains ?
I feel like that extensions pertains to rules_nixpkgs_cc itself, with tags for the config. But maybe it is not implemented yet ?
| import %workspace%/../../.bazelrc.remote-cache | ||
|
|
||
| build --host_platform=@rules_nixpkgs_core//platforms:host | ||
| build --crosstool_top=@nixpkgs_config_cc//:toolchain |
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.
Having crosstool_top used seems weird. Should it not be selected by default by the toolchain selection engine ?
Or is this intended as a demo on how to force a specific toolchain ?
I feel like crosstool_top is on the verge of deprecation.
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.
Is this intended ? Do we want this version to diverge from ../../.bazeliskrc ?
| @@ -1,2 +1,3 @@ | |||
| bazel-* | |||
| result | |||
| MODULE.bazel.lock | |||
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.
Again, why is this repo different ? Is this related to python 7 and any run altering that lockfile ?
Uh oh!
There was an error while loading. Please reload this page.