adoptium / temurin-build

Eclipse Temurin™ build scripts - common across all releases/versions
Apache License 2.0
1.02k stars 249 forks source link

Clarity on where config parameters should be defined #2129

Open sxa opened 4 years ago

sxa commented 4 years ago

Spun out from discussion in https://github.com/AdoptOpenJDK/openjdk-build/pull/2125#pullrequestreview-504661752

The location for changes to config parameters is something we need to provide guidance on. I came across it when putting this section of the FAQ together as things are currently in one of three places. Which to use depends on who we want to be affected and to me knowledge we've avoided being clear on where changes should be made which has already lead to confusion in the past. Quick summary:

Location Impact
groovy files (as per this PR) Only when run through our jenkins pipelines
platform-specific-configurations scripts Those using build-farm/make-adopt-build-farm.sh (inc. our pipelines) - should be stuff specific to our machines
build.sh Anyone (including end users) who are running makejdk-any-platform.sh

So it depends what we want the last line of those to be. If it's to allow users to replicate the adopt builds with the same configuration options as us as far as possible, then I think it ought to be in build.sh but if we want it to be optional for users building it themselves then the jenkins pipelines aren't a bad choice. But we should really make it clear for new people to the project where changes should be made e.g. via an update to the FAQ.

We should discuss when to use each type, citing examples of when things should be added in each of the above three places.

I would suggest:

Understanding where to make changes to the build scripts overall is also part of https://github.com/AdoptOpenJDK/openjdk-build/issues/957 but I'm creating this to limit scope a little to get this important issue clarified

karianna commented 4 years ago

We also had an issue open to split up our files between repos, I think that would help...

sxa commented 4 years ago

I wasn't too convinced by fragmenting like that, but regardless we'd need to decide what should be where, and documenting it would be a trivial first step (Well, deciding would be a good first step, then we can document)

adamfarley commented 3 years ago

The configure options set in both the build.sh and the groovy scripts should be merged into the platform scripts, and I think the scripts should then be moved under makejdk-any-platform.sh. Passing in a single flag (e.g. --use-default-config-args) could trigger them, or omitting that flag would disable them (so the scripts only use the user's config arguments).

These things are a good idea because:

sophia-guo commented 3 years ago

When I tried to implement the build-jdk action I started from readme and used makejdk-any-platform.sh to build jdk, which means platform-specific-configurations is invisible.

I wondered if we could move files under platform-specific-configurations to be same level of build.sh so jenkins, git-hub actions, users to build jdk natively, etc., no matter what build system environments it is, could also use it? Those platform-specific-configurations are platform specific, not jenkins specific, I suppose.

Groovy scripts are jenkins specifc build scripts, which will be split to separate repo https://github.com/AdoptOpenJDK/openjdk-build/issues/1108?

M-Davies commented 3 years ago

Groovy jenkins parameters have been clarified in the README.md of the jenkins repo via https://github.com/AdoptOpenJDK/ci-jenkins-pipelines/pull/67. Users should now have a good sense of scope regarding what parameters are available and where new ones should be created. #2506 adjusts the FAQ on this side of the project.

I now intend on adjusting the FAQ of this (the openjdk-build) repo to clarify that jenkins specific parameters should be done over at https://github.com/AdoptOpenJDK/ci-jenkins-pipelines/pull/67 where machine based params and global params should be done in the platform files and build.sh respectively

M-Davies commented 3 years ago

https://github.com/AdoptOpenJDK/openjdk-build/pull/2518 has been merged, completing the doc changes. This should handle anyone who wants to add new parameters to the project. The final part of this issue will be to look into our existing parameters and locations, evaluating each one for it's appropriability at its current location and, based off this evaluation, if they need to be moved to another location. This will be a lot of work however and I am unlikely to have the time to complete this task in a reasonable period of time considering multiple higher priority tasks I am handling at the moment.

As such, I'll remove my assignment and defer to someone else to reevaluate our existing parameters.