Skip to content

verilator@5.034#4341

Merged
meteorcloudy merged 12 commits into
bazelbuild:mainfrom
UebelAndre:verilator
Apr 24, 2025
Merged

verilator@5.034#4341
meteorcloudy merged 12 commits into
bazelbuild:mainfrom
UebelAndre:verilator

Conversation

@UebelAndre

@UebelAndre UebelAndre commented Apr 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

closes #3926

@bazel-io

bazel-io commented Apr 17, 2025

Copy link
Copy Markdown
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

UebelAndre commented Apr 17, 2025

Copy link
Copy Markdown
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

UebelAndre commented Apr 17, 2025

Copy link
Copy Markdown
Contributor Author

FYI @hzeller

hzeller
hzeller reviewed Apr 17, 2025
View reviewed changes

@hzeller hzeller left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread modules/verilator/5.034/MODULE.bazel
@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
@UebelAndre

UebelAndre commented Apr 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This is impacted by:

@meteorcloudy

meteorcloudy commented Apr 17, 2025

Copy link
Copy Markdown
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

UebelAndre commented Apr 17, 2025

Copy link
Copy Markdown
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
UebelAndre commented Apr 17, 2025
View reviewed changes
Comment thread modules/verilator/metadata.json
@jmillikin jmillikin mentioned this pull request Apr 18, 2025
Closed
@stridge-tnp stridge-tnp mentioned this pull request Apr 24, 2025
Draft
@UebelAndre

UebelAndre commented Apr 24, 2025

Copy link
Copy Markdown
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

meteorcloudy commented Apr 24, 2025

Copy link
Copy Markdown
Member

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

@meteorcloudy

meteorcloudy commented Apr 24, 2025

Copy link
Copy Markdown
Member

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

@meteorcloudy

meteorcloudy commented Apr 24, 2025

Copy link
Copy Markdown
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
meteorcloudy reviewed Apr 24, 2025
View reviewed changes
Comment thread modules/verilator/5.034/presubmit.yml
@meteorcloudy

meteorcloudy commented Apr 24, 2025

Copy link
Copy Markdown
Member

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

@UebelAndre

UebelAndre commented Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

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

jmillikin
jmillikin reviewed Apr 24, 2025
View reviewed changes
Comment thread modules/verilator/5.034/overlay/BUILD.bazel Outdated
@meteorcloudy

meteorcloudy commented Apr 24, 2025

Copy link
Copy Markdown
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

jmillikin
jmillikin reviewed Apr 24, 2025
View reviewed changes
Comment thread modules/verilator/5.034/overlay/private/verilator_utils.bzl
@meteorcloudy meteorcloudy removed the skip-incompatible-flags-test Bypass the incompatible flags test in BCR presubmit label Apr 24, 2025
@meteorcloudy

meteorcloudy commented Apr 24, 2025

Copy link
Copy Markdown
Member

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

@UebelAndre UebelAndre mentioned this pull request Apr 24, 2025
Closed
@UebelAndre

UebelAndre commented Apr 24, 2025

Copy link
Copy Markdown
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
@UebelAndre UebelAndre deleted the verilator branch April 25, 2025 01:02
filmil pushed a commit to filmil/bazel-central-registry that referenced this pull request Jun 23, 2025
@UebelAndre @filmil
verilator@5.034 (bazelbuild#4341)
150818b 
closes bazelbuild#3926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@meteorcloudy meteorcloudy meteorcloudy approved these changes

+2 more reviewers

@hzeller hzeller hzeller left review comments

@jmillikin jmillikin jmillikin left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

wanted: verilator

5 participants

@UebelAndre @bazel-io @meteorcloudy @hzeller @jmillikin