Add constructor based installer #247

proppy · 2022-09-27T15:25:15Z

Depends on #246
Fixes #245

Test with:

curl -L -O https://github.com/proppy/conda-eda/releases/download/v0.0-1358-g4ea08b3/openlane-sky130a-0-Linux-x86_64.sh
bash openlane-sky130a-0-Linux-x86_64.sh -b -p silicon-env/
source silicon-env/bin/activate 
flow.tcl -design inverter

/cc @QuantamHD @umarcor @mithro

mithro · 2022-10-10T00:37:05Z

@PiotrZierhoffer / @umarcor - Could you please review? I think it looks fine and thus we should merge but are unsure about the failures?

mithro · 2022-10-15T03:44:00Z

@proppy - Any idea why things are failing in the CI?

proppy · 2022-10-15T03:45:45Z

seems unrelated, nightly CI has similar failure: https://github.com/hdl/conda-eda/actions/runs/3253556045

proppy · 2022-10-15T03:46:27Z

but we can get #246 reviewed indepently if it eases the process.

ajelinski

I'm genuinely impressed by the whole idea and Conda construct itself as I didn't know the tool.

I tested using the installer and unfortunately stumbled upon such an error:

error copying "/root/silicon-env/share/openlane/designs/inverter/runs/RUN_2022.10.18_13.16.56/results/signoff/inverter.magic.gds": no such file or directory
    while executing
"file copy -force $::env(MAGIC_GDS) $::env(CURRENT_GDS)"
    (procedure "run_magic" line 21)
    invoked from within
"run_magic"
    (procedure "run_magic_step" line 3)
    invoked from within
"[lindex $step_exe 0] [lindex $step_exe 1] "
    (procedure "run_non_interactive_mode" line 52)
    invoked from within
"run_non_interactive_mode {*}$argv"
    (file "/root/silicon-env/share/openlane/flow.tcl" line 415)
    invoked from within
"source "$::env(CONDA_PREFIX)/share/openlane/flow.tcl""
    (file "/root/silicon-env/bin/flow.tcl" line 3)

but perhaps it'll get fixed by disabling this klayout xor?

What bothers me a little is that we'll have a tag added for each run of the Construct workflow, i.e., with each Upload workflow run from master. Do I get it right? Isn't there any other option to publish the installer?

ajelinski · 2022-10-18T13:11:58Z

.github/workflows/Construct.yml

+        with:
+          name: "openlane-sky130a-installer"
+          path: |
+            /home/runner/work/conda-eda/conda-eda/openlane-sky130a-0-Linux-x86_64.sh


Isn't there any variable available with this path?

I think that would be ${{ env.GITHUB_WORKSPACE }}/openlane-sky130a-0-Linux-x86_64.sh

Using openlane-sky130a-0-Linux-x86_64.sh should also work. Since actions/checkout@v3 is used, the default location for other steps is the root of the repo already.

FWIW I have no preference on the two.

proppy · 2022-10-18T13:41:27Z

I tested using the installer and unfortunately stumbled upon such an error:

Can you share the full log? I also have a more recent version of the installer here https://github.com/proppy/conda-eda/releases/tag/v0.0-1418-g170b751

What bothers me a little is that we'll have a tag added for each run of the Construct workflow, i.e., with each Upload workflow run from master. Do I get it right? Isn't there any other option to publish the installer?

