hdl / conda-eda

Conda recipes for FPGA EDA tools for simulation, synthesis, place and route and bitstream generation.
https://anaconda.org/LiteX-Hub
Apache License 2.0
95 stars 26 forks source link

misc: add openlane #246

Closed proppy closed 2 years ago

proppy commented 2 years ago

Fixes #244

/cc @QuantamHD @mithro

proppy commented 2 years ago

OpenLane UNKNOWN (with mounted scripts from fatal: not a git repository: '$PREFIX/share/openlane/.git')

Maybe we could patch https://github.com/The-OpenROAD-Project/OpenLane/blob/32da932761213af689f10088d671e1e3dc38f273/flow.tcl#L395-L398 so that it include the conda version?

.../openlane/designs/inverter/runs/RUN_2022.09.27_15.06.12/results/signoff/inverter.klayout.gds' wasn't found. Skipping GDS XOR.

We should probably set RUN_KLAYOUT_XOR=0 in misc/openlane/disable-missing-tools.tcl so that it get skilled: https://github.com/The-OpenROAD-Project/OpenLane/blob/32da932761213af689f10088d671e1e3dc38f273/flow.tcl#L151

ajelinski commented 2 years ago

Maybe we could patch https://github.com/The-OpenROAD-Project/OpenLane/blob/32da932761213af689f10088d671e1e3dc38f273/flow.tcl#L395-L398 so that it include the conda version?

Perhaps adding echo $PKG_VERSION >$PREFIX/share/openlane/install/installed_version to build.sh could be enough thanks to: https://github.com/The-OpenROAD-Project/OpenLane/blob/32da932761213af689f10088d671e1e3dc38f273/flow.tcl#L381

proppy commented 2 years ago

Perhaps adding echo $PKG_VERSION >$PREFIX/share/openlane/install/installed_version to build.sh could be

Yes figured this out too with https://github.com/hdl/conda-eda/pull/246/commits/ac1610246abca451b540dc649fa9186876c6a266 but I think we also need https://github.com/The-OpenROAD-Project/OpenLane/pull/1439.

proppy commented 2 years ago

@ajelinski what do you think of bundling tcllib like I'm doing in https://github.com/proppy/conda-eda/tree/master/misc/tcllib ? would you prefer to have this as a separate PR?

ajelinski commented 2 years ago

@ajelinski what do you think of bundling tcllib like I'm doing in https://github.com/proppy/conda-eda/tree/master/misc/tcllib ? would you prefer to have this as a separate PR?

I have no problem with including it in this PR as such.

I don't know tcl very well, is tcllib OS-independent? Shouldn't it be actually built with conda-build?

proppy commented 2 years ago

I don't know tcl very well, is tcllib OS-independent? Shouldn't it be actually built with conda-build?

There is actually a conda-forge package we could backport: https://github.com/conda-forge/tcllib-feedstock

Looking at the underlying installer it seems that it's mostly copying files around: https://core.tcl-lang.org/tcllib/file?name=installer.tcl&ci=tip so I'm not sure why it's not noarch https://anaconda.org/conda-forge/tcllib/files

ajelinski commented 2 years ago

There is actually a conda-forge package we could backport: https://github.com/conda-forge/tcllib-feedstock

Looking at the underlying installer it seems that it's mostly copying files around: https://core.tcl-lang.org/tcllib/file?name=installer.tcl&ci=tip so I'm not sure why it's not noarch https://anaconda.org/conda-forge/tcllib/files

I'll "build" and upload the tcllib package from a branch then and move the package on the litex-hub channel to main. IMO adding it to the repository to have it built nightly doesn't make sense in such a case especially that the source doesn't change at all.

Perhaps it isn't noarch because it has the tk package in run requirements? Not sure but maybe let's keep it as it is especially that we only need the package for Linux, WDYT?

proppy commented 2 years ago

I'll "build" and upload the tcllib package from a branch then and move the package on the litex-hub channel to main.

Oh I didn't realize you had upload write to anaconda main! I'll keep this in mind for other commonly used package that are only in conda-forge.

Perhaps it isn't noarch because it has the tk package in run requirements?

Ah that would make sense, looking at the various module documentation it seems that only only few package depends on Tk: https://github.com/tcltk/tcllib/search?q=%22%5Brequire+Tk%22+titledesc:

Looking at the installer it seems that there is a +excluded flag https://github.com/tcltk/tcllib/blob/efe4404da89b4bd7adafeadd4106a1a4592492ab/installer.tcl#L474 we could leverage to remove them (or we could patch out the part that depends on tk). I'm not familiar with the main requirement and that might be moot since tk looks already well packaged: https://anaconda.org/main/tk

ajelinski commented 2 years ago

Oh I didn't realize you had upload write to anaconda main! I'll keep this in mind for other commonly used package that are only in conda-forge.

Oh no, I'm not that powerful. :-) I only meant moving the package from the ci-<branch>-<run_id> to the main Conda label. It only makes the package built by CI from a conda-eda branch "visible" without providing the label name (because main is the default label).

Ah that would make sense, looking at the various module documentation it seems that only only few package depends on Tk: https://github.com/tcltk/tcllib/search?q=%22%5Brequire+Tk%22+titledesc:

* map

* comm

* tepam

* math

Looking at the installer it seems that there is a +excluded flag https://github.com/tcltk/tcllib/blob/efe4404da89b4bd7adafeadd4106a1a4592492ab/installer.tcl#L474 we could leverage to remove them (or we could patch out the part that depends on tk). I'm not familiar with the main requirement and that might be moot since tk looks already well packaged: https://anaconda.org/main/tk

Is having a noarch package worth such a struggle?

I've just made the tcllib package for Linux available on the LiteX-Hub channel so please test it and modify this package to use it.

proppy commented 2 years ago

Is having a noarch package worth such a struggle?

Not really, but it also allow us to remove the transitive dependencies on tk for openlane :)

proppy commented 2 years ago

I've just made the tcllib package for Linux available on the LiteX-Hub channel so please test it and modify this package to use it.

Done.

proppy commented 2 years ago

Not really, but it also allow us to remove the transitive dependencies on tk for openlane :)

Filed https://github.com/hdl/conda-eda/issues/250#issuecomment-1284136411 so that we can explore this separatly.

proppy commented 2 years ago

squash the commits.

Done.

ajelinski commented 2 years ago

Unfortunately the build failed -- there's something wrong with the patch @proppy

proppy commented 2 years ago

Unfortunately the build failed -- there's something wrong with the patch @proppy

refreshed the patch

proppy commented 2 years ago

@ajelinski curious why the CI still fails?

ajelinski commented 2 years ago

The patch was merged upstream yesterday. I've just merged and removed the patch. :)