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

[email protected] (#2511) #2512

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[email protected] (#2511) #2512

wants to merge 1 commit into from
+183 −4

Conversation

dmitry-j-mikhin
Copy link
Contributor

@dmitry-j-mikhin dmitry-j-mikhin commented Jul 30, 2024 •
edited
Loading

@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (libxml2) have been updated in this PR. Please review the changes.

@meteorcloudy meteorcloudy added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Jul 30, 2024
meteorcloudy
meteorcloudy previously approved these changes Jul 30, 2024
View reviewed changes
@meteorcloudy meteorcloudy enabled auto-merge (squash) July 30, 2024 15:26
@dmitry-j-mikhin
Copy link
Contributor Author

@meteorcloudy thanks for approval, I will try to fix failed checks.

@dmitry-j-mikhin dmitry-j-mikhin marked this pull request as draft July 30, 2024 15:53
auto-merge was automatically disabled July 30, 2024 15:53

Pull request was converted to draft

@bazel-io bazel-io dismissed meteorcloudy’s stale review July 30, 2024 15:57

Require module maintainers' approval for newly pushed changes.

@dmitry-j-mikhin
Copy link
Contributor Author

dmitry-j-mikhin commented Jul 30, 2024 •
edited
Loading

The issue seems to only exist on Windows in rules_foreign_cc during toolchain resolution. Opened discussion - bazel-contrib/rules_foreign_cc#1239

@dmitry-j-mikhin dmitry-j-mikhin mentioned this pull request Jul 30, 2024
Closed
@bazel-io
Copy link
Member

bazel-io commented Aug 2, 2024

Hello @jsharpe, modules you maintain (rules_foreign_cc) have been updated in this PR. Please review the changes.

@dmitry-j-mikhin
Copy link
Contributor Author

@meteorcloudy @jsharpe looks like I fixed the Windows build.
Now all the checkmarks are green. dmitry-j-mikhin/rules_foreign_cc@a2d3edf - this commit fixes the execution of rules_foreign_cc on Win with bzlmod.
I'll be on vacation next week, so I can prepare clean PRs after that.

@bazel-io
Copy link
Member

Hello @jsharpe, modules you maintain (rules_foreign_cc) have been updated in this PR. Please review the changes.

@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (libxml2) have been updated in this PR. Please review the changes.

jsharpe
jsharpe suggested changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@jsharpe jsharpe left a comment

Choose a reason for hiding this comment

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

The problem with this point fix to 0.11.1 is it's lost with 0.12.0, can you instead issue a PR to the main repo for rules_foreign_cc so we can verify it passes the tests and we can then get a proper release version to support this.

@dmitry-j-mikhin
Copy link
Contributor Author

The problem with this point fix to 0.11.1 is it's lost with 0.12.0, can you instead issue a PR to the main repo for rules_foreign_cc so we can verify it passes the tests and we can then get a proper release version to support this.

Yep, I'm working on it.

@dmitry-j-mikhin
Copy link
Contributor Author

@jsharpe clean PR based on 0.12.0 here - bazel-contrib/rules_foreign_cc#1264, I think it should be merged first, and then I can refactor the current PR to BCR. Right now the PR in BCR is based on my testing release - https://github.com/dmitry-j-mikhin/rules_foreign_cc/releases/tag/0.12.1-fix1239, it should be switched to the official rules_foreign_cc release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsharpe jsharpe jsharpe requested changes

@meteorcloudy meteorcloudy meteorcloudy left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

wanted: [GNOME/libxml2]
4 participants
@dmitry-j-mikhin @bazel-io @jsharpe @meteorcloudy