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.

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

[Master] Patch missing type shapes #16154

Merged
merged 30 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4de35ad
Patch missing type shapes
dkijania Sep 11, 2024
36829c6
use soft_fail in VerisionLint dhall
dkijania Sep 16, 2024
72c6d04
move gsutil upload to docker
dkijania Sep 18, 2024
dd8dca7
dhall make lint
dkijania Sep 18, 2024
7530ac4
do not copy debian key
dkijania Sep 18, 2024
ccf827d
fix path to key.json
dkijania Sep 18, 2024
42270d3
upload file after revert
dkijania Sep 18, 2024
33dedec
remove no-merges from git log and patch version linter for base branc…
dkijania Sep 18, 2024
f6f8e3d
add remote when evaluating commit
dkijania Sep 19, 2024
4f16f98
fix variable checking
dkijania Sep 19, 2024
b027e32
Revert unnecessary changes
dkijania Sep 30, 2024
523986d
more revert
dkijania Sep 30, 2024
cb9eebb
fix typo and use shorter BUILDKITE_COMMIT when looking for reference …
dkijania Sep 30, 2024
1e4eec5
add requests import to version-linter.py
dkijania Sep 30, 2024
93fa787
remove duplicated check for mina-type-shape which should be assured i…
dkijania Oct 2, 2024
64a960e
removing (faulty) code which tries to find ref file on last 10 commits
dkijania Oct 3, 2024
2472c2f
rename function to signal only one commit will be returned
dkijania Oct 4, 2024
33cfd01
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 8, 2024
eb7ccc2
clean up code. remove unused functions
dkijania Oct 14, 2024
7f85c14
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 15, 2024
11362e7
strip string when calculating commit
dkijania Oct 15, 2024
2b73046
Merge remote-tracking branch 'origin/dkijania/generate_version_lint_i…
dkijania Oct 15, 2024
2de6151
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 16, 2024
59e6ccc
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 21, 2024
182467c
Revert "Auxiliary commit to revert individual files from b027e324d6ff…
dkijania Oct 21, 2024
32b6d92
revert Lib/Cmds.dhall change
dkijania Oct 21, 2024
32aec5c
bring back secrets volume
dkijania Oct 21, 2024
a43a260
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 28, 2024
9aa8585
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 28, 2024
8160eb9
Merge branch 'master' into dkijania/generate_version_lint_inafly_master
dkijania Oct 28, 2024
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
2 changes: 2 additions & 0 deletions buildkite/scripts/dump-mina-type-shapes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ export TYPE_SHAPE_FILE=${MINA_COMMIT_SHA1}-type_shape.txt

echo "--- Create type shapes git note for commit: ${MINA_COMMIT_SHA1}"
mina internal dump-type-shapes > ${TYPE_SHAPE_FILE}

source buildkite/scripts/gsutil-upload.sh ${TYPE_SHAPE_FILE} gs://mina-type-shapes
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 now necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

We did an effort recently to setup agents in a way that now env vars which allows to correctly setup gsutil are passed to toolchain and allow us to upload file to gs directly from toolchain. Previously we had to ran gsutil directly from agent which was a bit of dance (dumping file on toolchains, then uploading on agents level).

In VersionLinter.dhall i removed line with upload to gcloud bucket

11 changes: 11 additions & 0 deletions buildkite/scripts/gsutil-upload.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

KEY_FILE=/var/secrets/google/key.json

if [ ! -f $KEY_FILE ]; then
echo "Cannot use gsutil for upload as key file cannot be foud in $KEY_FILE"
fi

gcloud auth activate-service-account --key-file=$KEY_FILE

gsutil cp $1 $2
50 changes: 50 additions & 0 deletions buildkite/scripts/version-linter-patch-missing-type-shapes.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/bin/bash

set -eox pipefail

