add libpng 1.6.51 #6636
add libpng 1.6.51 #6636
Conversation
|
@bazel-io skip_check unstable_url |
|
Hello @Vertexwahn, modules you maintain (libpng) have been updated in this PR. |
bazel-io
added
the
skip-url-stability-check
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for libpng version 1.6.51. The overall structure is good, but there are a couple of issues that need to be addressed. Firstly, there's a misconfiguration in source.json where it mixes the modern overlay/ directory mechanism with the legacy overlay field, which will likely cause failures. Secondly, the main MODULE.bazel file for the BCR entry is overly verbose; it should be simplified by removing dependency declarations, which belong in the MODULE.bazel file within the overlay. I've provided specific suggestions to resolve these issues.
| "strip_prefix": "libpng-1.6.51", | ||
| "overlay": { | ||
| "BUILD.bazel": "sha256-12C6LaW6jdwqJzGfBe4LmbhS0rpaJ9UilhDArScYV0E=", | ||
| "MODULE.bazel": "sha256-WtQb+OE1ZVGBv6xUyXMFxwJOQGDBmZWixqVt+wB1kNk=" | ||
| } |
There was a problem hiding this comment.
There's a configuration issue here that mixes two different overlay mechanisms. The PR correctly uses the modern overlay/ directory, which is supported by Bazel versions >=7.2.1 (as specified in bazel_compatibility). This makes the legacy overlay field in this source.json file both redundant and incorrect, as it points to files that don't exist at the expected location for this mechanism.
This will likely cause presubmit failures. To fix this, please remove the overlay field and rely solely on the overlay/ directory.
"strip_prefix": "libpng-1.6.51"There was a problem hiding this comment.
When did this field become deprecated? I see all other modules are still having it.
| module( | ||
| name = "libpng", | ||
| version = "1.6.51", | ||
| bazel_compatibility = [">=7.2.1"], # need support for "overlay" directory | ||
| compatibility_level = 1, | ||
| ) | ||
|
|
||
| bazel_dep(name = "platforms", version = "1.0.0") | ||
| bazel_dep(name = "rules_cc", version = "0.2.14") | ||
| bazel_dep(name = "rules_license", version = "1.0.0") | ||
| bazel_dep(name = "zlib", version = "1.3.1.bcr.7") |
There was a problem hiding this comment.
The MODULE.bazel file at the root of a version directory in the BCR should be minimal. It primarily serves as an entry point for bzlmod and should not declare the library's own dependencies. The dependencies required to build libpng (platforms, rules_cc, rules_license, zlib) should be declared only in the MODULE.bazel file that is overlaid into the source archive (i.e., overlay/MODULE.bazel).
Including them here can cause Bazel to process these dependencies twice. The bazel_compatibility attribute is also unnecessary in this file and belongs in overlay/MODULE.bazel.
Please simplify this file as suggested.
module(
name = "libpng",
version = "1.6.51",
compatibility_level = 1,
)
There was a problem hiding this comment.
Can a human check this suggestion? So far, whenever I had a different MODULE.bazel files within the overlay and in the root, I got build issues.
modules/libpng/1.6.51/presubmit.yml
Outdated
| - ubuntu2204 | ||
| - ubuntu2404 | ||
| - windows | ||
| bazel: [7.x, 8.x, rolling] |
There was a problem hiding this comment.
Change this to bazel: [7.x, 8.x, 9.*]. rolling will use Bazel 10 prerelease.
There was a problem hiding this comment.
Done, but now the maintainer needs to approve the change in order for CI to re-run.
DoDoENT
force-pushed
the
feature/libpng-1.6.51
branch
from
24ddc66 to
46cce94
Compare
|
@meteorcloudy Can you approve? |
bazel-io
left a comment
bazel-io
left a comment
There was a problem hiding this comment.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
|
@kotlaja Can you approve? |
3 participants
No description provided.