opensearch-build
opensearch-build copied to clipboard
remove OpenSearch `build.sh`: moved to OpenSearch repo
build.sh has been copied over to all active branches(*) of the OpenSearch repo and can now consequently be deleted here. this means, that from now on, the scripts from the respository will be picked up.
for more information on the order of evaluation of the custom build scripts, see the documentation.
this gives the advantage to have branch-specific new logic in place there. this has already been used to add the no-jdk targets to the builds on main and 2.x.
two tests were based on the assumption that there would be a build.sh for OpenSearch in this repository, however these are generic tests and just used OpenSearch as an example. they have been switched to be based on OpenSearch-Dashboards instead which - for the time being - still has its own build.sh here.
(*): the following branches have been considered as active:
- main
- 2.x
- 2.3
- 1.x
- 1.3
this is part of opensearch-project/opensearch-build#99
Signed-off-by: Ralph Ursprung [email protected]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.
important: i'm not aware of a way to properly test this manually, thus this remains untested!
Codecov Report
Merging #2835 (e6288a3) into main (9a48acc) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## main #2835 +/- ##
=======================================
Coverage 94.15% 94.15%
=======================================
Files 157 157
Lines 4242 4242
=======================================
Hits 3994 3994
Misses 248 248
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
i was sort-of hoping to get this in for 2.4.0 😄. but otherwise we can also push it back
Hi @rursprung I wonder if we can block this until 2.4.0 is released to public. We are at the edge of the release in about a week or so.
I want to make sure this has minimum impact.
Let me know what you think.
Thanks.
@peterzhuamazon it shouldn't break anything should it? we just moved a bash file
Hi @rursprung I wonder if we can block this until 2.4.0 is released to public. We are at the edge of the release in about a week or so. I want to make sure this has minimum impact. Let me know what you think. Thanks.
@peterzhuamazon it shouldn't break anything should it? we just moved a bash file
We are building release candidate of 2.4.0 tomorrow. @bbarani what is your thought on this? Thanks.
Hi @rursprung we have seen there is changes to add no-jdk build in build.sh.
$ diff -bi build.sh build.sh_new
142c142
< ./gradlew :distribution:$TYPE:$TARGET:assemble -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER
---
> ./gradlew :distribution:$TYPE:$TARGET:assemble :distribution:$TYPE:no-jdk-$TARGET:assemble -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER
We need some additional time to review and plan the no-jdk addition. So in order to not block the 2.4.0 release. Let us postpone this changes by next possible release.
Thanks again for the contribution and let us discuss more on this.
Thanks.
Hi @rursprung we have seen there is changes to add no-jdk build in build.sh.
yes, that was added in https://github.com/opensearch-project/OpenSearch/pull/4902 and ported to 2.x in https://github.com/opensearch-project/OpenSearch/pull/4993
Based on the offline conversation I will test the build on local to ensure that no-jdk is not affecting the normal jdk build. If not we can merge this, else will need to wait until 1.3.7 is out.
Thanks.
I have confirmed it will not impact the existing code base.
But, instead of building both jdk and no-jdk in one take, we should add a --no-jdk param in build repo, so the target will be properly set as
:distribution:$TYPE:$TARGET:assemble
instead of
:distribution:$TYPE:no-jdk-$TARGET:assemble
This will also resolve the follow up assemble issue with no-jdk bundle.
There are still a lot of steps to go, I would recommend hold this PR for a bit longer, and start changing the build.sh on OpenSearch Core repo.
Ex:
echo -e "-j \t[Optional] with or without JDK bundled, default is 'true'."
......
if [ "$JDK" != "true" ]; then
JDK_TARGET="-no-jdk"
fi
case $PLATFORM-$DISTRIBUTION-$ARCHITECTURE$JDK_TARGET in
linux-tar-x64-no-jdk|darwin-tar-x64-no-jdk)
PACKAGE="tar"
EXT="tar.gz"
TYPE="archives"
TARGET="no-jdk-$PLATFORM-$PACKAGE"
SUFFIX="$PLATFORM-x64"
;;
./gradlew :distribution:$TYPE:$TARGET:assemble -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER
Thanks.
I also want to mention this seems significantly more complex than initially anticipated. cc @bbarani into this as this requires several weeks of dedicated resource to get it run properly.
Thanks.
I am confused. We just need an old build.sh in those old opensearch branches. Why is this any more complicated?
I am confused. We just need an old build.sh in those old opensearch branches. Why is this any more complicated?
i guess the problem was that we (well, I) mixed two things:
- moving
build.shto the repo (I copied them 1:1 to all active branches and we got these PRs merged, the outstanding work there was this PR here) - add no-jdk to the build (separate PRs on main & 2.x, merged there before this here was merged)
it might make sense to just revert the latter (no-jdk) and get this one here merged to at least close the move job. and then we can look into the no-jdk discussion again?
The issue is more that instead of adding a no-jdk, the original implementation is to run both jdk + no-jdk build in parallel, which is not supposed to be as the current code base does not support the actions after that.
I am with you @rursprung. Let's focus on this PR! Can we merge just deleting the OpenSearch build.sh from this repo?
I am with you @rursprung. Let's focus on this PR! Can we merge just deleting the OpenSearch build.sh from this repo?
i've created the reversal PRs and they're ready for review:
- https://github.com/opensearch-project/OpenSearch/pull/5465
- https://github.com/opensearch-project/OpenSearch/pull/5466
- https://github.com/opensearch-project/OpenSearch/pull/5467
- (other branches not affected because i only added this to
main&2.x(before2.4had been branched away)
but i just realised that one more step will be needed before this here can be done:
in the meantime, @mnin has merged the PR https://github.com/opensearch-project/opensearch-build/pull/2526 which modified the OpenSearch build.sh here but not in the OpenSearch repo itself (because that one hasn't been used yet). so that change needs to be replicated there as well before dropping it here (otherwise the DEB build is lost again).
Hi @peterzhuamazon Should we move ahead with this PR?
@peterzhuamazon @rursprung What's the next step on this PR? Please provide your comments.
@peterzhuamazon @rursprung What's the next step on this PR? Please provide your comments.
between the moment where i added the build.sh to the OpenSearch repo and the moment this PR got reviewed the first time there were subsequent changes to the build.sh in this repo which had not been ported to the OpenSearch repo the last time i checked (the author wanted to wait because he planned further changes, i hadn't checked back since) - and i also haven't checked if subsequent changes have happened here.
so before this PR can be rebased & merged we have to ensure that build.sh is up to date in the OpenSearch repo. but if we do this then we have to ensure this time that no further changes are added to it in this repo until the updates in the OpenSearch repo and this PR here is merged.
@peterzhuamazon What would you like to do with this (finish/close)?