-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
allowing build from diectory/src snapshot #3724
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -178,8 +178,9 @@ configureMacOSCodesignParameter() { | |
# Get the OpenJDK update version and build version | ||
getOpenJDKUpdateAndBuildVersion() { | ||
cd "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}" | ||
|
||
if [ -d "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/.git" ]; then | ||
if [ "${BUILD_CONFIG[OPENJDK_FOREST_DIR]}" == "true" ]; then | ||
echo "Version: local dir; OPENJDK_BUILD_NUMBER set as ${BUILD_CONFIG[OPENJDK_BUILD_NUMBER]}" | ||
elif [ -d "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/.git" ]; then | ||
|
||
# It does exist and it's a repo other than the Temurin one | ||
cd "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}" || return | ||
|
@@ -1802,15 +1803,20 @@ getFirstTagFromOpenJDKGitRepo() { | |
TAG_SEARCH="aarch64-shenandoah-jdk8u*-b*" | ||
fi | ||
|
||
# If openj9 and the closed/openjdk-tag.gmk file exists which specifies what level the openj9 jdk code is based upon, | ||
# read OPENJDK_TAG value from that file. | ||
local openj9_openjdk_tag_file="${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/closed/openjdk-tag.gmk" | ||
if [[ "${BUILD_CONFIG[BUILD_VARIANT]}" == "${BUILD_VARIANT_OPENJ9}" ]] && [[ -f "${openj9_openjdk_tag_file}" ]]; then | ||
firstMatchingNameFromRepo=$(grep OPENJDK_TAG ${openj9_openjdk_tag_file} | awk 'BEGIN {FS = "[ :=]+"} {print $2}') | ||
if [ "${BUILD_CONFIG[OPENJDK_FOREST_DIR]}" == "true" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't modify the getFirstTagFromOpenJDKGitRepo() method at all, it should only be called for a "git repo", There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanx, will do!
I think it defaults to some unknonw or head, but I will double check. If it do not default, then I would rather default it to some "local" rather then make it mandatory. Of course, it it is set, then it will be used |
||
echo "Skipping tag reading from local dir. You shoud have set up at least OPENJDK_FEATURE_NUMBER" 1>&2 | ||
firstMatchingNameFromRepo="" | ||
else | ||
git fetch --tags "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}" | ||
firstMatchingNameFromRepo=$(git tag --list "$TAG_SEARCH" | "$get_tag_cmd") | ||
fi | ||
# If openj9 and the closed/openjdk-tag.gmk file exists which specifies what level the openj9 jdk code is based upon, | ||
# read OPENJDK_TAG value from that file. | ||
local openj9_openjdk_tag_file="${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/closed/openjdk-tag.gmk" | ||
if [[ "${BUILD_CONFIG[BUILD_VARIANT]}" == "${BUILD_VARIANT_OPENJ9}" ]] && [[ -f "${openj9_openjdk_tag_file}" ]]; then | ||
firstMatchingNameFromRepo=$(grep OPENJDK_TAG ${openj9_openjdk_tag_file} | awk 'BEGIN {FS = "[ :=]+"} {print $2}') | ||
else | ||
git fetch --tags "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}" | ||
firstMatchingNameFromRepo=$(git tag --list "$TAG_SEARCH" | "$get_tag_cmd") | ||
fi | ||
fi | ||
|
||
if [ -z "$firstMatchingNameFromRepo" ]; then | ||
echo "WARNING: Failed to identify latest tag in the repository" 1>&2 | ||
|
@@ -2241,6 +2247,7 @@ echo "build.sh : $(date +%T) : Configuring workspace inc. clone and cacerts gene | |
configureWorkspace | ||
|
||
echo "build.sh : $(date +%T) : Initiating build ..." | ||
set -x | ||
getOpenJDKUpdateAndBuildVersion | ||
if [[ "$OSTYPE" == "cygwin" ]] || [[ "$OSTYPE" == "msys" ]]; then | ||
patchFreetypeWindows | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,19 @@ | |
# shellcheck disable=SC2153 | ||
function setOpenJdkVersion() { | ||
local forest_name=$1 | ||
set -x | ||
if [ -e "${forest_name}" ] ; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is the right place for this logic.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two things mixed togehter. Yyou are absolutly correct and I woudl very like to avoid the "expects exactly one argument, unless --openjdk-source si used, then it expects no argument" Unless you are expecting the usage as
In comaprison to
wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So maybe, the syntax of makejdk-any-platform.sh, is:
Where jdk_version is what has historically been named as the "forest version"
These are needed throughtout the sbin/build.sh build logic.. So you will need to set these from some_terrible_name/path/zip ?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would actually prefer:
because the existing logic very clearly set:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Exactly, And thats what that dir/zip i susually named. See #3727 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Probably my main reason is to save typing
properly. Do you really insists? |
||
BUILD_CONFIG[OPENJDK_FOREST_DIR]="true" | ||
BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]=$(readlink -f "${forest_name}") | ||
BUILD_CONFIG[DISABLE_ADOPT_BRANCH_SAFETY]="true" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. User should specify build arg: --disable-adopt-branch-safety There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was breaking in the branching block. I can remove this, but conisder:
I will obey as you need, but the setup of it sounds to me more clean and less harmfull. |
||
if [ ! -z "${BUILD_CONFIG[OPENJDK_FOREST_ALTID]}" ] ; then | ||
forest_name="${BUILD_CONFIG[OPENJDK_FOREST_ALTID]}" | ||
else | ||
# we want to use "." and similar | ||
forest_name=$(basename "${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}") | ||
fi | ||
BUILD_CONFIG[OPENJDK_SOURCE_DIR]="${forest_name}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't set OPENJDK_SOURCE_DIR, as that is set to the folder under the WORK_DIR, default "src", and is where the git clone of openjdk clones the source. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yah, I agree, I think I set it because I was unsure how to proeprly set the name to be read by. Will elaborate and fix |
||
fi | ||
|
||
# Derive the openjdk_core_version from the forest name. | ||
local openjdk_core_version=${forest_name} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,10 @@ NUM_PROCESSORS | |
OPENJDK_BUILD_NUMBER | ||
OPENJDK_CORE_VERSION | ||
OPENJDK_FEATURE_NUMBER | ||
OPENJDK_FOREST_ALTID | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think all you need is a OPENJDK_FOREST_SOURCE build arg There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you mean |
||
OPENJDK_FOREST_NAME | ||
OPENJDK_FOREST_DIR | ||
OPENJDK_FOREST_DIR_ABSPATH | ||
OPENJDK_SOURCE_DIR | ||
OPENJDK_UPDATE_VERSION | ||
OS_KERNEL_NAME | ||
|
@@ -306,6 +309,9 @@ function parseConfigurationArguments() { | |
"--jdk-boot-dir" | "-J" ) | ||
BUILD_CONFIG[JDK_BOOT_DIR]="$1";shift;; | ||
|
||
"--jdk-dir-altid") | ||
BUILD_CONFIG[OPENJDK_FOREST_ALTID]="$1";shift;; | ||
|
||
"--cross-compile" ) | ||
BUILD_CONFIG[CROSSCOMPILE]=true;; | ||
|
||
|
@@ -457,6 +463,15 @@ function configDefaults() { | |
# The full forest name, e.g. jdk8, jdk8u, jdk9, jdk9u, etc. | ||
BUILD_CONFIG[OPENJDK_FOREST_NAME]="" | ||
|
||
# whether the forest is local directory or not | ||
BUILD_CONFIG[OPENJDK_FOREST_DIR]="false" | ||
|
||
# whether the forest is local directory or not | ||
BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]="" | ||
|
||
# whether user provided original repo if the directory is useless | ||
BUILD_CONFIG[OPENJDK_FOREST_ALTID]="" | ||
|
||
# The abridged openjdk core version name, e.g. jdk8, jdk9, etc. | ||
BUILD_CONFIG[OPENJDK_CORE_VERSION]="" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ | |
################################################################################ | ||
|
||
set -eu | ||
|
||
set -x | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove debug.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure thing. before merge, all |
||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
|
||
# shellcheck source=sbin/common/constants.sh | ||
|
@@ -37,14 +37,52 @@ ALSA_LIB_CHECKSUM=${ALSA_LIB_CHECKSUM:-5f2cd274b272cae0d0d111e8a9e363f0878332915 | |
ALSA_LIB_GPGKEYID=${ALSA_LIB_GPGKEYID:-A6E59C91} | ||
FREETYPE_FONT_SHARED_OBJECT_FILENAME="libfreetype.so*" | ||
|
||
|
||
copyFromDir() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. copyFromDir() and unpackFromArchive() need to "copy"/"unpack" the forest openjdk source to where the build scripts intends to build the source within the defined WORKING_DIR,
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original clonnign script is using the I will heapily switch to full path instead of "." but allt he cloning methods were really exppecting other parts to |
||
echo "Copyng existing ${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]} to `pwd`/${BUILD_CONFIG[OPENJDK_FOREST_NAME]} to be built" | ||
mkdir -p "./${BUILD_CONFIG[OPENJDK_FOREST_NAME]}" | ||
# we really do not want to use .git for dirs, as we expect user have them set up, ignoring them | ||
local files=$(find ${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]} -maxdepth 1 -mindepth 1 | grep -v -e "/workspace$" -e "/build$" -e "/.git") | ||
cp -rf $files "./${BUILD_CONFIG[OPENJDK_FOREST_NAME]}/" | ||
} | ||
|
||
unpackFromArchive() { | ||
echo "Extracting source tarball ${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]} to `pwd`/${BUILD_CONFIG[OPENJDK_FOREST_NAME]} to be built" | ||
mkdir -p "./${BUILD_CONFIG[OPENJDK_FOREST_NAME]}" | ||
# if the tarball contains .git files, they should be ignored later withut shame | ||
# todo, support also zips? | ||
pushd "./${BUILD_CONFIG[OPENJDK_FOREST_NAME]}" | ||
local topLevelItems=$(tar --exclude='*/*' -tf "${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}" | wc -l) | ||
if [ "$topLevelItems" -eq "1" ] ; then | ||
echo "Source tarball contans exaclty one directory, using" | ||
tar --strip-components 1 -xf "${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}" | ||
else | ||
echo "Source tarball do not contains top level directory, using" | ||
tar -xf "${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}" | ||
fi | ||
popd | ||
} | ||
|
||
copyFromDirOrUnpackFromArchive() { | ||
if [ -d "${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}" ] ; then | ||
copyFromDir | ||
elif [ -f "${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}" ] ; then | ||
unpackFromArchive | ||
else | ||
echo "Not directory nor file ${BUILD_CONFIG[OPENJDK_FOREST_DIR_ABSPATH]}" | ||
exit 1 | ||
fi | ||
rm -vf ./${BUILD_CONFIG[OPENJDK_FOREST_NAME]}/build/*/configure-support/config.status | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should not be needed as the build-scripts will clean the autoconf configuration in the intended build location. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unluckily it is necessary. The methods later in build.sh, are not expoecting prexsiting configuration, and the builds were failing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you copyOrUntarZip() methods should be copying/untaring the source to where temurin-build scripts is doing the build, ie. to ${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
# Create a new clone or update the existing clone of the OpenJDK source repo | ||
# TODO refactor this for Single Responsibility Principle (SRP) | ||
checkoutAndCloneOpenJDKGitRepo() { | ||
|
||
cd "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}" | ||
|
||
# Check that we have a git repo, we assume that it is a repo that contains openjdk source | ||
if [ -d "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/.git" ]; then | ||
if [ -d "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/.git" -a "${BUILD_CONFIG[OPENJDK_FOREST_DIR]}" == "false" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use && to be consistent with this script There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ouch. Sorry. Will do! |
||
set +e | ||
git --git-dir "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/.git" remote -v | ||
echo "${BUILD_CONFIG[OPENJDK_CORE_VERSION]}" | ||
|
@@ -78,6 +116,8 @@ checkoutAndCloneOpenJDKGitRepo() { | |
echo "If this is inside a docker you can purge the existing source by passing --clean-docker-build" | ||
exit 1 | ||
fi | ||
elif [ "${BUILD_CONFIG[OPENJDK_FOREST_DIR]}" == "true" ]; then | ||
copyFromDirOrUnpackFromArchive | ||
elif [ ! -d "${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}/.git" ]; then | ||
echo "Could not find a valid openjdk git repository at $(pwd)/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]} so re-cloning the source to openjdk" | ||
rm -rf "${BUILD_CONFIG[WORKSPACE_DIR]:?}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}" | ||
|
@@ -106,8 +146,9 @@ checkoutAndCloneOpenJDKGitRepo() { | |
fi | ||
fi | ||
|
||
git clean -ffdx | ||
|
||
if [ ! "${BUILD_CONFIG[OPENJDK_FOREST_DIR]}" == "true" ]; then | ||
git clean -ffdx | ||
fi | ||
updateOpenj9Sources | ||
|
||
createSourceTagFile | ||
|
@@ -118,6 +159,17 @@ checkoutAndCloneOpenJDKGitRepo() { | |
# Checkout the required code to build from the given cached git repo | ||
# Set checkoutRc to result so we can retry | ||
checkoutRequiredCodeToBuild() { | ||
|
||
if [ "${BUILD_CONFIG[OPENJDK_FOREST_DIR]}" == "true" ]; then | ||
echo "skipping checkoutRequiredCodeToBuild - local directory under processing:" | ||
echo " workspace = ${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}" | ||
echo " BUILD_VARIANT = ${BUILD_CONFIG[BUILD_VARIANT]}" | ||
echo " TAG = ${BUILD_CONFIG[TAG]} - Used only in name, if at all" | ||
echo " BRANCH = ${BUILD_CONFIG[BRANCH]} - UNUSED!" | ||
checkoutRc=0 | ||
return | ||
fi | ||
|
||
checkoutRc=1 | ||
|
||
cd "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/${BUILD_CONFIG[OPENJDK_SOURCE_DIR]}" | ||
|
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.
OPENJDK_BUILD_NUMBER is in fact just the openjdk build number, so for example jdk-21.0.2+13, the build number is just "13"
I think you probably want to use "TAG" in this echo statement, and when you build from your own forest source you would pass arguments:
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.
Nope. Surprisingly in this whole enhancement I was not calculating with the tag. It is jsut tarball/dir - I do pretend I do not care baout whatever version it is/was. However jdk major "the number" is absolutely necessary, is deducted few times, by various ways, and there is no way around.
I will keep this as it is, unless the tags will come more to play.