if [[ $# -ne 1 ]]; then
echo "Usage: $0 <release-branch>"
exit 1
fi

git config --global --add safe.directory /workdir

source buildkite/scripts/handle-fork.sh
source buildkite/scripts/export-git-env-vars.sh

release_branch=${REMOTE}/$1
mrmr1993 marked this conversation as resolved.
Show resolved Hide resolved

RELEASE_BRANCH_COMMIT=$(git log -n 1 --format="%h" --abbrev=7 $release_branch)

function revert_checkout() {
git checkout $BUILDKITE_COMMIT
git submodule sync
git submodule update --init --recursive
}

function checkout_and_dump() {
local __commit=$1
git checkout $__commit
git submodule sync
git submodule update --init --recursive
eval $(opam config env)
TYPE_SHAPE_FILE=${__commit:0:7}-type_shape.txt
dune exec src/app/cli/src/mina.exe internal dump-type-shapes > /tmp/${TYPE_SHAPE_FILE}
revert_checkout
source buildkite/scripts/gsutil-upload.sh /tmp/${TYPE_SHAPE_FILE} gs://mina-type-shapes
}

if ! $(gsutil ls gs://mina-type-shapes/$BUILDKITE_COMMIT 2>/dev/null); then
checkout_and_dump $BUILDKITE_COMMIT
fi

if ! $(gsutil ls gs://mina-type-shapes/$RELEASE_BRANCH_COMMIT 2>/dev/null); then
checkout_and_dump $RELEASE_BRANCH_COMMIT
fi

if [[ -n "${BUILDKITE_PULL_REQUEST_BASE_BRANCH:-}" ]]; then
BUILDKITE_PULL_REQUEST_BASE_BRANCH_COMMIT=$(git log -n 1 --format="%h" --abbrev=7 ${REMOTE}/${BUILDKITE_PULL_REQUEST_BASE_BRANCH} )
if ! $(gsutil ls gs://mina-type-shapes/$BUILDKITE_PULL_REQUEST_BASE_BRANCH_COMMIT 2>/dev/null); then
checkout_and_dump $BUILDKITE_PULL_REQUEST_BASE_BRANCH_COMMIT
fi
fi
14 changes: 6 additions & 8 deletions buildkite/src/Jobs/Test/VersionLint.dhall
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
let Cmd = ../../Lib/Cmds.dhall

let S = ../../Lib/SelectFiles.dhall

let B = ../../External/Buildkite.dhall
Expand All @@ -14,12 +12,12 @@ let JobSpec = ../../Pipeline/JobSpec.dhall

let Command = ../../Command/Base.dhall

let RunInToolchain = ../../Command/RunInToolchain.dhall

let Docker = ../../Command/Docker/Type.dhall

let Size = ../../Command/Size.dhall

let RunInToolchain = ../../Command/RunInToolchain.dhall
Copy link
Member

Choose a reason for hiding this comment

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

Why move this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing


let dependsOn = [ { name = "MinaArtifactBullseye", key = "build-deb-pkg" } ]

let buildTestCmd
Expand All @@ -34,18 +32,18 @@ let buildTestCmd
RunInToolchain.runInToolchain
([] : List Text)
"buildkite/scripts/dump-mina-type-shapes.sh"
# [ Cmd.run
"gsutil cp \$(git log -n 1 --format=%h --abbrev=7 --no-merges)-type_shape.txt \$MINA_TYPE_SHAPE gs://mina-type-shapes"
]
# RunInToolchain.runInToolchain
([] : List Text)
"buildkite/scripts/version-linter-patch-missing-type-shapes.sh ${release_branch}"
# RunInToolchain.runInToolchain
([] : List Text)
"buildkite/scripts/version-linter.sh ${release_branch}"
, label = "Versioned type linter for ${release_branch}"
, key = "version-linter-${release_branch}"
, soft_fail = Some soft_fail
Copy link
Member

Choose a reason for hiding this comment

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

Why move this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Issues with merging. Removing

, target = cmd_target
, docker = None Docker.Type
, depends_on = dependsOn
, soft_fail = Some soft_fail
, artifact_paths = [ S.contains "core_dumps/*" ]
}

Expand Down
26 changes: 19 additions & 7 deletions buildkite/src/Lib/Cmds.dhall
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ let module =
\(environment : List Text)
-> let Docker =
{ Type =
{ image : Text, extraEnv : List Text, privileged : Bool }
, default = { extraEnv = [] : List Text, privileged = False }
{ image : Text
, extraEnv : List Text
, privileged : Bool
, useBash : Bool
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be unused in this PR, and generally orthogonal to the changes. Can you remove this?

}
, default =
{ extraEnv = [] : List Text
, privileged = False
, useBash = True
}
}

let Cmd = { line : Text, readable : Optional Text }
Expand Down Expand Up @@ -58,12 +66,16 @@ let module =
: Text
= "/var/buildkite/shared"

let entrypoint
: Text
= if docker.useBash then "/bin/bash" else "/bin/sh"

in { line =
"docker run -it --rm --entrypoint /bin/sh --init --volume ${sharedDir}:/shared --volume ${outerDir}:/workdir --workdir /workdir${envVars}${ if docker.privileged
"docker run -it --rm --entrypoint ${entrypoint} --init --volume /var/secrets:/var/secrets --volume ${sharedDir}:/shared --volume ${outerDir}:/workdir --workdir /workdir${envVars}${ if docker.privileged

then " --privileged"
then " --privileged"

else ""} ${docker.image} -c '${inner.line}'"
else ""} ${docker.image} -c '${inner.line}'"
, readable =
Optional/map
Text
Expand Down Expand Up @@ -130,7 +142,7 @@ let tests =
let dockerExample =
assert
: { line =
"docker run -it --rm --entrypoint /bin/sh --init --volume /var/buildkite/shared:/shared --volume \\\$BUILDKITE_BUILD_CHECKOUT_PATH:/workdir --workdir /workdir --env ENV1 --env ENV2 --env TEST foo/bar:tag -c 'echo hello'"
"docker run -it --rm --entrypoint /bin/bash --init --volume /var/secrets:/var/secrets --volume /var/buildkite/shared:/shared --volume \\\$BUILDKITE_BUILD_CHECKOUT_PATH:/workdir --workdir /workdir --env ENV1 --env ENV2 --env TEST foo/bar:tag -c 'echo hello'"
, readable = Some "Docker@foo/bar:tag ( echo hello )"
}
=== M.inDocker
Expand All @@ -142,7 +154,7 @@ let tests =

let cacheExample =
assert
: "./buildkite/scripts/cache-through.sh data.tar \"docker run -it --rm --entrypoint /bin/sh --init --volume /var/buildkite/shared:/shared --volume \\\$BUILDKITE_BUILD_CHECKOUT_PATH:/workdir --workdir /workdir --env ENV1 --env ENV2 --env TEST foo/bar:tag -c 'echo hello > /tmp/data/foo.txt && tar cvf data.tar /tmp/data'\""
: "./buildkite/scripts/cache-through.sh data.tar \"docker run -it --rm --entrypoint /bin/bash --init --volume /var/secrets:/var/secrets --volume /var/buildkite/shared:/shared --volume \\\$BUILDKITE_BUILD_CHECKOUT_PATH:/workdir --workdir /workdir --env ENV1 --env ENV2 --env TEST foo/bar:tag -c 'echo hello > /tmp/data/foo.txt && tar cvf data.tar /tmp/data'\""
=== M.format
( M.cacheThrough
M.Docker::{
Expand Down
53 changes: 42 additions & 11 deletions scripts/version-linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,50 @@ def set_error():
global exit_code
exit_code=1

def branch_commit(branch):
def branch_commits(branch,n):
print ('Retrieving', branch, 'head commit...')
result=subprocess.run(['git','log','-n','1','--format="%h"','--abbrev=7',f'{branch}'],
capture_output=True)
output=result.stdout.decode('ascii')
print ('command stdout:', output)
print ('command stderr:', result.stderr.decode('ascii'))
return output.replace('"','').replace('\n','')

def download_type_shapes(role,branch,sha1) :
return output.replace('"','').splitlines()

def url_to_type_shape_file(file):
'''
Return url to mina type shape file
'''
return f'https://storage.googleapis.com/mina-type-shapes/{file}'

def sha_exists(sha1):
'''
Checks if mina type shape with given sha exists
'''
file = type_shape_file(sha1)
return url_exists(url_to_type_shape_file(file))

def url_exists(url):
'''
Checks if url exists (by sending head and validating that status code is ok)
'''
return requests.head(url).status_code == 200

def find_latest_type_shape_ref_on(branch,n=1):
'''
Function tries to find best type shape reference commit by retrieving n last commits
and iterate over collection testing if any item points to valid url
'''
commits = branch_commits(branch, n)
candidates = list(filter(lambda x: sha_exists(x), commits))
if not any(candidates):
raise Exception(f'Cannot find type shape file for {branch}. I tried {n} last commits')
else:
return candidates[0]

def download_type_shape(role,branch,sha1) :
file=type_shape_file(sha1)
print ('Downloading type shape file',file,'for',role,'branch',branch,'at commit',sha1)
result=subprocess.run(['wget','--no-clobber',f'https://storage.googleapis.com/mina-type-shapes/{file}'])
result=subprocess.run(['wget','--no-clobber',url_to_type_shape_file(file)])

def type_shape_file(sha1) :
# created by buildkite build-artifact script
Expand Down Expand Up @@ -237,18 +268,18 @@ def assert_commit(commit, desc):

subprocess.run(['git','fetch'],capture_output=False)

base_branch_commit=branch_commit(base_branch)
download_type_shapes('base',base_branch,base_branch_commit)
base_branch_commit = find_latest_type_shape_ref_on(base_branch,n=10)
download_type_shape('base',base_branch,base_branch_commit)

print('')

release_branch_commit=branch_commit(release_branch)
download_type_shapes('release',release_branch,release_branch_commit)
release_branch_commit=find_latest_type_shape_ref_on(release_branch, n=10)
download_type_shape('release',release_branch,release_branch_commit)

print('')

pr_branch_commit=branch_commit(pr_branch)
download_type_shapes('pr',pr_branch,pr_branch_commit)
pr_branch_commit=find_latest_type_shape_ref_on(pr_branch)
download_type_shape('pr',pr_branch,pr_branch_commit)

print('')

Expand Down