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.

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

Jump to bottom

Remove patter that does not glob anything #745

Merged
merged 2 commits into from
Sep 10, 2022
Merged

Remove patter that does not glob anything #745

merged 2 commits into from
Sep 10, 2022

Conversation

limdor
Copy link
Contributor

@limdor limdor commented Aug 24, 2022

This pattern does not glob anything because inside the upbc folder there is a BUILD file.
Removing this allows to build upb with the flag incompatible_disallow_empty_glob

Note: This is needed in order to build Bazel with the flag flipped.

@limdor
Remove patter that does not glob anything
9a02d16 
This pattern does not glob anything because inside the upbc folder there is a BUILD file.
Removing this allows to build upb with the flag incompatible_disallow_empty_glob
@fowles fowles requested a review from ericsalo August 24, 2022 20:24
@limdor limdor mentioned this pull request Aug 24, 2022
Closed
56 tasks
ericsalo
ericsalo reviewed Aug 26, 2022
View reviewed changes
Copy link
Member

@ericsalo ericsalo left a comment

Choose a reason for hiding this comment

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

This seems like an innocent enough change but many of the tests are now breaking.

@limdor
Copy link
Contributor Author

limdor commented Aug 26, 2022

This seems like an innocent enough change but many of the tests are now breaking.

Can you point me to the log of a failing test? The ones here in github they seem to fail but to me looks like something unrelated. Or do you think that is related to the change?

Error: google-github-actions/auth failed with: retry function failed with 0 attempts: the GitHub Action workflow must specify exactly one of "workload_identity_provider" or "credentials_json"! If you are specifying input values via GitHub secrets, ensure the secret is being injected into the environment. By default, secrets are not passed to workflows triggered from forks, including Dependabot.

Another option is to add the allow_empty=True, but it would be good to investigate why the tests are failing, I cannot see how this could change behaviour.

@ericsalo
Copy link
Member

The cause of the failures has been identified. Once we have fixed them on our end we should be able to proceed with this PR. Unfortunately most of the team will be away on an offsite this coming week so it may be a little while before we can get to it.

@limdor
Copy link
Contributor Author

limdor commented Aug 28, 2022

Ok, thanks for the update. The goal would be before the cut for Bazel 6 on September 12th to have this merged and Bazel updated with a version that contains this change.

@limdor
Copy link
Contributor Author

limdor commented Sep 7, 2022

@ericsalo any updates on this?

@copybara-service copybara-service bot merged commit 8ab26d4 into protocolbuffers:main Sep 10, 2022
@limdor
Copy link
Contributor Author

limdor commented Sep 12, 2022

@ericsalo would it be possible to bring this change to the 21.x branch? What would be the process for that?

@fowles
Copy link
Contributor

fowles commented Sep 12, 2022

a PR with a cherrypick would suffice

@limdor
Copy link
Contributor Author

limdor commented Sep 12, 2022

Thanks @fowles, I created the PR in #780

copybara-service bot pushed a commit to bazelbuild/bazel that referenced this pull request Oct 7, 2024
@limdor @copybara-github
Flip disallow empty glob
538b119 
This PR is to flip the default value of `incompatible_disallow_empty_glob`. This flag is tracked in #8195 and it is available since Bazel 0.29 from August 2019.
Once this is merged, users still can allow empty globs without being explicit setting `--incompatible_disallow_empty_glob=false`.

The motivation of the flip is bazel-contrib/SIG-rules-authors#37 and the fact of not keeping an incompatible flag for 4 years without flipping it.

What needs to be done before flipping this:

- [x] Merge protocolbuffers/upb#584
- [x] Update upb to a version that has the change in protocolbuffers/upb#584
- [x] Merge protocolbuffers/upb#745
- [x] Bring the changes from protocolbuffers/upb#745 to the 21.x branch
- [x] Bring the changes from protocolbuffers/upb@e5f2601 to the 21.x branch (protocolbuffers/upb#781)
- [x] Update upb to the latest commit of the 21.x branch (#16343)
- [x] Merge #15374
- [x] Merge #15339
- [x] Merge #15330
- [x] Merge #16431
- [x] Merge #16468
- [x] Get green checks for this PR
- [ ] Fix failures in downstream projects: https://buildkite.com/bazel/bazelisk-plus-incompatible-flags
  - [ ] Fix empty globs from IntellIJ Plugin (https://github.com/bazelbuild/intellij)
    - [ ] Issue: bazelbuild/intellij#4040
    - [ ] Android Studio Plugin
    - [ ] Android Studio Plugin Google
    - [ ] CLion Plugin
    - [ ] CLion Plugin Google
    - [ ] IntelliJ Plugin
    - [ ] IntelliJ Plugin Aspect
    - [ ] IntelliJ Plugin Aspect Google
    - [ ] IntelliJ Plugin Google
    - [ ] IntelliJ UE Plugin
    - [ ] IntelliJ UE Plugin Google
    - Opened PRs
      - [x] bazelbuild/intellij#4025
      - [x] bazelbuild/intellij#4038
  - [x] Fix empty globs from Bazel Bench (https://github.com/bazelbuild/bazel-bench)
    - [ ] bazelbuild/bazel-bench#174
    - [x] bazelbuild/bazel-bench#149
  - [x] Fix empty globs from Buildfarm (https://github.com/bazelbuild/bazel-buildfarm)
  - [x] Fix empty globs from Cargo
  - [x] ~~Fix empty globs from Cartographer (https://github.com/cartographer-project/cartographer)~~ (No longer maintained)
    - Issue: cartographer-project/cartographer#1908
    - PR to enforce it cartographer-project/cartographer#1944
  - [x] Fix empty globs from Cloud Robotics Core (https://github.com/googlecloudrobotics/core/)
    - [x] PR to prevent regresions googlecloudrobotics/core#279
  - [ ] Fix empty globs from Envoy
  - [x] Fix empty globs from FlatBuffers
  - [x] Fix empty globs from Flogger
  - [ ] Fix empty globs from Gerrit (https://gerrit.googlesource.com/gerrit.git)
    - Issue: https://issues.gerritcodereview.com/issues/40011754
  - [x] Fix empty globs from :bazel:Protobuf
  - [x] Fix empty globs from rules_android_ndk
  - [ ] Fix empty globs from rules_dotnet
  - [x] Fix empty globs from :bazel:rules_foreign_cc bazel-contrib/rules_foreign_cc#974
  - [x] Fix empty globs from rules_go
  - [x] Fix empty globs from rules_haskell tweag/rules_haskell#1827
  - [x] Fix empty globs from :bazel:rules_jvm_external
  - [x] Fix empty globs from rules_kotlin
  - [x] Fix empty globs from rules_nodejs
  - [x] Fix empty globs from :bazel:rules_python
  - [ ] Fix empty globs from rules_rust
  - [x] Fix empty globs from rules_swift
  - [x] Fix empty globs from rules_webtesting
  - [x] Fix empty globs from TensorFlow  tensorflow/tensorflow#58155
    - [x] tensorflow/tensorflow#58008
    - [x] tensorflow/tensorflow#58154
  - [x] Fix empty globs from upb

RELNOTES[INC]: The flag `--incompatible_disallow_empty_glob` now defaults to true.

Closes #15327.

PiperOrigin-RevId: 683316885
Change-Id: Id34ffd32a9e979fada9207e475b826a508100ba5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericsalo ericsalo ericsalo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@limdor @ericsalo @fowles