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

WindowsPB: Include cmake in cygwin package list #1958

Merged
merged 2 commits into from
Feb 25, 2021

Conversation

Haroon-Khel
Copy link
Contributor

Ive removed the cmake role from the main.yml of the windows playbook since we use cygwin's cmake for openj9 on windows. Ive also included cmake as the list of packages that will get installed with cygwin.

Will test on one of the alibaba machines shortly

Copy link
Contributor

@Willsparker Willsparker left a comment

Choose a reason for hiding this comment

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

Given we're removing cmake from main.yml, should we remove the whole role? ping @sxa

@karianna karianna added this to the February 2021 milestone Feb 22, 2021
@sxa
Copy link
Member

sxa commented Feb 22, 2021

Given we're removing cmake from main.yml, should we remove the whole role? ping @sxa

Since I don't think we call it from any other playbooks I'd be ok with removing it completely since it seems unlikely we would put it back in - I'm not aware of anything else that would still need it (Second opinion from @AdamBrousseau might be useful).

@sxa
Copy link
Member

sxa commented Feb 22, 2021

Will test on one of the alibaba machines shortly

How did the testing go?

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Feb 22, 2021

Seemed to do the trick. On alibaba-2:

$ ls -la cygwin64/bin/ | grep cmake
-rwxr-xr-x    1 Administrator None  5211155 Aug 10  2020 ccmake.exe
-rwxr-xr-x    1 Administrator None  5622291 Aug 10  2020 cmake.exe

Which were definitely not there before (previously there was only a link to the Program Files cmake). Running a jdk11 openj9 job on the machine
https://ci.adoptopenjdk.net/job/build-scripts/job/jobs/job/jdk11u/job/jdk11u-windows-x64-openj9/

@Haroon-Khel
Copy link
Contributor Author

Fails before getting to the cmake part, with this error

13:44:48  checking for OPENSSL... configure: error: Cannot locate the the path of OPENSSL_DIR
13:44:48  configure: The path of OPENSSL_DIR, which resolves as "/cygdrive/c/openjdk/OpenSSL-1.1.1j-x86_64-VS2017", is invalid.

Its never hit this before. Investigating

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Feb 22, 2021

I see that the openssl version was recently updated from i to j #1947
Ill rerun the openssl role on alibaba -2

@Haroon-Khel
Copy link
Contributor Author

@sxa
Copy link
Member

sxa commented Feb 22, 2021

Copy link
Contributor

@AdamBrousseau AdamBrousseau left a comment

Choose a reason for hiding this comment

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

We do call the role when we run the playbooks internally but upon investigation, we don't use and we shouldn't be installing it so I'm fine with removing it completely.

Copy link
Contributor

@Willsparker Willsparker left a comment

Choose a reason for hiding this comment

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

As per the above conversation, just remove the standalone cmake roles and I'll approve 👍

@Haroon-Khel
Copy link
Contributor Author

@Willsparker Cmake role removed 👍🏻

Copy link
Contributor

@Willsparker Willsparker left a comment

Choose a reason for hiding this comment

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

Thanks!

@Haroon-Khel Haroon-Khel merged commit c933d1b into adoptium:master Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants