[Reproducible rosetta build] Adds support for building rosetta with local patches and an already generated patch dir #354

terrykong · 2023-11-01T07:04:08Z

flowchart TD
    B -->|./extract-patches.sh| PG1["./patches/patchlist.txt.gen"] & PG2["./patches/*.patches"]
    PO[patchlist.txt] -->|copy| PI
    subgraph main build
    direction LR
    PI[patchlist.txt]-->|--build-arg|B[docker buildx]
    PG1
    PG2
    end

    PG1 --> |copy|PG11["./patches/patchlist.txt.gen"] 
    PG2 --> |copy|PG22["./patches/*.patches"]
    PG11 --> |copy| PG111
    PG22 --> |copy| PG222

    subgraph reproducible build
    direction LR
    PG111["./patches/patchlist.txt.gen"] --> |--build-arg| B2
    PG222["./patches/*.patches"] --> |copy| B2
    B2[docker buildx]
    end

This introduces these new mechanisms to the rosetta build:

Building with local patches
Generating local patches for reproducibility
Add a --build-arg for specifying a different patchlist file
Save patches as github artifacts

The above is a high level flow of how the build can be reproduced after this PR. We can now build an image from an existing image or existing patchlists by doing:

scripts/extract-patches.sh <img-name>  # Extracts generated patch dir under ./patches/
docker buildx build --build-arg T5X_PATCHLIST=patches/t5x/patchlist-t5x.txt.gen --build-arg FLAX_PATCHLIST=patches/flax/patchlist-flax.txt.gen --target rosetta --tag rosetta:latest -f Dockerfile.t5x .

and this should yield an identical distribution.

To create local patches, this change updates git cherry-pick calls to git format-patch + git am to preserve metadata in patch files.

note: This will not guarantee that the pip packages will be reproducible as that will need to be handled by a different PR.

update rosetta workflows with new mealkit/final mechanic
update ci.yaml

Co-authored-by: Terry Kong <[email protected]>

terrykong · 2023-11-01T17:21:40Z

Manual CI run:

~~https://github.com/NVIDIA/JAX-Toolbox/actions/runs/6722919706~~ (failed b/c airio version captured in 11/01 nightly is broken)
~~https://github.com/NVIDIA/JAX-Toolbox/actions/runs/6724117747~~ (failed b/c caught dali merge conflict)
pax:
- https://github.com/NVIDIA/JAX-Toolbox/actions/runs/6774104444
  - example generated patches
t5x
- https://github.com/NVIDIA/JAX-Toolbox/actions/runs/6774106221
  - example generated patches

terrykong · 2023-11-13T17:50:03Z

Converting to draft in light of #371 . Will rebase this on top of that branch and add this feature

generated patch dir

the conflict

terrykong · 2023-11-14T00:43:17Z

Testing using the mealkit images from the CI to trigger rosetta CI:

t5x:
- ghcr.io/nvidia/jax-toolbox-internal:7008474026-upstream-t5x (w/o suffix)
- ~~https://github.com/NVIDIA/JAX-Toolbox/actions/runs/6871878648~~ (before pip-compile in main)
- https://github.com/NVIDIA/JAX-Toolbox/actions/runs/7009488013
pax
- ghcr.io/nvidia/jax-toolbox-internal:7008474026-upstream-pax (w/o suffix)
- rosetta-ci:
  - ~~https://github.com/NVIDIA/JAX-Toolbox/actions/runs/6951489840~~
  - https://github.com/NVIDIA/JAX-Toolbox/actions/runs/7009490306

yhtang · 2023-11-13T15:17:10Z

.github/workflows/_build_rosetta.yaml

@@ -95,6 +95,15 @@ jobs:
          build-args: |
            BASE_IMAGE=${{ steps.defaults.outputs.BASE_IMAGE }}

+      - name: Extract patches
+        run: rosetta/scripts/extract-patches.sh ${{ steps.meta.outputs.tags }}


Could we specify the name of the output patch file via an argument? Currently, we can only infer it from the artifact upload step below.

