search

The-OpenROAD-Project / OpenROAD

OpenROAD's unified application implementing an RTL-to-GDS Flow. Documentation at https://openroad.readthedocs.io/en/latest/
https://theopenroadproject.org/
BSD 3-Clause "New" or "Revised" License
1.62k stars 561 forks source link

Update Yosys version on DependencyInstaller.sh #6140

Closed gudeh closed 7 hours ago

gudeh commented 1 week ago

I was having packages issues with EQY version, it should be the same version as Yosys. The DependencyInstaller.sh was not solving the issue. After manually updating to 0.47 my EQY and Yosys versions matched, as supposed to.

github-actions[bot] commented 1 week ago

clang-tidy review says "All clean, LGTM! :+1:"

gudeh commented 1 week ago

I noticed on secure-CI and even nightly log reports from EQY state Yosys version as 0.43, being outdated. Although Yosys logs itself have the correct version 0.47.

I did some greps with the logs downloaded from the last successful nightly 4736 for asap7/jpeg:

image

eder-matheus commented 1 week ago

@maliberty The CI is failing for this PR with the following messages:

[2024-11-12T21:28:44.817Z] 211.2 Cloning into 'yosys'...
[2024-11-12T21:28:44.817Z] 211.3 warning: Could not find remote branch yosys-0.47 to clone.
[2024-11-12T21:28:44.817Z] 211.3 fatal: Remote branch yosys-0.47 not found in upstream origin

Looking at both yosys and eqy repositories, in fact we don't have this yosys-0.47 branch. But we also don't have the yosys-0.43, which is the branch name used previously. Do you know how it works? I think we are missing something here...

maliberty commented 1 week ago

@povik please advise

povik commented 1 week ago

We dropped the yosys- prefix from release tags, the current release tag is 0.47. Sorry about that, there were some reasons for the change which I don't remember now

github-actions[bot] commented 1 week ago

clang-tidy review says "All clean, LGTM! :+1:"

eder-matheus commented 1 week ago

We dropped the yosys- prefix from release tags, the current release tag is 0.47. Sorry about that, there were some reasons for the change which I don't remember now

Apparently the eqy repo is using the yosys- prefix on its tags. So using 0.47 or yosys-0.47 will lead to errors in the CI.

maliberty commented 1 week ago

There looks to be a yosys-0.47 tag - https://github.com/YosysHQ/eqy/releases/tag/yosys-0.47

Why does that lead to an error?

eder-matheus commented 1 week ago

There looks to be a yosys-0.47 tag - https://github.com/YosysHQ/eqy/releases/tag/yosys-0.47

Why does that lead to an error?

When we use the yosys-0.47 as the version name, this error is raised:

[2024-11-12T21:28:43.833Z] #8 211.2 Cloning into 'yosys'...
[2024-11-12T21:28:43.833Z] #8 211.3 warning: Could not find remote branch yosys-0.47 to clone.
[2024-11-12T21:28:43.833Z] #8 211.3 fatal: Remote branch yosys-0.47 not found in upstream origin
[2024-11-12T21:28:44.817Z] #8 ERROR: process "/bin/sh -c /tmp/DependencyInstaller.sh -common $INSTALLER_ARGS" did not complete successfully: exit code: 128

When using 0.47, this is the error:

[2024-11-13T17:04:49.992Z] #8 312.2 Cloning into 'eqy'...
[2024-11-13T17:04:49.992Z] #8 312.4 warning: Could not find remote branch 0.47 to clone.
[2024-11-13T17:04:49.992Z] #8 312.4 fatal: Remote branch 0.47 not found in upstream origin
[2024-11-13T17:04:54.310Z] #8 ERROR: process "/bin/sh -c /tmp/DependencyInstaller.sh -common $INSTALLER_ARGS" did not complete successfully: exit code: 128

It seems that we're trying to clone both eqy and yosys repos using the same version name, which will not work.

maliberty commented 1 week ago

@povik could you put a 0.47 tag on same eqy commit as yosys-0.47?

povik commented 1 week ago

