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.

Sign up for GitHub

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

Jump to bottom

Add constructor based installer #247

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add constructor based installer #247

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f82700
misc: add openlane
proppy Sep 27, 2022
89a4136
installer: add constructor for openlane.sky130a
proppy Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .github/workflows/Construct.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: Build installers
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).


on:
workflow_call:

permissions:
contents: write

defaults:
run:
shell: bash

jobs:
openlane-sky130a-installer:
runs-on: "ubuntu-20.04"
steps:
- name: checkout conda-eda
uses: actions/checkout@v3
with:
fetch-depth: 0 # fetch all tags

- name: construct installer
run: |
export CI_SCRIPTS_PATH="$(pwd)/ci"
./ci/install.sh
conda install constructor
constructor installer/openlane.sky130a/

- name: archive openlane.sky130a installer
uses: actions/upload-artifact@v3
with:
name: "openlane-sky130a-installer"
path: |
/home/runner/work/conda-eda/conda-eda/openlane-sky130a-0-Linux-x86_64.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there any variable available with this path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I have no preference on the two.


- run: |
git config --local user.name conda-eda@bot
git config --local user.email conda-eda@bot
TAG=$(git describe --tags)
git tag $TAG
git push origin "HEAD:refs/tags/$TAG"
echo "TAG=$TAG" >> $GITHUB_ENV
Comment on lines +39 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

I've just realized there are two issues here:

  1. 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-...
  2. There's no check if the tag already exists and the workflow will be triggered every day (with Upload.yml nightly runs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


- uses: softprops/action-gh-release@v1
with:
name: ${{ env.TAG }}
tag_name: ${{ env.TAG }}
files: |
openlane-sky130a-installer/*.sh
6 changes: 6 additions & 0 deletions .github/workflows/Upload.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ jobs:
ANACONDA_USER: ${{ secrets.ANACONDA_USER }}


Construct:
needs: Upload
if: always() && (github.event_name != 'pull_request') && (github.ref == 'refs/heads/master')
uses: ./.github/workflows/Construct.yml


Cleanup:
runs-on: ubuntu-20.04
steps:
Expand Down
15 changes: 15 additions & 0 deletions installer/openlane.sky130a/construct.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: openlane-sky130a
version: 0

channels:
- https://conda.anaconda.org/litex-hub
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

- http://repo.anaconda.com/pkgs/main/

specs:
- python 3.7*
- conda
- openlane
- open_pdks.sky130a
- xls

post_install: post_install.sh
9 changes: 9 additions & 0 deletions installer/openlane.sky130a/post_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash

set -ex

cat >> $PREFIX/share/openlane/install/env.tcl <<EOF
set ::env(PDK) "sky130A"
set ::env(STD_CELL_LIBRARY) "sky130_fd_sc_hd"
set ::env(STD_CELL_LIBRARY_OPT) "sky130_fd_sc_hd"
EOF