This line pulls all of the patches, not just one. The dir copied out is pulled from a predetermined path /opt/rosetta/patches. I don't foresee any reason to configure this

yhtang · 2023-11-28T09:15:13Z

.github/workflows/nightly-rosetta-t5x-build-test.yaml

+  # TODO: ARM
+  publish-build-badge:
+    needs: [metadata, amd64, arm64]
+    uses: ./.github/workflows/_publish_badge.yaml


Note that _publish_badge.yaml will be deprecated soon in favor of the sitrep system.

yhtang · 2023-11-28T12:59:40Z

.github/workflows/nightly-rosetta-pax-build.yaml

@@ -100,6 +95,29 @@ jobs:
      BASE_IMAGE: ${{ needs.metadata.outputs.BASE_IMAGE_ARM64 }}
    secrets: inherit

+  publish-build-badge:
+    needs: [metadata, amd64, arm64]
+    uses: ./.github/workflows/_publish_badge.yaml


Note that _publish_badge.yaml will be deprecated soon.

yhtang · 2023-11-28T13:09:10Z

.github/workflows/nightly-rosetta-pax-build.yaml

@@ -123,49 +150,45 @@ jobs:
        ${{ needs.arm64.outputs.DOCKER_TAG_FINAL }}
      TARGET_IMAGE: pax
      TARGET_TAGS: |
-        type=raw,value=latest,priority=1000
-        type=raw,value=nightly-${{ needs.metadata.outputs.BUILD_DATE }},priority=900  
+        ${{ needs.test-amd64.outputs.TEST_STATUS == 'success' && 'type=raw,value=latest,priority=1000' || '' }}


I'd recommend to put this logic in a 'metadata' job/jobstep, and then reference the job/jobstep output for better readability.

This depends on test results. metadata is an ancestor of the test step.

yhtang · 2023-11-28T13:09:42Z

.github/workflows/nightly-rosetta-pax-build.yaml

-    needs: [metadata, amd64, arm64, test-pax]
+  # TODO: ARM Tests
+  publish-test-badge:
+    needs: [metadata, publish-build-badge, test-amd64]
    uses: ./.github/workflows/_publish_badge.yaml