You're not the first to ask (YosysHQ/yosys#4586). I will need to get the internal go-ahead which will take a day or two

povik commented 1 week ago

@povik could you put a 0.47 tag on same eqy commit as yosys-0.47?

It won't happen this week that we get a consistent tag. I would say it's simple enough but it's not up to me.

maliberty commented 1 week ago

@gudeh you can wait or split the tag for now

eder-matheus commented 1 week ago

@gudeh you can wait or split the tag for now

I would argue for spliting the tag for now, and in the new yosys update I will go back to it and fix the tag names.

github-actions[bot] commented 3 days ago

clang-tidy review says "All clean, LGTM! :+1:"

gudeh commented 3 days ago

I noticed a limitation in the current dependency installer script. Currently, it installs yosys, sby, and eqy only if they are not found in the /usr/local/ path. This means it does not update these dependencies if an older version is already installed. We might want to consider checking the installed version and updating it if it doesn't match the latest version.

I tried implementing this feature but was not successful at a first moment, so I am currently only doing the split tag for yosys and eqy/sby.

I noticed this behavior for yosys, eqy, and sby, but it could potentially occur with other dependencies as well.

gudeh commented 10 hours ago

I do not understand why the pr-head is failing, are we still having CI issues?

12:27:33  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/build/src/gui/gui_autogen/mocs_compilation.cpp:8:
12:27:33  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/build/src/gui/gui_autogen/UVLADIE3JM/moc_findDialog.cpp:10:
12:27:33  /tmp/workspace/OpenROAD-Public_PR-6140-head/build/src/gui/gui_autogen/UVLADIE3JM/../../../../../src/gui/src/findDialog.h:46:8: warning: 'accept' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
12:27:33    void accept();
12:27:33         ^
12:27:33  /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:97:18: note: overridden virtual function is here
12:27:33      virtual void accept();
12:27:33                   ^
12:27:33  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/build/src/gui/gui_autogen/mocs_compilation.cpp:8:
12:27:33  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/build/src/gui/gui_autogen/UVLADIE3JM/moc_findDialog.cpp:10:
12:27:33  /tmp/workspace/OpenROAD-Public_PR-6140-head/build/src/gui/gui_autogen/UVLADIE3JM/../../../../../src/gui/src/findDialog.h:47:8: warning: 'reject' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
12:27:33    void reject();
12:27:33         ^
12:27:33  /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:98:18: note: overridden virtual function is here
12:27:33      virtual void reject();
12:27:33                   ^
12:27:33  7 warnings generated.
12:27:35  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/src/gui/src/gui.cpp:46:
12:27:35  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/src/gui/src/mainWindow.h:43:
12:27:35  /tmp/workspace/OpenROAD-Public_PR-6140-head/src/gui/src/findDialog.h:46:8: warning: 'accept' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
12:27:35    void accept();
12:27:35         ^
12:27:35  /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:97:18: note: overridden virtual function is here
12:27:35      virtual void accept();
12:27:35                   ^
12:27:35  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/src/gui/src/gui.cpp:46:
12:27:35  In file included from /tmp/workspace/OpenROAD-Public_PR-6140-head/src/gui/src/mainWindow.h:43:
12:27:35  /tmp/workspace/OpenROAD-Public_PR-6140-head/src/gui/src/findDialog.h:47:8: warning: 'reject' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
12:27:35    void reject();
12:27:35         ^
12:27:35  /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:98:18: note: overridden virtual function is here
12:27:35      virtual void reject();
12:27:35                   ^
12:27:36  2 warnings generated.
12:27:36  2 warnings generated.
12:27:37  2 warnings generated.
12:27:38  2 warnings generated.
12:27:41  [ 79%] Linking CXX static library gui.a
12:27:41  [ 79%] Built target gui
12:27:41  gmake: *** [Makefile:146: all] Error 2
12:27:41  
12:27:41  real  1m41.456s
12:27:41  user  46m10.424s
12:27:41  sys   3m20.033s

I tried restarting the run a couple of times already

github-actions[bot] commented 9 hours ago

clang-tidy review says "All clean, LGTM! :+1:"