-
Notifications
You must be signed in to change notification settings - Fork 331
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
[nfc] dependency updates, expand usage of bazel implementation_deps #1583
Conversation
include_prefix = ".", | ||
copts = ["-w"], | ||
defines = [ | ||
"ADA_SSE2=1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just need some context... Why are you removing this define?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ada-url auto-detects support for SSE2, see https://github.com/ada-url/ada/blob/7ed703c4fddeaaa0178383a86df9f40d34ff2881/include/ada/common_defs.h#L292. SSE2 support is required for x86_64 machines and detected as such, so there is no need to enable it explicitly. I previously thought that this could also cause problems when compiling on arm64 as SSE2 is an x86-specific instruction set, but luckily ada checks for the presence of ADA_NEON before checking for SSE2 (Neon instructions are required for arm64 and also autodetected here).
Note that this also enables warnings for ada-url – disabling warnings can be useful for 3rd-Party dependencies and we do it manually for a few in .bazelrc but I didn't encounter any warnings locally on macOS so it might not be needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah nice. I added this originally because the build had failed originally locally without it but appears to be working with this now. Interesting.
f5b8a06
to
ad38155
Compare
- Update rules_fuzzing, update a comment on its impact on absl and do not import superfluous dependencies. - Update rules_python. Calling py_repositories() is now mandatory. Based on rules_fuzzing still using a deprecated pip_parse() parameter, we can't take this further than 0.26.0. - rules_benchmark() dependencies are not actually required, do not include them. - Update capnproto - Update bazel to 7.0.2 - Lint WORKSPACE and replace a stray Unicode dash character.
This helps us reduce package interdependencies and results in simpler, more readable compile commands. Move pyodide dependency out of io – it is relatively large and not needed there. Also do some linting and clean up BUILD.ada-url
ad38155
to
bbae171
Compare
This was previously blocked on rules_fuzzing still using outdated pip_parse syntax, see #1583.
This was previously blocked on rules_fuzzing still using outdated pip_parse syntax, see #1583.
This was previously blocked on rules_fuzzing still using outdated pip_parse syntax, see #1583.
No description provided.