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

Update parsing TEMURIN_BUILD_ARGS #3824

Merged
merged 14 commits into from
Jun 21, 2024
Merged

Update parsing TEMURIN_BUILD_ARGS #3824

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5ebfb93
Archive SBOM file and exit instant if build jdk fails
sophia-guo May 23, 2024
d98d20e
Testing
sophia-guo May 23, 2024
55ce82b
Install strace
sophia-guo May 28, 2024
1415629
Add --cap-add SYS_PTRACE for docker run
sophia-guo May 28, 2024
8af8e39
Turn off --enable-sbom-strace
sophia-guo May 30, 2024
75e969a
update
sophia-guo Jun 3, 2024
b2050b5
Update build args setting
sophia-guo Jun 5, 2024
6919b17
Add --build-reproducible-date args
sophia-guo Jun 7, 2024
5ebad2c
Leave windows alone
sophia-guo Jun 7, 2024
636999a
USING_DEVKIT is set the wrong way around
sophia-guo Jun 13, 2024
0ee465d
Keep old method of parsing configure-args to work for old GAs
sophia-guo Jun 18, 2024
2ccf570
Adding back the extra space
sophia-guo Jun 19, 2024
c614969
Comment update
sophia-guo Jun 19, 2024
baafa00
Naming change
sophia-guo Jun 20, 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
4 changes: 3 additions & 1 deletion test/system/reproducibleCompare/playlist.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
<include>reproducible.mk</include>
<test>
<testCaseName>Rebuild_Same_JDK_Reproducibility_Test</testCaseName>
<command>docker run -v "$(TEST_RESROOT):/home/jenkins/test" -w "/home/jenkins/" -v "$(TEST_JDK_HOME):/home/jenkins/jdkbinary/" --name reproducibleCompare centos:7 /bin/bash /home/jenkins/test/linux_repro_build_compare.sh $(SBOM_FILE) /home/jenkins/jdkbinary; \
<command>docker container rm reproducibleCompare; \
docker run -v "$(TEST_RESROOT):/home/jenkins/test" -w "/home/jenkins/" -v "$(TEST_JDK_HOME):/home/jenkins/jdkbinary/" --name reproducibleCompare centos:7 /bin/bash /home/jenkins/test/linux_repro_build_compare.sh $(SBOM_FILE) /home/jenkins/jdkbinary; \
$(TEST_STATUS); \
docker cp reproducibleCompare:/home/jenkins/reprotest.diff ./; \
docker cp reproducibleCompare:/home/jenkins/reproJDK.tar.gz ./; \
docker cp reproducibleCompare:/home/jenkins/SBOM.json ./; \
docker container rm reproducibleCompare
</command>
<levels>
Expand Down
74 changes: 58 additions & 16 deletions tooling/reproducible/linux_repro_build_compare.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash
# ********************************************************************************
karianna marked this conversation as resolved.
Show resolved Hide resolved
# Copyright (c) 2024 Contributors to the Eclipse Foundation
#
Expand All @@ -25,7 +25,7 @@ ANT_SHA=9028e2fc64491cca0f991acc09b06ee7fe644afe41d1d6caf72702ca25c4613c
ANT_CONTRIB_VERSION=1.0b3
ANT_CONTRIB_SHA=4d93e07ae6479049bb28071b069b7107322adaee5b70016674a0bffd4aac47f9
isJdkDir=false

USING_DEVKIT="false"
installPrereqs() {
if test -r /etc/redhat-release; then
yum install -y gcc gcc-c++ make autoconf unzip zip alsa-lib-devel cups-devel libXtst-devel libXt-devel libXrender-devel libXrandr-devel libXi-devel
Expand Down Expand Up @@ -75,28 +75,50 @@ downloadAnt() {
fi
}

setEnvironment() {
setGccEnvironment() {
export CC="${LOCALGCCDIR}/bin/gcc-${GCCVERSION}"
Copy link
Member

Choose a reason for hiding this comment

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

I'd be a bit happier if we changed the name of this. The devkit on Linux still using gcc so this may cause some confusion. Maybe setNonDevkitGccEnvironment?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes agree, that is what was going through my head when I suggested it, ie.set the Gcc environment when not using a DevKit...

export CXX="${LOCALGCCDIR}/bin/g++-${GCCVERSION}"
export LD_LIBRARY_PATH="${LOCALGCCDIR}/lib64"
# /usr/local/bin required to pick up the new autoconf if required
}

setAntEnvironment() {
export PATH="${LOCALGCCDIR}/bin:/usr/local/bin:/usr/bin:$PATH:/usr/local/apache-ant-${ANT_VERSION}/bin"
ls -ld "$CC" "$CXX" "/usr/lib/jvm/jdk-${BOOTJDK_VERSION}/bin/javac" || exit 1
}

cleanBuildInfo() {
# shellcheck disable=SC3043
local DIR="$1"
# BUILD_INFO name of OS level build was built on will likely differ
sed -i '/^BUILD_INFO=.*$/d' "${DIR}/release"
}

setTemurinBuildArgs() {
local buildArgs="$1"
local bootJdk="$2"
local timeStamp="$3"
local ignoreOptions=("--enable-sbom-strace ")
for ignoreOption in "${ignoreOptions[@]}"; do
Copy link
Member

Choose a reason for hiding this comment

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

I'd personally go with this as it feels less complex than using an array - you can add extra things to filter out later with multiple -e parameters if needed. But I'm ok with either approach. We could also combine this with the jdk-boot-dir sed below.

Suggested change
local ignoreOptions=("--enable-sbom-strace ")
# Remove arguments that are not necessary for reproducibility comparison
buildArgs="$(echo buildArgs | sed -e 's/--enable-sbom-strace//g')"

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 not a code standard issue. I personally feel the array is simpler if extra options need to be filtered out. Using multiple -e make it less readable and more error-prone for me.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced this logic is working. When I tried it out it converted:

--create-sbom --enable-sbom-strace --use-adoptium-devkit

in the string to:

--create-sbom /--use-adoptium-devkit

so if you want to leave it with the bash-specific construct, I think you'll need to switch // to / in the translation expression.

Copy link
Contributor Author

@sophia-guo sophia-guo Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Confused. @sxa which logic did you try? Seems you were trying buildArgs="$(echo buildArgs | sed -e 's/--enable-sbom-strace//g')"? As I said I would prefer using the array, similar to the window reproducible comparing script.

Copy link
Member

@sxa sxa Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Seems you were trying buildArgs="$(echo buildArgs | sed -e 's/--enable-sbom-strace//g')"

No, the version with sed works ok - the current version with the array adds in an extra / unless you remove one of them as you can see in this example:

sxa@fedora:~$ buildArgs="abc --enable-sbom-strace def"
sxa@fedora:~$ echo $buildArgs | sed -e 's/--enable-sbom-strace//g'
abc  def
sxa@fedora:~$ ignoreOptions=("--enable-sbom-strace ")
sxa@fedora:~$   for ignoreOption in "${ignoreOptions[@]}"; do
    buildArgs="${buildArgs/${ignoreOption}//}"
  done
sxa@fedora:~$ echo $buildArgs
abc /def
sxa@fedora:~$

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 see, updated.

buildArgs="${buildArgs/${ignoreOption}/}"
done
# set --build-reproducible-date if not yet
if [[ "${buildArgs}" != *"--build-reproducible-date"* ]]; then
buildArgs="--build-reproducible-date \"${timeStamp}\" ${buildArgs}"
fi
#reset --jdk-boot-dir
# shellcheck disable=SC2001
buildArgs="$(echo "$buildArgs" | sed -e "s|--jdk-boot-dir [^ ]*|--jdk-boot-dir /usr/lib/jvm/jdk-${bootJdk}|")"
echo "${buildArgs}"
}

downloadTooling() {
local using_DEVKIT=$1

Copy link
Member

Choose a reason for hiding this comment

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

Noting that this may be a little confusing to anyone reading this to see a local variable using_DEVKIT and a global one USING_DEVKIT

if [ ! -r "/usr/lib/jvm/jdk-${BOOTJDK_VERSION}/bin/javac" ]; then
echo "Retrieving boot JDK $BOOTJDK_VERSION" && mkdir -p /usr/lib/jvm && curl -L "https://api.adoptium.net/v3/binary/version/jdk-${BOOTJDK_VERSION}/linux/${NATIVE_API_ARCH}/jdk/hotspot/normal/eclipse?project=jdk" | (cd /usr/lib/jvm && tar xpzf -)
fi
if [ ! -r "${LOCALGCCDIR}/bin/g++-${GCCVERSION}" ]; then
echo "Retrieving gcc $GCCVERSION" && curl "https://ci.adoptium.net/userContent/gcc/gcc$(echo "$GCCVERSION" | tr -d .).$(uname -m).tar.xz" | (cd /usr/local && tar xJpf -) || exit 1
if [[ "${using_DEVKIT}" == "false" ]]; then
if [ ! -r "${LOCALGCCDIR}/bin/g++-${GCCVERSION}" ]; then
echo "Retrieving gcc $GCCVERSION" && curl "https://ci.adoptium.net/userContent/gcc/gcc$(echo "$GCCVERSION" | tr -d .).$(uname -m).tar.xz" | (cd /usr/local && tar xJpf -) || exit 1
fi
fi
if [ ! -r temurin-build ]; then
git clone https://github.com/adoptium/temurin-build || exit 1
Expand All @@ -113,7 +135,6 @@ checkAllVariablesSet() {
installPrereqs
downloadAnt

# shellcheck disable=SC3010
if [[ $SBOM_PARAM =~ ^https?:// ]]; then
echo "Retrieving and parsing SBOM from $SBOM_PARAM"
curl -LO "$SBOM_PARAM"
Expand All @@ -126,23 +147,41 @@ BOOTJDK_VERSION=$(jq -r '.metadata.tools[] | select(.name == "BOOTJDK") | .versi
GCCVERSION=$(jq -r '.metadata.tools[] | select(.name == "GCC") | .version' "$SBOM" | sed 's/.0$//')
LOCALGCCDIR=/usr/local/gcc$(echo "$GCCVERSION" | cut -d. -f1)
TEMURIN_BUILD_SHA=$(jq -r '.components[0] | .properties[] | select (.name == "Temurin Build Ref") | .value' "$SBOM" | awk -F/ '{print $NF}')
TEMURIN_BUILD_ARGS=$(jq -r '.components[0] | .properties[] | select (.name == "makejdk_any_platform_args") | .value' "$SBOM" | cut -d\" -f4 | sed -e "s/--disable-warnings-as-errors --enable-dtrace --without-version-pre --without-version-opt/'--disable-warnings-as-errors --enable-dtrace --without-version-pre --without-version-opt'/" -e "s/ --disable-warnings-as-errors --enable-dtrace/ '--disable-warnings-as-errors --enable-dtrace'/" -e 's/\\n//g' -e "s,--jdk-boot-dir [^ ]*,--jdk-boot-dir /usr/lib/jvm/jdk-$BOOTJDK_VERSION,g")
TEMURIN_VERSION=$(jq -r '.metadata.component.version' "$SBOM" | sed 's/-beta//' | cut -f1 -d"-")
BUILDSTAMP=$(jq -r '.components[0].properties[] | select(.name == "Build Timestamp") | .value' "$SBOM")
TEMURIN_BUILD_ARGS=$(jq -r '.components[0] | .properties[] | select (.name == "makejdk_any_platform_args") | .value' "$SBOM")

# Using old method to escape configure-args in SBOM (build PR 3835)", can be removed later.
if echo "$TEMURIN_BUILD_ARGS" | grep -v configure-args\ \\\"; then
TEMURIN_BUILD_ARGS=$(echo "$TEMURIN_BUILD_ARGS" | \
cut -d\" -f4 | \
sed -e "s/--with-jobs=[0-9]*//g" \
-e "s/--disable-warnings-as-errors --enable-dtrace --without-version-pre --without-version-opt/'--disable-warnings-as-errors --enable-dtrace --without-version-pre --without-version-opt'/" \
-e "s/--disable-warnings-as-errors --enable-dtrace *--with-version-opt=ea/'--disable-warnings-as-errors --enable-dtrace --with-version-opt=ea'/" \
-e "s/ --disable-warnings-as-errors --enable-dtrace/ '--disable-warnings-as-errors --enable-dtrace'/" \
-e 's/\\n//g')
fi

NATIVE_API_ARCH=$(uname -m)
if [ "${NATIVE_API_ARCH}" = "x86_64" ]; then NATIVE_API_ARCH=x64; fi
if [ "${NATIVE_API_ARCH}" = "armv7l" ]; then NATIVE_API_ARCH=arm; fi
if [[ "$TEMURIN_BUILD_ARGS" =~ .*"--use-adoptium-devkit".* ]]; then
USING_DEVKIT="true"
fi

checkAllVariablesSet

downloadTooling
setEnvironment
downloadTooling "$USING_DEVKIT"
if [[ "${USING_DEVKIT}" == "false" ]]; then
setGccEnvironment
fi
setAntEnvironment
echo "original temurin build args is ${TEMURIN_BUILD_ARGS}"
TEMURIN_BUILD_ARGS=$(setTemurinBuildArgs "$TEMURIN_BUILD_ARGS" "$BOOTJDK_VERSION" "$BUILDSTAMP")

if [ -z "$JDK_PARAM" ] && [ ! -d "jdk-${TEMURIN_VERSION}" ] ; then
Copy link
Member

Choose a reason for hiding this comment

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

Extra blank line here should be removed

JDK_PARAM="https://api.adoptium.net/v3/binary/version/jdk-${TEMURIN_VERSION}/linux/${NATIVE_API_ARCH}/jdk/hotspot/normal/eclipse?project=jdk"
fi

# shellcheck disable=SC3010
if [[ $JDK_PARAM =~ ^https?:// ]]; then
echo Retrieving original tarball from adoptium.net && curl -L "$JDK_PARAM" | tar xpfz - && ls -lart "$PWD/jdk-${TEMURIN_VERSION}" || exit 1
elif [[ $JDK_PARAM =~ tar.gz ]]; then
Expand All @@ -158,20 +197,23 @@ if [ "${isJdkDir}" = true ]; then
comparedDir=$JDK_PARAM
fi

echo "Rebuild Temurin build args is $TEMURIN_BUILD_ARGS"
echo " cd temurin-build && ./makejdk-any-platform.sh $TEMURIN_BUILD_ARGS 2>&1 | tee build.$$.log" | sh
Copy link
Member

Choose a reason for hiding this comment

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

I feel it's nice to make this explicit for anyone that wants to replicate this outside the build jobs:

Suggested change
echo "Rebuild Temurin build args is $TEMURIN_BUILD_ARGS"
echo "makejdk_any_platform.sh build args are: $TEMURIN_BUILD_ARGS"


echo Comparing ...
mkdir compare.$$
tar xpfz temurin-build/workspace/target/OpenJDK*-jdk_*tar.gz -C compare.$$
cp temurin-build/workspace/target/OpenJDK*-jdk_*tar.gz reproJDK.tar.gz
cp "$SBOM" SBOM.json

cleanBuildInfo "${comparedDir}"
cleanBuildInfo "compare.$$/jdk-$TEMURIN_VERSION"

rc=0

# shellcheck disable=SC2069
diff -r "${comparedDir}" "compare.$$/jdk-$TEMURIN_VERSION" 2>&1 > "reprotest.diff" || rc=$?

if [ $rc = 0 ]; then
if [ $rc -eq 0 ]; then
echo "Compare identical !"
else
cat "reprotest.diff"
Expand Down
Loading