Skip to content

Use swig from BCR, allowing to remove last dep on WORKSPACE#9555

Merged
maliberty merged 2 commits intoThe-OpenROAD-Project:masterfrom
hzeller:feature-20260226-use-bcr-swig
Mar 2, 2026
Merged

Use swig from BCR, allowing to remove last dep on WORKSPACE#9555
maliberty merged 2 commits intoThe-OpenROAD-Project:masterfrom
hzeller:feature-20260226-use-bcr-swig

Conversation

@hzeller
Copy link
Collaborator

@hzeller hzeller commented Feb 26, 2026

No description provided.

@hzeller
Copy link
Collaborator Author

hzeller commented Feb 26, 2026

CC: @oharboe

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request migrates the swig dependency from a WORKSPACE definition to using the version from the Bazel Central Registry (BCR). This is a great improvement for modernizing the build and removing the legacy WORKSPACE file. The changes in the .bzl files to update the swig label are correct. I've only left one comment in MODULE.bazel regarding the alphabetical sorting of dependencies to improve maintainability.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@oharboe
Copy link
Collaborator

oharboe commented Feb 26, 2026

😌

@hzeller
Copy link
Collaborator Author

hzeller commented Feb 26, 2026

It is unclear why the CI failed - there are no usable logs.

@hzeller hzeller marked this pull request as draft February 26, 2026 17:50
@hzeller
Copy link
Collaborator Author

hzeller commented Feb 26, 2026

making this a draft for now. Looks like the other CI shows an actual error that can be traced to swig

https://jenkins.openroad.tools/job/OpenROAD-Public/job/PR-9555-merge/1/pipeline-overview/?start-byte=34095&selected-node=161#log-161-138

@maliberty
Copy link
Member

[2026-02-26T17:18:41.705Z] (17:18:41) ERROR: /tmp/workspace/OpenROAD-Public_PR-9555-head-1-bazel-ci/src/ant/BUILD:75:12: Action src/ant/swig.cc [for tool] failed: (Exit 1): swig failed: error executing Action command (from target //src/ant:swig) bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/swig+/swig -tcl8 -c++ -module ant -namespace -prefix ant -Isrc/ant/src -o bazel-out/k8-opt-exec-ST-d57f47055a04/bin/src/ant/swig.cc ... (remaining 1 argument skipped)
[2026-02-26T17:18:41.705Z]
[2026-02-26T17:18:41.705Z] Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
[2026-02-26T17:18:41.705Z] :EOF: Error: Unable to find 'swig.swg'
[2026-02-26T17:18:41.705Z] :2: Error: Unable to find 'tcl8.swg'

@hzeller
Copy link
Collaborator Author

hzeller commented Feb 26, 2026

Looks like the BCR version is doing things a bit differently. Will have to dig into that and possibly upstream a fix.
So this PR might take a little longer.

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller hzeller force-pushed the feature-20260226-use-bcr-swig branch from 82ccb13 to 18634e4 Compare March 2, 2026 18:07
Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller hzeller force-pushed the feature-20260226-use-bcr-swig branch from 18634e4 to b02bdbf Compare March 2, 2026 18:08
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

clang-tidy review says "All clean, LGTM! 👍"

@hzeller
Copy link
Collaborator Author

hzeller commented Mar 2, 2026

Added swig 4.3.0.bcr.2 with tcl support in in bazelbuild/bazel-central-registry#7732 so this should work now.

Note, even though we could remove WORKSPACE in this PR (yay!), we can't remove the common --enable_workspace quite yet; apparently this requires the rules_python transition from #9558 first which currently awaits yaml-issue-resolving.

@hzeller hzeller marked this pull request as ready for review March 2, 2026 19:09
@hzeller
Copy link
Collaborator Author

hzeller commented Mar 2, 2026

Ready for review.

Copy link
Member

Choose a reason for hiding this comment

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

🥇

@maliberty maliberty merged commit 38e5ad7 into The-OpenROAD-Project:master Mar 2, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants