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

Ensure Q is defined in moveDmp.mk to prevent unquoted test on Solaris #157

Merged
merged 1 commit into from
Mar 17, 2021

Conversation

sxa
Copy link
Member

@sxa sxa commented Mar 11, 2021

It may be the case that this should have already included the definition via settings.mk or https://github.com/AdoptOpenJDK/TKG/blob/cca8f28c9c557037d8a04f7d097250fc53226d0d/makeGen.mk#L28 but either way setting it in the file works. If there is a preferred solution to obtain it via the inclusion of one of the other files I'm happy to do that instead.

(As an aside this would also work if you ran with make SHELL=bash compile but for portability that's not the best option here unless things get problematic further down the line)

Signed-off-by: Stewart X Addison sxa@redhat.com

Signed-off-by: Stewart X Addison <sxa@redhat.com>
@sxa sxa added the bug Something isn't working label Mar 11, 2021
@sxa sxa requested review from smlambert and Mesbah-Alam March 11, 2021 12:20
@sxa sxa self-assigned this Mar 11, 2021
@sxa
Copy link
Member Author

sxa commented Mar 11, 2021

For historic reference this was raisd as a result of Grinder 7578 failing with this error:

14:28:20  if [ -z $(tail -1 /export/home/jenkins/workspace/Grinder/openjdk-tests/TKG/../TKG/output_compilation/compilation.log | grep 0) ]; then perl scripts/moveDmp.pl --compileLogPath=/export/home/jenkins/workspace/Grinder/openjdk-tests/TKG/../TKG/output_compilation/compilation.log --testRoot=/export/home/jenkins/workspace/Grinder/openjdk-tests/TKG/..; false; else rm -f -r /export/home/jenkins/workspace/Grinder/openjdk-tests/TKG/../TKG/output_compilation; fi
14:28:20  /bin/sh: syntax error at line 4: `(' unexpected
14:28:20  gmake: *** [makefile:75: compileTools] Error 2

@smlambert smlambert requested a review from renfeiw March 11, 2021 12:32
Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

Q is set here: https://github.com/AdoptOpenJDK/TKG/blob/master/settings.mk#L38 so would like to understand if/why we have a case were moveDmp is used before settings, and if there is a preferred way to handle it besides defining it twice

@renfeiw
Copy link
Contributor

renfeiw commented Mar 11, 2021

moveDmp.mk doesn't include settings.mk at this moment. Since only Q is needed, I think it's not preferred to include setting.mk. So we can have Q added in moveDmp.mk like in this change. I think a better way is to create another small mk file which contains Q, D, etc. and we can include it wherever we need. But this can be a future improvement.

@smlambert smlambert removed the request for review from Mesbah-Alam March 17, 2021 14:33
@smlambert smlambert merged commit 921bfaa into adoptium:master Mar 17, 2021
@karianna karianna added this to the March 2021 milestone Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants