Skip to content
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

Fix flaky build and test failures on windows #2217

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

daivinhtran
Copy link
Contributor

@daivinhtran daivinhtran commented Oct 26, 2023

This PR initially patched #1899 to fix any unrelated errors and then reverted the PR. Two fixes in this PR:

  1. Shortening the test name to avoid maximum path length limit on windows.
  2. Disable more proto targets on windows because building rust_prost_toolchain now fails with the hash introduced by the transition in Support -Cpanic=abort #1899. The hash makes the the path over the limit as mentioned in (1).

@daivinhtran daivinhtran force-pushed the fix-test-names branch 2 times, most recently from b1d5428 to ba11c6d Compare October 26, 2023 15:39
@daivinhtran daivinhtran changed the title Patch github.com/bazelbuild/rules_rust/pull/1899 Fix flaky test failures on windows Oct 27, 2023
@daivinhtran daivinhtran changed the title Fix flaky test failures on windows Fix flaky build and test failures on windows Oct 27, 2023
@daivinhtran daivinhtran marked this pull request as ready for review October 27, 2023 15:41
Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Thanks! Just one question

- "-//test/unit/pipelined_compilation/..."
# The proto rules do not work on windows
- "-//proto/..."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we instead tag specific targets as incompatible with windows? How many targets fail on windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty much all proto targets fail on windows because rust_prost_toolchain fails building on windows after #1899 introduces a transition to make the filepath even longer with the config hash.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@UebelAndre could we move this forward and disable windows the path length issues are taken care of?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m not convinced we need to disable the unit tests but haven’t had time to figure out if transitioning for the panic PR is necessary.

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.

3 participants