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

Clarity on where config parameters should be defined #2129

Open
sxa opened this issue Oct 8, 2020 · 6 comments
Open

Clarity on where config parameters should be defined #2129

sxa opened this issue Oct 8, 2020 · 6 comments
Assignees
Labels
documentation Issues that request updates to our documentation enhancement Issues that enhance the code or documentation of the repo in any way help wanted Issues that need an extra hand helping out with them
Milestone

Comments

@sxa
Copy link
Member

sxa commented Oct 8, 2020

Spun out from discussion in #2125 (review)

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:

  • The platform scripts when it affects environment variables for the bulids, and including config parameters to override memory usage for our specific build machines. We do currently have things like CUDA and OpenSSL enablement for OpenJ9 in there - they could be moved into build.sh is we want to have VM specifics in there (Putting it into the groovy would result in lots more editing if they change).
  • The groovy scripts currently have things like dtrace and other openJ9 options such as JITserver in there (I was never a fan of putting them in here to be honest) although the argument in favour of it is that it's a clean way to add things specific to a java version or VM, however the definitions are somewhat opaque if you're not using jenkins. Perhaps we should keep these for jenkins-specific things like which tests suites to run by default and perhaps the options to differentiate normal and large heap builds and strip out everything else?
  • build.sh sets things like our vendor information etc. which indicates who built it, wher eto report bugs as well as separate flags for GA builds. We have things like freetype alsa, and X11 dev paths defined in there (perhaps they should be in the platform scripts). I would suggest that for maximum impact if we want adoptopenjdk consistently built in a specific way that developers can replicate most of our default configure options that impact how OpenJDK is built should be in here (or one of the scripts called form it)

Understanding where to make changes to the build scripts overall is also part of #957 but I'm creating this to limit scope a little to get this important issue clarified

@sxa sxa added the documentation Issues that request updates to our documentation label Oct 8, 2020
@sxa sxa added this to the October 2020 milestone Oct 8, 2020
@M-Davies M-Davies added the enhancement Issues that enhance the code or documentation of the repo in any way label Oct 8, 2020
@karianna
Copy link
Contributor

karianna commented Oct 8, 2020

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

@sxa
Copy link
Member Author

sxa commented Oct 9, 2020

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
Copy link
Contributor

adamfarley commented Dec 4, 2020

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:

  • There would only be one location where config arguments are set, making it easier to find and change them.
  • Moving the platform scripts under makejdk-any-platform.sh means that a docker build using our docker images only has to clone the one repo (after we're done dividing the build repo up).
  • A user can launch makejdk-any-platform.sh without having to set any configure arguments, and be assured that the build config will have all the options it needs, assuming they're using one of our docker files to build in.
  • We can set version "ranges" for each configure argument, rather than copying the same config files for each release. This means fewer files, less code duplication, and also reduces the number of things that can go wrong when we're preparing for new OpenJDK versions.

@sophia-guo
Copy link
Contributor

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 #1108?

@M-Davies
Copy link
Contributor

M-Davies commented Mar 3, 2021

Groovy jenkins parameters have been clarified in the README.md of the jenkins repo via adoptium/ci-jenkins-pipelines#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 adoptium/ci-jenkins-pipelines#67 where machine based params and global params should be done in the platform files and build.sh respectively

M-Davies added a commit to M-Davies/openjdk-build that referenced this issue Mar 5, 2021
* Further explains how and where shell script parameters should be injected
* Follow on from adoptium#2506
* Part 2 of adoptium#2129

Signed-off-by: Morgan Davies <morgandavies2020@gmail.com>
@karianna karianna modified the milestones: December 2020, March 2021 Mar 8, 2021
M-Davies added a commit that referenced this issue Mar 10, 2021
* Expand config parameter location explanation

* Further explains how and where shell script parameters should be injected
* Follow on from #2506
* Part 2 of #2129

Signed-off-by: Morgan Davies <morgandavies2020@gmail.com>
Co-authored-by: Stewart X Addison <6487691+sxa@users.noreply.github.com>
@M-Davies
Copy link
Contributor

#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.

@M-Davies M-Davies removed their assignment Mar 10, 2021
@M-Davies M-Davies added the help wanted Issues that need an extra hand helping out with them label Mar 10, 2021
@karianna karianna removed their assignment Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues that request updates to our documentation enhancement Issues that enhance the code or documentation of the repo in any way help wanted Issues that need an extra hand helping out with them
Projects
None yet
Development

No branches or pull requests

7 participants