--ignore_dev_dependency: this is an extremely clever idea. I guess the inspiration came from Node?
Glad you like it, Lukács :D> Underscore as name component separator: it looks like you settled upon using underscore as the name component separator so that the namespace of module names can be hierarchical.I don't think we made such a decision. Underscore is just like any other character allowed in module names.
> --ignore_dev_dependency: this is an extremely clever idea. I guess the inspiration came from Node?I think this was mostly modeled after Cargo's dev dependency thing (Yun can give more context)> Overrides only in the top level module: I understand why only the top level module is allowed to do overrides, but the documentation says *_override is only legal in top-level MODULE.bazel files. Have you considered ignoring *_override statements in non-top-level modules? (probably yes, but I can't find the discussion...)Ignoring them was the idea at first, but +Jon Brandvein convinced me that it's better to disallow them altogether, in the case that we eventually allow some form of overrides from transitive dependencies. It's fine to allow something that used to be disallowed, but backwards-incompatible to start giving meaning to something that used to be ignored.
> git_override: why is git special-cased as the only source control system that can supply overrides?The class of "non-registry overrides" (archive, git, local_path) can be extended fairly easily. Git was just the first one that we put there because it's the most popular one and there's already a git_repository repo rule in bazel_tools. Theoretically we can easily add something like hg_override (provided we get such a repo rule in bazel_tools first).
> module_ctx.which: why is this particular functionality of the shell exposed in the otherwise very Spartan module_ctx interface?Module extension implementation functions mostly try to run some tools to deduce what repos to generate, so `which` seemed to be a reasonable inclusion. It mostly differs from repository_ctx in that writing into the working directory is discouraged, since content there isn't addressable later on.> module_ctx.os: this looks like the same mechanism as the one repository_ctx has which predates platforms. Have you talked to the platforms folks to see if they have ideas about an API that would be more coherent with the platforms one they are busy migrating to?We haven't discussed this recently, and this is somewhat of a chicken-and-egg problem. Bzlmod technically works in a loading-interleaved phase, at which point none of the platform/toolchain resolution thing has happened yet. So it doesn't really make sense for module extension evaluation to depend on things like constraint settings and values.Last time I chatted to +Jon Brandvein about this, we decided that it may be possible to do a workspace-inside-a-workspace thing if one really cared, and left it at that.
> labels in patches=: why are these labels? The documentation says that they have to reference the top-level module so the repository part of a label doesn't make sense here and the label syntax implies that you raise an error when the ":" is at the wrong place (i.e. //a/b:c/d instead of //a:b/c/d or //a/b/c:d) which doesn't look easily implementable, does it?This is just because the underlying repo rule http_archive accepts labels for the `patches` argument. I believe the ":" thing is actually checked because we use PackageLookupFunction to find the file. That said, the usage of labels in repo rules / module extensions has always been a bit inelegant since only source file labels can be used, but we didn't deem it bad enough to address.
" YÁNG Xùdōng (杨旭东)# SURNAME Givenname= Software Engineer@ Google MunichOn Fri, Sep 23, 2022 at 1:47 PM Lukács T. Berki <lbe...@google.com> wrote:Hey Xudong,I finally managed to review the progress of bzlmod before making it generally available and I have to say, it's excellent work; I am not wise enough to tell whether it will work out as a full replacement for WORKSPACE files, but this is about the best attempt I can imagine that's humanly possible so I'll do my best to move the needle along. I especially like the effort that went into making it possible to gradually migrate from WORKSPACE files.I have a few observations, none showstoppers and quite possibly some of them are only due to my ignorance and probably quite some of them can't be changed without much sweat anyways, but it's better to discuss them now before bzlmod becomes officially supported.Underscore as name component separator: it looks like you settled upon using underscore as the name component separator so that the namespace of module names can be hierarchical. This is somewhat unfortunate because underscore is used in many places, but I guess it's too late to change now, so meh. There isn't an obviously better choice anyway.--ignore_dev_dependency: this is an extremely clever idea. I guess the inspiration came from Node?Overrides only in the top level module: I understand why only the top level module is allowed to do overrides, but the documentation says *_override is only legal in top-level MODULE.bazel files. Have you considered ignoring *_override statements in non-top-level modules? (probably yes, but I can't find the discussion...)git_override: why is git special-cased as the only source control system that can supply overrides?module_ctx.which: why is this particular functionality of the shell exposed in the otherwise very Spartan module_ctx interface? I was about to write that it's trivially implementable using execute(), but that's not the case for Windows and that may be enough of a reason to expose it?module_ctx.os: this looks like the same mechanism as the one repository_ctx has which predates platforms. Have you talked to the platforms folks to see if they have ideas about an API that would be more coherent with the platforms one they are busy migrating to?labels in patches=: why are these labels? The documentation says that they have to reference the top-level module so the repository part of a label doesn't make sense here and the label syntax implies that you raise an error when the ":" is at the wrong place (i.e. //a/b:c/d instead of //a:b/c/d or //a/b/c:d) which doesn't look easily implementable, does it?--Lukács T. Berki | Software Engineer | lbe...@google.com |Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany | Geschäftsführer: Paul Manicle, Halimah DeLaine Prado | Registergericht und -nummer: Hamburg, HRB 86891
> module_ctx.os: this looks like the same mechanism as the one repository_ctx has which predates platforms. Have you talked to the platforms folks to see if they have ideas about an API that would be more coherent with the platforms one they are busy migrating to?We haven't discussed this recently, and this is somewhat of a chicken-and-egg problem. Bzlmod technically works in a loading-interleaved phase, at which point none of the platform/toolchain resolution thing has happened yet. So it doesn't really make sense for module extension evaluation to depend on things like constraint settings and values.Last time I chatted to +Jon Brandvein about this, we decided that it may be possible to do a workspace-inside-a-workspace thing if one really cared, and left it at that.(cc John C)This looks reasonable and I totally thought that Bazel learned the set of constraints representing the platform it runs on by fetching a special repository and therefore there would be a circular dependency if I had my way. That turns out to be technically correct, but the code that determines it happens to live in Java:If Bazel determined the platform it's running on by shelling out from a repository, I'd totally agree with that assessment, but as it is, it looks simple to lift the code that generates the list of constraint labels to some place where module_ctx.os() can call it. WDYT? If we want a nice symmetric API between bzlmod and platforms, now is the easiest time.
3. Keep doing whatever we're doing with module_ctx.os and leave platforms out of it, as platforms are analysis time and modules are load time.
> Trying to channel Ivo here, I think it would be better if one required a load() statement because moving git_repository out of @bazel_tools is in line with his desire to move the Starlark rules out of the Bazel binary, too, but if he's fine with this, I'm fine with it, too.We can't remove everything from @bazel_tools (for example, http_archive is never going away; neither is the shell runfiles library)
. So it seems to me that unless the specific need arises, we shouldn't try to move git_repository out.
> If module_ctx.os() returned e.g. "@platforms//:x86_64" as a simple string or a Label, it would already be a little more coherent with the rest of Bazel than what it does now.We certainly could do that, but that'd be a departure from repository_ctx.os, and I don't want to introduce this discrepancy. Right now module_ctx and repository_ctx are much more similar to each other than to the rest of Bazel, and for good reason.
- Even the shell runfiles library is subject to the long-term "remove everything from Bazel" goal.
- There isn't a fundamental reason why http_archive() should stay there (although we might choose to do so because it's convenient)
. So it seems to me that unless the specific need arises, we shouldn't try to move git_repository out....but regardless of the answer to the above, I don't think it's worth spending time on moving git_repository out *right now* because we have bigger fish to try.
- Even the shell runfiles library is subject to the long-term "remove everything from Bazel" goal.
The shell runfiles library needs to stay in bazel_tools, because the boilerplate code to "import" the runfiles library expects a stable repo name. With Bzlmod, bazel_tools is the only one that has a stable canonical repo name.
Oof. On the surface, it looks like Python (and any other interpreted language) would also be affected by this and I suppose Python is not because it tries every top-level directory in the runfiles tree when importing?
--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CAOu%2B0LVi2bfqdEC_knjWddiwX7qpowhok%2BkQZT82qr67XUmyhA%40mail.gmail.com.