Note that `_publish_badge.yaml`` will be deprecated soon.

Would it be okay to tackle this in a subsequent PR? My thinking is we can get the reproducible build/ci changes out and then circle back to sitrep. I expect the reproducible PRs to all be flushed out within a week or two :)

yhtang · 2023-11-28T13:10:28Z

.github/workflows/nightly-rosetta-t5x-build-test.yaml

+        ${{ needs.arm64.outputs.DOCKER_TAG_FINAL }}
+      TARGET_IMAGE: t5x
+      TARGET_TAGS: |
+        ${{ ( needs.test-t5x-amd64.outputs.TEST_STATUS == 'success' && needs.test-vit-amd64.outputs.TEST_STATUS == 'success' && needs.test-unit-amd64.outputs.TEST_STATUS == 'success' ) && 'type=raw,value=latest,priority=1000' || '' }}


Same comment regarding the placement of the logic in a separate job/jobstep.

DwarKapex · 2023-11-29T05:24:15Z

rosetta/scripts/extract-patches.sh

+#!/bin/bash
+
+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
+cd $SCRIPT_DIR


Maybe let's not jump into this folder, but use pushd-popd instead?

pushd $SCRIPT_DIR <the rest code> popd

pushd/popd isn't needed here b/c I only enter one directory and this is in a subshell so the calling parent shell doesn't change dirs

terrykong · 2023-11-30T07:45:54Z

@DwarKapex @yhtang
Closing this in favor of #405 since I undid some of the stuff in this PR to accommodate the manifest change.

yhtang and others added 15 commits October 23, 2023 08:37

wip

abb6f97

Co-authored-by: Terry Kong <[email protected]>

fix typo

068aab9

wip

fb7cf0b

wip

4b0c406

wip

4044450

wip

38da8a1

wip

9ad94c8

use full clone

1aac03a

update pip-tools script

42ec9bb

update pip-tools script

2f02023

update pip-tools script

ad12e78

fix t5x dockerfile

bdc34c8

fix t5x dockerfile

a5c478e

test flax hack

b4fd2d8

flax hack

fe6e3d8

terrykong marked this pull request as draft November 1, 2023 07:04

yhtang added 13 commits November 2, 2023 18:26

hack for git top of tree Flax dependency

ad83f21

update URL req

b7e1a6c

update

0dd9617

editability

dc44fe4

editability

acf2acf

editability

027cd63

editability

7c10ffa

editability

f1046d2

wip

bbf2c21

wip

cd0d5d1

wip

e973c9a

fix shell

1f6d4f1

fix arg order

6d9ae00

yhtang added 4 commits November 13, 2023 15:11

disable sandbox

602002f

i'm jet-lagged

12a57eb

use Pax image for TE testing

dbaba5b

Fix job dependency

ccafb52

terrykong marked this pull request as draft November 13, 2023 17:49

terrykong added 7 commits November 13, 2023 11:22

Adds support for building rosetta with local patches and an already

279c388

generated patch dir

comment

2b5ee7c

Add steps to archive patches in run

978f3d0

Date the patches for readability

b784727

Better log msg

cc16e92

switch to --3way since that produces a merge conflict to help understand

7cc4abb

the conflict

Switch to mealkit+finalize mechanic for rosetta builds

4a3bd4a

terrykong force-pushed the rosetta-reproducible-patches branch from 8f380cd to 4a3bd4a Compare November 13, 2023 23:30

terrykong changed the base branch from main to add-pip-compile November 13, 2023 23:31

Add github.run_id to artifacts for provenance

f6a446e

terrykong mentioned this pull request Nov 14, 2023

Use pip-compile to help with consistent Python dependency resolution #371

Merged

1 task

Update all rosetta workflows with mealkit/final mechanism

7fffabf

Base automatically changed from add-pip-compile to main November 21, 2023 06:41

terrykong added 5 commits November 21, 2023 16:42

Merge branch 'main' into rosetta-reproducible-patches

71d960c

improve comments

7bf3790

revert sandbox

2a2ca94

merge issues, fixed manually

e977aa2

Merge branch 'main' into rosetta-reproducible-patches

8c23902

terrykong marked this pull request as ready for review November 27, 2023 17:16

yhtang requested changes Nov 28, 2023

View reviewed changes

DwarKapex reviewed Nov 29, 2023

View reviewed changes

terrykong closed this Nov 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reproducible rosetta build] Adds support for building rosetta with local patches and an already generated patch dir #354

[Reproducible rosetta build] Adds support for building rosetta with local patches and an already generated patch dir #354

terrykong commented Nov 1, 2023 •

edited

Loading

terrykong commented Nov 1, 2023 •

edited

Loading

terrykong commented Nov 13, 2023

terrykong commented Nov 14, 2023 •

edited

Loading

yhtang Nov 13, 2023

terrykong Nov 28, 2023

yhtang Nov 28, 2023

yhtang Nov 28, 2023

yhtang Nov 28, 2023

terrykong Nov 28, 2023

yhtang Nov 28, 2023

terrykong Nov 28, 2023

yhtang Nov 28, 2023

DwarKapex Nov 29, 2023

terrykong Nov 29, 2023

terrykong commented Nov 30, 2023

[Reproducible rosetta build] Adds support for building rosetta with local patches and an already generated patch dir #354

[Reproducible rosetta build] Adds support for building rosetta with local patches and an already generated patch dir #354

Conversation

terrykong commented Nov 1, 2023 • edited Loading

terrykong commented Nov 1, 2023 • edited Loading

terrykong commented Nov 13, 2023

terrykong commented Nov 14, 2023 • edited Loading

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

Choose a reason for hiding this comment

terrykong commented Nov 30, 2023

terrykong commented Nov 1, 2023 •

edited

Loading

terrykong commented Nov 1, 2023 •

edited

Loading

terrykong commented Nov 14, 2023 •

edited

Loading