Use HEAD versions in presubmit CI #782
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,8 +21,8 @@ on: | |
| BUMP_MANIFEST: | ||
| type: boolean | ||
| description: Bump git repos in manifest.yaml to head of tree? | ||
| default: true | ||
| required: true | ||
| MERGE_BUMPED_MANIFEST: | ||
| type: boolean | ||
| description: "(used if BUMP_MANIFEST=true) If true: attempt to PR/merge manifest branch" | ||
|
|
@@ -88,7 +88,11 @@ jobs: | |
| id: manifest-branch | ||
| shell: bash -x -e {0} | ||
| run: | | ||
| if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then | ||
| BUMP_MANIFEST="${{ inputs.BUMP_MANIFEST }}" | ||
| else | ||
| BUMP_MANIFEST="true" | ||
| fi | ||
| MERGE_BUMPED_MANIFEST=${{ github.event_name == 'schedule' || inputs.MERGE_BUMPED_MANIFEST || 'false' }} | ||
| # Prepend nightly manifest branch with "z" to make it appear at the end | ||
| if [[ "$BUMP_MANIFEST" == "true" ]]; then | ||
|
|
@@ -155,15 +159,15 @@ jobs: | |
| # converts manifest yaml to a json object of {SOFTWARE_NAME: URL#REF, ...} | ||
| urlrefs=$( | ||
| cat .github/container/manifest.yaml |\ | ||
| yq -o=json 'to_entries | .[] | select(.value.mode == "git-clone") | {( .key | upcase | sub("-", "_") ): "URLREF_" + ( .key | upcase | sub("-", "_") ) + "=" + .value.url + "#" + .value.commit}' |\ | ||
| jq -c -s 'add' | ||
| ) | ||
| # SOURCE_OVERRIDES is a comma-separated list of package=urlref pairs | ||
| IFS=, read -ra overrides <<< "${{ inputs.SOURCE_OVERRIDES }}" | ||
| for override in "${overrides[@]}"; do | ||
| PACKAGE=$(cut -d= -f 1 <<< "${override}" | tr '[:lower:]' '[:upper:]' | tr '-' '_') | ||
| URLREF=$(cut -d= -f 2- <<< "${override}") | ||
| urlrefs=$(echo "$urlrefs" | jq -c ". + {\"$PACKAGE\": \"URLREF_${PACKAGE}=$URLREF\"}") | ||
|
Comment on lines
165
to
+170
There was a problem hiding this comment. Hey @olupton , can you walk me through the idea of how manifest.yaml and If I want to install FLAX on an older commit, I have two options: I can change the manifest.yaml, or use There was a problem hiding this comment. I think the idea here is that there are two modes. First, when you're using this machinery to test some feature or fix, you don't particularly want to commit/push to JAX-Toolbox, so you can inject your feature/fix branch using Second, when you are preparing a release, you gradually want to converge on a fixed and reproducible set of versions. In that case, you would take some seed Ultimately I don't see why you would want to mix the two modes, and the second one ought to be pedantic. In mode one, you default to taking the HEAD and recording what that was. In mode two, forgetting to specify a version of something should maybe be an error. |
||
| done | ||
| echo "SOURCE_URLREFS=${urlrefs}" >> $GITHUB_OUTPUT | ||
|
|
||
|
Comment on lines
172
to
173
There was a problem hiding this comment.
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong, but you suggest to use default values for jax
ref#commitfrom Dockerfile rather than from manifest file?What is the scenario for NGC release? Update Dockerfiles accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely yes for the normal CI: by default we take the Dockerfile defaults, which is basically "HEAD".
When preparing releases, one option (noted here: #782 (comment)) is to commit a manifest file to the release branch and use that. Your suggestion of changing the default values in the Docker files in the release branches would also work, I think, and would require less machinery (to translate manifest --> URLREF build arg) but might be a little more tedious to seed at the beginning of the release process.
cc: @terrykong @ko3n1g