Skip to content
This repository has been archived by the owner on Feb 20, 2024. It is now read-only.

Add support for bzlmod #49

Closed
wants to merge 6 commits into from
Closed

Conversation

agluszak
Copy link
Contributor

@agluszak agluszak commented Oct 13, 2023

Closes #37

TODO:

  • add some tests to make sure bzlmod and workspace versions work the same way?
  • merge format/repositories.bzl and non_module_deps.bzl?

Comment on lines -2 to +3
w:: "World",
"Hello": $.w
w:: 'World',
Hello: $.w,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These examples have been formatted accidentally, but this makes me think: what's the role of example files?

  1. Are they examples of the output of formatters? (but some files are formatted and some are not)
  2. Are they examples of input files to the formatters?

In either case I think we could turn them into diff tests, i.e. add pairs (unformatted file, expected output) for each formatter and test whether running bazel-super-formatter with all flags turned on transforms (unformatted file) into (expected output). Of course this will have a side effect of depending on specific formatting behavior exhibited by a given formatter, but in general what we want to test is whether the formatter is run at all by our meta-formatter

)
use_repo(go, "go_sdk")

non_module_deps = use_extension("//:non_module_deps.bzl", "non_module_deps")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we can start using jvm_rules_external at least in the bzlmod version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another side effect of running bazel-super-formatter on its own repo with all flags turned on


go = use_extension("@io_bazel_rules_go//go:extensions.bzl", "go_sdk")
go.download(
name = "go_sdk",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line causes an error when used from another repo:
Error in fail: go_sdk.download: name must not be specified in non-root module aspect_rules_format

pip.parse(
hub_name = "pip",
python_version = "3.11",
requirements_lock = "//:requirements_lock.txt",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does a user customise the version of black that is installed with this approach? In some cases typing_extensions also needs installing for black to work correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repo is the formatter, and we always wanted the linter. I have started one.

In rules_lint, the answer is "we don't try to provide the toolchain or the tools, you must provide them".

For example, here's how we get flake8:
https://github.com/aspect-build/rules_lint/blob/main/example/lint.bzl#L18

If a user is formatting code in a certain language, they probably already have the toolchain for that language. It wouldn't be too much effort for them to add three lines to a BUILD file to declare the black binary for us.

This would help with the error above about Go as well. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I'm doing some prototyping over here aspect-build/rules_lint#14

So far it looks promising

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay it's more than a prototype. Pretty sure that's the right way to do this with bzlmod, and this repo can just be the WORKSPACE way. Would love your input @agluszak


python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(
is_default = True,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this MODULE file should be setting the default python toolchain?

@alexeagle
Copy link
Member

Thanks for the PR that motivated rules_lint! Time to archive this repo.

@alexeagle alexeagle closed this Feb 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FR]: bzlmod support?
3 participants