Skip to content

Add Starlark/Buck2 related extensions #591

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.

Sign up for GitHub

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

Closed
lf- wants to merge 1 commit into from
+6 −2
Closed

Add Starlark/Buck2 related extensions #591

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions identify/extensions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
'bmp': {'binary', 'image', 'bitmap'},
'bz2': {'binary', 'bzip2'},
'bz3': {'binary', 'bzip3'},
'bzl': {'text', 'bazel'},
'bxl': {'text', 'bazel'},

@asottile asottile Sep 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

prior to this, tools used bazel to target starlark-like files -- I'd be afraid this would be an incompatible change of the file type

@lf- lf- Sep 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

is this not a set of things? ie, this is just adding things that are matched? anyway i do see that i made a mistake where bxl should have been bazel.

'bzl': {'text', 'bazel', 'starlark'},
'c': {'text', 'c'},
'c++': {'text', 'c++'},
'c++m': {'text', 'c++'},
Expand Down Expand Up @@ -247,6 +248,7 @@
'scm': {'text', 'scheme'},
'scss': {'text', 'scss'},
'sh': {'text', 'shell'},
'sky': {'text', 'starlark'},
'sln': {'text', 'sln'},
'slnx': {'text', 'xml', 'slnx', 'msbuild'},
'sls': {'text', 'salt'},
Expand All @@ -255,6 +257,7 @@
'spec': {'text', 'spec'},
'sql': {'text', 'sql'},
'ss': {'text', 'scheme'},
'star': {'text', 'starlark'},
'sty': {'text', 'tex'},
'styl': {'text', 'stylus'},
'sv': {'text', 'system-verilog'},
Expand Down Expand Up @@ -390,6 +393,7 @@
'bblayers.conf': EXTENSIONS['bb'],
'bitbake.conf': EXTENSIONS['bb'],
'Brewfile': EXTENSIONS['rb'],
'BUCK': EXTENSIONS['bzl'],
'BUILD': EXTENSIONS['bzl'],
'Cargo.toml': EXTENSIONS['toml'] | {'cargo'},
'Cargo.lock': EXTENSIONS['toml'] | {'cargo-lock'},
Expand All @@ -398,7 +402,6 @@
'config.ru': EXTENSIONS['rb'],
'Containerfile': {'text', 'dockerfile'},
'CONTRIBUTING': EXTENSIONS['txt'],
'copy.bara.sky': EXTENSIONS['bzl'],

@asottile asottile Sep 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this at the very least is a breaking change

@lf- lf- Sep 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

no it's not, except, perhaps for someone expecting copy.bara.sky to match as bazel rather than starlark.

i added sky as an extension for starlark, because copybara actually uses any .bara.sky file if you load() them (and indeed .sky is a legacy extension for starlark in general), so my change in this respect is a bug fix.

i want more direction here: how afraid are we of minor breaking changes to the (very rare) users of copybara? should i leave this as is and add a comment that says we're leaving this incorrectly marked as bazel for compatibility reasons, while having other non-bazel starlark just marked as starlark?

what's the general attitude towards breaking changes?

@asottile asottile Sep 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what's the general attitude towards breaking changes?

we try pretty hard to not have breaking changes -- especially because they would be extremely subtle and difficult to track down. this library is really meant to be something that can be blindly updated at will

this is a breaking change in that someone using types: [bazel] to target starlark files would suddenly not be targetting this particular file (because it lost its bazel tag). and in the other case it's the opposite problem -- one may have been targetting types: [bazel] to format starlark and now they're receiving a .bxl file which isn't (?) starlark? -- and actually that file extension itself is ambiguous (a quick search of leads to a bunch of other things that aren't bazel so we can't accept it anyway)

@lf- lf- Sep 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

wait what, bxl isn't unique to buck? that's really surprising, i didn't know that.

bxl as used by buck is starlark.

@lf- lf- Sep 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

https://github.com/search?q=path%3A*.bxl&type=code ok here's a global search of all bxl files on GitHub. 100% of these are, based on my knowledge as a buck user, buck related, and are thus all starlark.

'COPYING': EXTENSIONS['txt'],
'Dockerfile': {'text', 'dockerfile'},
'direnvrc': EXTENSIONS['bash'],
Expand All @@ -418,6 +421,7 @@
'makefile': EXTENSIONS['mk'],
'NEWS': EXTENSIONS['txt'],
'NOTICE': EXTENSIONS['txt'],
'PACKAGE': EXTENSIONS['bzl'],
'PATENTS': EXTENSIONS['txt'],
'Pipfile': EXTENSIONS['toml'],
'Pipfile.lock': EXTENSIONS['json'],
Expand Down
Loading