I couldn't think of something else if we want to publish the installer as a GitHub release, another possibility it to move them on GCS in timestamped directory, that's something I think would also make sense for packages (see #204).

ajelinski · 2022-10-18T13:50:42Z

Log: openlane.log

umarcor · 2022-10-18T15:00:19Z

.github/workflows/Construct.yml

@@ -0,0 +1,49 @@
+name: Build installers


Why is this a separated workflow instead of an additional job in Build?

It's actually an additional Job in Upload, see https://github.com/hdl/conda-eda/pull/247/files/4f827004e8068de55e99f6cdc7ad8f625ff07326..89a4136869d5488f40e5f7bcc0138e0ebb461638#diff-1ad29a53f212151bde5f56f9e784ef9ea58dbc5ba9df201cb0c26fa2c6116c87

Not the same. Construct is a new workflow, which is called as a reusable workflow from Upload. I mean having it as an additional job in workflow Build, without creating a new (reusable) workflow.
The Build workflow might be executed directly, not as a reusable workflow, which would ignore the recipes/jobs defined in Construct. However, if you put this job in Build, then it will always be run together with other recipes.

Not the same. Construct is a new workflow, which is called as a reusable workflow from Upload. I mean having it as an additional job in workflow Build, without creating a new (reusable) workflow. The Build workflow might be executed directly, not as a reusable workflow, which would ignore the recipes/jobs defined in Construct. However, if you put this job in Build, then it will always be run together with other recipes.

Good point that it shouldn't be in fact run from Upload but from Build. Perhaps you could move the Construct job then @proppy and use needs: "openlane-linux" for it? It'd be also nice to have openlane.sky130a parametrized in Construct.yml so that it's reusable if we ever want to have more installers constructed.

The Build workflow might be executed directly, not as a reusable workflow, which would ignore the recipes/jobs defined in Construct. However, if you put this job in Build, then it will always be run together with other recipes.

My thinking that whe only want to "release" the installer when we actually publish the packages (so w/ that logic the same way we don't run Upload when we run Build independently, we couldn't run Construct).

umarcor · 2022-10-18T15:01:23Z

installer/openlane.sky130a/construct.yaml

+version: 0
+
+channels:
+  - https://conda.anaconda.org/litex-hub


litex-hub alone should also work, isn't it?

Since all the constructor example were using fully qualified URLs: https://github.com/conda/constructor/search?q=conda.anaconda.org I assumed it was best-practice.

It seems channel names are enough for other channels than main indeed: https://github.com/restlessronin/lila-deep/blob/415fc7172c49e0a1ae766c7c9331d0d85645ef33/.colab/construct.yaml

Though I found no construct.yaml with plain main on GH (https://github.com/search?q=main+extension%3Ayaml+filename%3Aconstruct.yaml&type=Code) so I have no preference on full URLs vs channel names.

ajelinski · 2022-10-19T11:12:27Z

.github/workflows/Construct.yml

+          TAG=$(git describe --tags)
+          git tag $TAG
+          git push origin "HEAD:refs/tags/$TAG"
+          echo "TAG=$TAG" >> $GITHUB_ENV


I've just realized there are two issues here:

git describe --tags will use tags from previous runs of this code in Construct.yml so pretty soon we'll have tags like: v0.0-1337-gc24a15291-X-gSHA-X-gSHA-X-gSHA-X-gSHA-...

There's no check if the tag already exists and the workflow will be triggered every day (with Upload.yml nightly runs).

Oh I did had this issue, and handled it by removing --tags from the git describe command

You're right it will overwrite the existing release with a new build even if there is no change in conda-eda which could be unfortunate if there are some upstream change but no conda-eda commit.

proppy · 2022-11-30T17:37:49Z

@PiotrZierhoffer, @umarcor would it make sense to setup a dedicated bucket for those installer artifacts so that we can get rid of the noise that automated tagging would add to the repo (as @ajelinski pointed in #247 (review))

proppy added 2 commits September 28, 2022 00:00

misc: add openlane

4f82700

installer: add constructor for openlane.sky130a

89a4136

proppy mentioned this pull request Oct 5, 2022

I use the Local Installer The-OpenROAD-Project/OpenLane#1300

Open

mithro requested review from umarcor and PiotrZierhoffer October 10, 2022 00:36

mithro changed the title ~~add constructor based installer~~ Add constructor based installer Oct 10, 2022

ajelinski reviewed Oct 18, 2022

View reviewed changes

umarcor reviewed Oct 18, 2022

View reviewed changes

ajelinski reviewed Oct 19, 2022

View reviewed changes

proppy mentioned this pull request Nov 14, 2022

add klayout conda-forge/staged-recipes#20396

Closed

10 tasks

proppy mentioned this pull request Nov 21, 2022

python modules not working in python 3.10 The-OpenROAD-Project/OpenROAD#2535

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add constructor based installer #247

Add constructor based installer #247

proppy commented Sep 27, 2022 •

edited

Loading

mithro commented Oct 10, 2022

mithro commented Oct 15, 2022

proppy commented Oct 15, 2022

proppy commented Oct 15, 2022

ajelinski left a comment

ajelinski Oct 18, 2022

proppy Oct 18, 2022

umarcor Oct 18, 2022

ajelinski Oct 19, 2022

proppy commented Oct 18, 2022

ajelinski commented Oct 18, 2022

umarcor Oct 18, 2022

proppy Oct 18, 2022

umarcor Oct 18, 2022

ajelinski Oct 19, 2022

proppy Oct 19, 2022

umarcor Oct 18, 2022

proppy Oct 18, 2022

ajelinski Oct 19, 2022

ajelinski Oct 19, 2022

proppy Oct 19, 2022

proppy commented Nov 30, 2022 •

edited

Loading

Add constructor based installer #247

Are you sure you want to change the base?

Add constructor based installer #247

Conversation

proppy commented Sep 27, 2022 • edited Loading

mithro commented Oct 10, 2022

mithro commented Oct 15, 2022

proppy commented Oct 15, 2022

proppy commented Oct 15, 2022

ajelinski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy commented Oct 18, 2022

ajelinski commented Oct 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy commented Nov 30, 2022 • edited Loading

proppy commented Sep 27, 2022 •

edited

Loading

proppy commented Nov 30, 2022 •

edited

Loading