-
Notifications
You must be signed in to change notification settings - Fork 312
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
Add libxml2 2.9.12 #282
Add libxml2 2.9.12 #282
Conversation
@jsharpe The module file is missing from the source archive. You can add it with a patch. |
This was generated with the add_module.py tooling; it should be updated to add the module file patch automatically. |
@meteorcloudy I stopped iterating on this because I don't have a local windows box setup to iterate the tests and having to wait for BCR maintainer approver for buildkite each time was becoming tedious. Are there some conditions under which we can persist the approval between CI runs on the same PR? |
Alternatively I can just drop windows from the matrix? |
I think if a new commit is pushed to the PR which was previously approved by a registry maintainer, and the commit didn't touch the presubmit.yml file, we could persist the approval. I'm not sure how feasible that is (and whether that actually reduces the churn in your case?) |
Or maybe we can add a label to indicate a PR is sent by a trusted contributor, then we bypass the BCR reviewer approval for CI run? |
GitHub actions has a nice default, where a contribution from someone with no commits on the project requires approval |
@Wyverald @meteorcloudy what is the policy regarding having to have linux, macos and windows support working in presubmit.yml. I don't have bandwidth nor a windows setup to work out how to fix this on windows; can I just remove the windows platform from the test matrix? |
Yes, you can leave a TODO in the presubmit to enable windows in the matrix, like
However I'm concerned that no one will ever pick up the TODO, so any module that depends on libxml won't work on Windows either. I think we should show this sort of compatibility warning on the registry.bazel.build to make it discoverable, though we'd have to do some parsing to figure it out. |
This is also a higher barrier to entry for module maintainers - they need to be fluent in building on all three major platforms in general to be able to publish high quality modules. I suspect that the number of people familiar with getting bazel builds working on windows is relatively low and may mean that generally windows becomes a second class citizen in the registry? |
If you're a rule author, you already had the bad news that you need to be able to debug Bazel problems on Windows. If you're a library author like libxml2 maintainers, you also had to add preprocessor definitions or something. The problem here is that the BCR has accidentally become a C/C++ package manager, so we have contributors who are not the rule author nor the library author. libxml2 probably does work on Windows, but you're just the messenger and don't know why unistd.h isn't on the windows machine used for CI. Windows is already a second class experience in Bazel but some policy here would be important to prevent that getting worse. |
I see becoming a C/C++ package manager as more than an accident: If we don't at least centralize the effort to improve Windows support for Bazel projects, it will never become better. It's just the right kind of complex and niche that everyone will silently try to fix their issues themselves in hackish ways. Maybe some companies that use Bazel on Windows could contribute a bit of funding to motivate module maintainers to figure out Windows builds? In this particular case, Windows support may require adding libiconv first (https://github.com/conan-io/conan-center-index/blob/master/recipes/libiconv/all/conanfile.py). |
We don't really have a policy for this. I think it's fine to remove windows for now and leave a TODO there. But I think we do need a better way to indicate which modules work on which platforms. |
Hello! I'm doing some routine cleanup of stale PRs. If you're still working on this and planning to make progress soon, please let me know and we can re-open it. |
No description provided.