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

feat: make sure for both cygwin and msys cygpath is used #343

Closed
wants to merge 4 commits into from

Conversation

zdtsw
Copy link
Contributor

@zdtsw zdtsw commented Aug 10, 2022

No description provided.

@smlambert smlambert requested a review from renfeiw August 11, 2022 02:31
@llxia
Copy link
Contributor

llxia commented Aug 11, 2022

Please provide the Grinder links for both cases. Thanks

@smlambert
Copy link
Contributor

What issue does this address?

@zdtsw
Copy link
Contributor Author

zdtsw commented Aug 12, 2022

for x86_64-linux https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/5417
for x86-32_windows https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/5418
for x86-64_mac https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/5419
for ppc64_aix https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/5420

What issue does this address?

when I was checking why run-aqa/aqa-tests not working on windows-2019, i found the code in temurin-build https://github.com/adoptium/temurin-build/blob/0aee022f86093836998a48b9795002fa6afeb817/sbin/prepareWorkspace.sh#L420 checks on both cygwin and mingw convert path. I do not know if this could be problem if we use a newer version or anyone wants to use AQA but install with mingw on their windows, just think it is good to align the logic in both code.

settings.mk Outdated Show resolved Hide resolved
@llxia
Copy link
Contributor

llxia commented Aug 15, 2022

@zdtsw what error did you encounter on windows-2019? Did this PR resolve the problem?

@zdtsw
Copy link
Contributor Author

zdtsw commented Aug 15, 2022

@zdtsw what error did you encounter on windows-2019? Did this PR resolve the problem?

the error can be found from https://github.com/adoptium/run-aqa/runs/7789840940?check_suite_focus=true it is on 2022/latest.
two issues are reported for this #342 and adoptium/run-aqa#133

the PR is not for fix the problem, but only to match what we have in the build script's logic to support both cygwin and minGW.

@smlambert
Copy link
Contributor

Given cygwin is a stated prereq for AQAvit, this change is likely not warranted.

@zdtsw
Copy link
Contributor Author

zdtsw commented Aug 15, 2022

Given cygwin is a stated prereq for AQAvit, this change is likely not warranted.

right! this PR probably not needed for the current requirement.
what I am worrying is, cygwin might not work with later windows version, if that is the case(probably as one item to investigate) then it either be limited to 2016 or need re-write logic of cygwin installation to something else or use wsl. https://cygwin.com/pipermail/cygwin-announce/2022-January/010438.html

@smlambert
Copy link
Contributor

Yes, in which case we should create an issue and discuss and design an approach, not just needle in a PR here or there without thinking of all of the implications (and testing that will need to happen) if we want to support a new environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants