Skip to content

Conversation

@aignas
Copy link
Collaborator

@aignas aignas commented Nov 10, 2025

Run the examples that BCR uses as tests in a more similar way to BCR.

Unfortunately, this breaks the gazelle plugin on Windows due to #3416, so testing of
it is removed.

Work towards #3392

@aignas
Copy link
Collaborator Author

aignas commented Nov 10, 2025

OK, getting late, but I think this is a good start.

@rickeylev, do you remember what kind of limitations we have for CI slots?

@rickeylev
Copy link
Collaborator

There's a limit on the number of jobs. I'm not sure if its hard or soft.

Looking at the recent BCR failure, i'm guess it is covering windows+bazel 8 while we're not

@rickeylev
Copy link
Collaborator

re: CI failures: I think we just need --windows_enable_symlinks, then it should be OK

@aignas
Copy link
Collaborator Author

aignas commented Nov 13, 2025

I remember @meteorcloudy suggesting that we should avoid forcing users to set this flag, because it requires Windows admin permissions. If we need to add it, then the Windows support may not work properly. I think the interesting thing is that we need this flag because we are trying to use our binary in a sh_binary, so this requirement may be not related to rules_python requirements but to rules_shell requirements.

@rickeylev
Copy link
Collaborator

That's a good point and would explain what we see better. Yeah, the regular tests, which also show up as symlinks on disk (even without the enable symlinks flag), work fine

I tested all the 8.x.0 versions and they all have this same issue. Bazel 7 didn't.

@fmeum
Copy link
Member

fmeum commented Nov 14, 2025

One of the failing runs tries to run bazel coverage on Windows with Bazel 7. That certainly isn't supported and if it worked before then only by chance. It may work in Bazel 9.

@aignas aignas force-pushed the aignas.chore.bump-for-bazel-9 branch from f2b6876 to e89a50b Compare November 24, 2025 11:08
@aignas aignas changed the title add ci config to test 7, 8 and 9 for bcr like setup ci: add ci config to test 7 and 8 for bcr like setup Nov 25, 2025
@aignas
Copy link
Collaborator Author

aignas commented Nov 25, 2025

OK, I am wondering how we can fix the current build that is saying that we need to update C++ version, but I am not sure where this is handled.

@rickeylev
Copy link
Collaborator

--copt=-std=c++17 should do it.

IIRC, the PR that upgrades rules_cc and protobuf ran into this, too?

@rickeylev
Copy link
Collaborator

Is bumping deps required for rules_python? IIRC, BCR is happy (enough) with rules_python right now.

For the gazelle piece, yes, we had to bump its deps

@rickeylev
Copy link
Collaborator

Ah, I think I see. the rules_python module depends on the gazelle plugin (dev dep)

I'm going to try and decouple them.

@rickeylev
Copy link
Collaborator

This is almost passing. It's just gazelle+windows that is failing due to the treesitter compiling bug.

@aignas
Copy link
Collaborator Author

aignas commented Dec 4, 2025

Haha:
image
image

@aignas
Copy link
Collaborator Author

aignas commented Dec 4, 2025

What should we do?

@rickeylev
Copy link
Collaborator

Lets comment out the line testing it.

@rickeylev
Copy link
Collaborator

Aha! We can use python format syntax for parameterized parts in the name attribute. e.g. name: {bazel}

@rickeylev rickeylev marked this pull request as ready for review December 4, 2025 06:02
@rickeylev rickeylev requested a review from dougthor42 as a code owner December 4, 2025 06:02
@rickeylev
Copy link
Collaborator

fyi @dougthor42 -- we've had to remove testing for windows for gazelle

@dougthor42
Copy link
Collaborator

fyi @dougthor42 -- we've had to remove testing for windows for gazelle

ACK and SGTM. The tree sitter compilation issue is super annoying. Sadly I haven't had any more time to dive into it (and having to learn gcc args doesn't help!)

Please make sure to note in the changelog that gazelle windows ci is temporarily disabled and that gazelle windows support is questionable.

@rickeylev
Copy link
Collaborator

note in changelog

Done


matrix:
platform:
- ubuntu2204
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add Ubuntu 2404 too? Or is it not available in our ci?

matrix:
platform:
- ubuntu2204
- debian11
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add debian 12? 13 is already out also.

@aignas
Copy link
Collaborator Author

aignas commented Dec 4, 2025

I see that usually people test a particular version of the OS and I'd like to leave this for some future PR.

Source: spot checked various modules on the bazel central registry a little.

@aignas aignas added this pull request to the merge queue Dec 4, 2025
Merged via the queue into bazel-contrib:main with commit 759f5da Dec 4, 2025
4 checks passed
@aignas aignas deleted the aignas.chore.bump-for-bazel-9 branch December 4, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants