Skip to content

[email protected] #4341

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

Merged
merged 12 commits into from
Apr 24, 2025
Merged

[email protected] #4341

merged 12 commits into from
Apr 24, 2025

Conversation

UebelAndre
Copy link
Contributor

@UebelAndre UebelAndre commented Apr 17, 2025

closes #3926

@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (verilator) have been updated in this PR.
Please review the changes. You can view a diff against the previous version in the "Generate module diff" check.

@UebelAndre UebelAndre force-pushed the verilator branch 2 times, most recently from 2c0d26d to 3ae67e6 Compare April 17, 2025 04:04
@UebelAndre
Copy link
Contributor Author

@bazel-io skip_check unstable_url

@bazel-io bazel-io added the skip-url-stability-check Skip the URL stability check for the PR label Apr 17, 2025
@UebelAndre UebelAndre marked this pull request as ready for review April 17, 2025 04:10
@UebelAndre
Copy link
Contributor Author

FYI @hzeller

Copy link
Contributor

@hzeller hzeller left a comment

Choose a reason for hiding this comment

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

Nice!
I think @QuantamHD might be interested to have a look.

@meteorcloudy meteorcloudy added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Apr 17, 2025
@UebelAndre UebelAndre force-pushed the verilator branch 5 times, most recently from 168d234 to b11bade Compare April 17, 2025 13:39
@meteorcloudy
Copy link
Member

@UebelAndre Thanks for all the fixes, to unblock this, you can comment @bazel-io skip_check incompatible_flags in this PR. See https://github.com/bazelbuild/bazel-central-registry/blob/main/docs/README.md#testing-incompatible-flags

@UebelAndre
Copy link
Contributor Author

For me this only blocks a migration to bzlmod but I'd rather not have to keep local patches for the issues identified in #4341 (comment). I'd rather wait for @jmillikin to review to avoid churn making *.bcr.* patches here. Hopefully that doesn't take more than a day or two.

@UebelAndre
Copy link
Contributor Author

@bazel-io skip_check incompatible_flags

@meteorcloudy this build actually fails for a different reason and actually conforms to all incompatibility flags. FIY:

(15:31:26) ERROR: /Users/buildkite/builds/bk-macos-arm64-6c7i/bazel-org-repo-root/output/verilator-5.034/ci/docker/run/hooks/BUILD:10:8: syntax error at 'build': expected newline
(15:31:26) ERROR: /Users/buildkite/builds/bk-macos-arm64-6c7i/bazel-org-repo-root/output/verilator-5.034/ci/docker/run/hooks/BUILD:10:40: invalid character: '$'
(15:31:26) ERROR: /Users/buildkite/builds/bk-macos-arm64-6c7i/bazel-org-repo-root/output/verilator-5.034/ci/docker/run/hooks/BUILD:10:60: invalid character: '$'
(15:31:26) ERROR: /Users/buildkite/builds/bk-macos-arm64-6c7i/bazel-org-repo-root/output/verilator-5.034/ci/docker/run/hooks/BUILD:10:80: invalid character: '$'
(15:31:26) ERROR: package contains errors: ci/docker/run/hooks
(15:31:28) ERROR: package contains errors: ci/docker/run/hooks: syntax error at 'build': expected newline
(15:31:28) WARNING: Target pattern parsing failed.
(15:31:28) ERROR: Skipping '//...': Error evaluating '//...': error loading package 'ci/docker/run/hooks': Package 'ci/docker/run/hooks' contains errors
(15:31:28) ERROR: Error evaluating '//...': error loading package 'ci/docker/run/hooks': Package 'ci/docker/run/hooks' contains errors

@meteorcloudy
Copy link
Member

Hmm, looks like the BUILD file was somehow changed? Let's retry the build first.

@meteorcloudy
Copy link
Member

I think this might be a weird bug in Bazel, I'm able to reproduce this locally by running bazel build twice.

@meteorcloudy
Copy link
Member

Although the content of

$ cat ci/docker/run/hooks/BUILD
#!/bin/bash
# DESCRIPTION: Docker hub hook to pass SOURCE_COMMIT
#
# Copyright 2020 by Stefan Wallentowitz. This program is free software; you
# can redistribute it and/or modify it under the terms of either the GNU
# Lesser General Public License Version 3 or the Perl Artistic License
# Version 2.0.
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0

docker build --build-arg SOURCE_COMMIT=${SOURCE_COMMIT} -f $DOCKERFILE_PATH -t $IMAGE_NAME .

is indeed confusing to bazel

@meteorcloudy meteorcloudy added the skip-incompatible-flags-test Bypass the incompatible flags test in BCR presubmit label Apr 24, 2025
@meteorcloudy
Copy link
Member

Let's wait for other reviewers to take a look then I can help merge it.

@UebelAndre
Copy link
Contributor Author

@meteorcloudy looks like windows workers are also running into a similar issue.

@meteorcloudy
Copy link
Member

https://github.com/verilator/verilator/blob/master/ci/docker/run/hooks/build is confusing Bazel.

@UebelAndre Let's add a .bazelignore file at the source root containing ci

$ cat .bazelignore
ci

@meteorcloudy meteorcloudy removed the skip-incompatible-flags-test Bypass the incompatible flags test in BCR presubmit label Apr 24, 2025
@meteorcloudy
Copy link
Member

@UebelAndre Looks like windows doesn't work, feel free to remove

@UebelAndre
Copy link
Contributor Author

@meteorcloudy this PR is good to go on my end. I leave the rest to you

@meteorcloudy meteorcloudy merged commit ba50db2 into bazelbuild:main Apr 24, 2025
14 checks passed
@UebelAndre UebelAndre deleted the verilator branch April 25, 2025 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wanted: verilator
5 participants