Skip to content
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

openroad-gui: add OpenROAD build with GUI option #228

Merged
merged 4 commits into from
Feb 22, 2023

Conversation

joennlae
Copy link

Thank you for this excellent repo 💯 . It saved me a ton of time 👍

This PR adds a GUI-supported build for OpenROAD. This build is tested and can be installed via conda:

conda install -c joennlae/label/ci-add-openroad-gui-2771065658 openroad-gui

@mithro mithro requested review from proppy and PiotrZierhoffer July 31, 2022 23:50
@proppy
Copy link
Contributor

proppy commented Aug 1, 2022

@joennlae Thanks for your contribution!

We had discussions on #193 (comment) on the differentiation between the package in this repository and the the one in conda-forge.

I'm curious if you'd be open to create a similar PR against https://github.com/conda-forge/staged-recipes and put me as a co-maintainer?

If we also want to keep the recipe in this repo, I think it might be better to have it as a package variant openroad.gui in https://github.com/hdl/conda-eda/tree/master/pnr/openroad (and share the build script/recipe) rather than a totally separate package, what do you think>

@joennlae
Copy link
Author

joennlae commented Aug 1, 2022

@proppy I completely get your point.

  1. How would you approach doing it as a variant instead of a package when they have a different build.sh?

  2. Regarding the conda-forge. I, unfortunately, do not have the capacity to do that atm. I see some hurdles:

  • No official .tar or .zip releases of OpenROAD
  • OpenROAD has a ton of different licenses.
  • I really like the daily builds as the development of OpenROAD is still very fast-paced.

@proppy
Copy link
Contributor

proppy commented Aug 1, 2022

How would you approach doing it as a variant instead of a package when they have a different build.sh?

I think we could have the .gui variant as a subpackage by defining one additional output https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#subpackage-requirements with a different build script (or one that parameterize the existing one, if it's not diverging too much).

No official .tar or .zip releases of OpenROAD

Good point, we'd need to convince them to tag more version; one thing we discussed before with them was to tag things similar to the verible repo: https://github.com/chipsalliance/verible/tags

OpenROAD has a ton of different licenses.

I think that would also be a proble with conda-eda, I was under the impression that all of OpenROAD was under BSD-3 if that's not the case we should update https://github.com/hdl/conda-eda/blob/master/pnr/openroad/meta.yaml#L60

I really like the daily builds as the development of OpenROAD is still very fast-paced.

conda-forge has some automation to bump packages everytime there is a release, see for e.g: magic https://github.com/conda-forge/magic-feedstock/pulls?q=is%3Apr+is%3Aclosed+author%3Aregro-cf-autotick-bot+ but I agree the frequency will be (back to your first point) very depending on how often OpenROAD tag a release.

@joennlae
Copy link
Author

joennlae commented Aug 1, 2022

Ok. If you help me move forward on the conda-forge front. I am happy to support you 👍 Regarding the subpackage. We can do that if you like it more that way.

@proppy
Copy link
Contributor

proppy commented Aug 14, 2022

@joennlae sorry for the late reply.

Ok. If you help me move forward on the conda-forge front. I am happy to support you

Would it be OK for us to re-submit your work here as a package on https://github.com/conda-forge/staged-recipes and add us both as co-maintainers?

+1 Regarding the subpackage. We can do that if you like it more that way.

Let me know if you have any questions, also happy to send a PR against your branch to get this started.

@proppy
Copy link
Contributor

proppy commented Aug 14, 2022

No official .tar or .zip releases of OpenROAD

Good point, we'd need to convince them to tag more version; one thing we discussed before with them was to tag things similar to the verible repo: https://github.com/chipsalliance/verible/tags

Started a discussion around this here:
The-OpenROAD-Project/OpenROAD#2153

pnr/openroad-gui/condarc Outdated Show resolved Hide resolved
@proppy
Copy link
Contributor

proppy commented Feb 21, 2023

/cc @xobs

pnr/openroad-gui/meta.yaml Outdated Show resolved Hide resolved
@proppy proppy force-pushed the add-openroad-gui-public branch from a5a7103 to f882260 Compare February 21, 2023 17:32
@proppy
Copy link
Contributor

proppy commented Feb 21, 2023

ended up adding the GUI to the default package, as @xobs pointed out this is what most folks expect by default. (and we can continue discussion more minimal variants of the package as part of #193)

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

build failing trying to find gl.h

CMake Error at /Users/runner/work/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677010546348/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/lib/cmake/Qt5Gui/Qt5GuiConfigExtras.cmake:9 (message):
    Failed to find "gl.h" in
    "/opt/MacOSX10.11.sdk/System/Library/Frameworks/OpenGL.framework/Headers;/opt/MacOSX10.11.sdk/System/Library/Frameworks/AGL.framework/Headers".
  Call Stack (most recent call first):
    /Users/runner/work/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677010546348/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/lib/cmake/Qt5Gui/Qt5GuiConfig.cmake:171 (include)
    /Users/runner/work/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677010546348/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/lib/cmake/Qt5Widgets/Qt5WidgetsConfig.cmake:88 (find_package)
    /Users/runner/work/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677010546348/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/lib/cmake/Qt5/Qt5Config.cmake:28 (find_package)
    src/gui/CMakeLists.txt:9 (find_package)

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

curious why an older version 5.9.7 of qt get selected rather than the latest one available in https://anaconda.org/main/qt/files.

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

@xobs skipped the ui for now on osx, does this look good to you?

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

Auditing the warning from the error logs:

2023-02-22T04:39:17.3893774Z �[32;1m04:39:17�[0m | Warning: rpath /root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677039184101/_build_env/lib is outside prefix /root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677039184101/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p (removing it)

seems safe to ignore since it's just about cleaning up the rpath.

2023-02-22T04:39:17.3925894Z �[32;1m04:39:17�[0m | WARNING :: Failed to get_static_lib_exports(/root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1677039184101/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/lib/libxcb-xv.a)

safe to ignore per @ajelinski in #165 (comment)

2023-02-22T04:39:17.4155044Z �[32;1m04:39:17�[0m | WARNING (openroad): run-exports library package pkgs/main::readline-8.2-h5eee18b_0 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

should probably remove readline from the host dep.

@xobs
Copy link
Contributor

xobs commented Feb 22, 2023

Looks good to me! I'll test it once it's merged and see how difficult it would be to at least build on my local osx-arm64 machine.

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

found another warning:

2023-02-22T06:33:30.4062078Z �[32;1m06:33:30�[0m | WARNING (openroad,bin/openroad): Needed DSO lib/libfmt.so.8 found in ['fmt']
2023-02-22T06:33:30.4062777Z �[32;1m06:33:30�[0m | WARNING (openroad,bin/openroad): .. but ['fmt'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

the fmt warning is expected since fmt is an header-only library, see

  ignore_run_exports_from:
    # header-only libraries
    - fmt

@proppy
Copy link
Contributor

proppy commented Feb 22, 2023

merging since build are passing on both platforms.

@proppy proppy merged commit d24a72b into hdl:master Feb 22, 2023
@b224hisl
Copy link

Excuse me, I still can't use the gui even I have installed this conda packages
image

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.

4 participants