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

unixPb: Remove hyphen from Bootjdk symlinks #3773

Merged
merged 6 commits into from
Nov 6, 2024

Conversation

Haroon-Khel
Copy link
Contributor

@Haroon-Khel Haroon-Khel commented Oct 9, 2024

  • commit message has one of the standard prefixes
  • faq.md updated if appropriate
  • other documentation is changed or added (if applicable)
  • playbook changes run through VPC or QPC (if you have access)
  • VPC/QPC not applicable for this PR
  • for inventory.yml changes, bastillion/nagios/jenkins updated accordingly

ref #2912

In light of #2912 (comment), this pr should not be merged while we have an upcoming release. And I will test the difference that this pr makes to the symlinks in our build containers by running builds in those containers

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

A block has been put on this Pull Request as this repository is temporarily under a code freeze due to an ongoing release cycle.

If this pull request needs to be merged during the release cycle then please comment /merge and a PMC member will be able to remove the block.

If the code freeze is over you can remove this block by commenting /thaw.

@Haroon-Khel Haroon-Khel marked this pull request as draft October 9, 2024 15:41
@Haroon-Khel
Copy link
Contributor Author

Ive removed the hyphen from the windows symlinks

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Oct 11, 2024

Solaris has one bootjdk, /opt/csw/java/jdk1.7.0_80, so I dont think any change is required there. On AIX we use a different format, /usr/java{{ jdk }}_64, so at the moment I am not going to change that. Only linux, windows and mac have been updated in the playbooks, and the alpine and linux build containers

@Haroon-Khel
Copy link
Contributor Author

Changes to the build scripts is here https://github.com/adoptium/temurin-build/pull/3992/files

@Haroon-Khel
Copy link
Contributor Author

@Haroon-Khel
Copy link
Contributor Author

Latest vpc, without windows as that passed in the last one
https://ci.adoptium.net/job/VagrantPlaybookCheck/1988/console

@Haroon-Khel Haroon-Khel marked this pull request as ready for review October 28, 2024 12:19
@Haroon-Khel
Copy link
Contributor Author

Latest vpc, https://ci.adoptium.net/job/VagrantPlaybookCheck/1995/console. Successes on the boxes it could connect to

@Haroon-Khel Haroon-Khel requested a review from sxa October 29, 2024 11:22
@Haroon-Khel
Copy link
Contributor Author

Without the hyphens in the symlinks the upstream configure is able to find the bootjdks without a problem, see #2912 (comment)

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Initial comment - we can hopefully remove a lot of these definitions in the dockerfiles (and elsewhere) since we are now using the default locations expected by the openjdk build process which means we don't have to have them explicitly specified. Having said that I could be ok with leaving them in for now and removing them in a future date

@sxa
Copy link
Member

sxa commented Oct 29, 2024

Solaris has one bootjdk, /opt/csw/java/jdk1.7.0_80, so I dont think any change is required there. On AIX we use a different format, /usr/java{{ jdk }}_64, so at the moment I am not going to change that.

Interesting thought ... I wonder where the openjdk build process looks by default on those platforms?

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Noting also that we have 50 machines in jenkins that have at least one _BOOT_DIR variable defined on them. Hopefully they can get cleared up too with these changes being made :-)

@karianna
Copy link
Contributor

/thaw

@github-actions github-actions bot dismissed their stale review October 30, 2024 18:35

Pull Request unblocked - code freeze is over.

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Nov 5, 2024

Im going to hold off on the windows changes. It looks like the upstream configure looks in C:\Program Files\Java/ (/cygdrive/c/Program Files/Java) which is where we already unpack the JDK on our machines

and

@sxa
Copy link
Member

sxa commented Nov 5, 2024

Im going to hold off on the windows changes. It looks like the upstream configure looks in C:\Program Files\Java/ (/cygdrive/c/Program Files/Java) which is where we already unpack the JDK on our machines

Confirmed that it does indeed "just work" with our current setup 👍🏻

@Haroon-Khel
Copy link
Contributor Author

Noting also that we have 50 machines in jenkins that have at least one _BOOT_DIR variable defined on them. Hopefully they can get cleared up too with these changes being made :-)

Ive removed the BOOT_DIR variables from jenkins nodes (as much as I could if more are found please let me know)

@Haroon-Khel Haroon-Khel merged commit 8803a16 into adoptium:master Nov 6, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants