adoptium
Now quoting all
This PR is trying to add quotes where possible. It should not harm, but to really enbale paths with spaces may go really deep into rabbit hole
I would like to understand the need for this PR. IMHO, adding quotes where possible doesn't seem necessary. Please remember to follow the KISS principle.
The non quoted strings are simply wrong. From time to time, someone use a string with space (does not meter if it is "vendor id" or "some path". All have spaces allowed. And will got a cryptic,. unrelated error later, which is wasting unnecessary time. All should be quoted by default, unless it must not be (eg list of commands). So this PR is (I'm afraid one on few more) setting a correct precedent. Every body, including myself, from time to time hurry up with something with "there never can be a space" in mind, just to be bitten later. Do not quote have nothing to do with KISS, its bug.
https://ci.adoptium.net/view/Test_grinder/job/Grinder/10674/
That is usntbale, but it is correct failure.
For sake of sanity, https://ci.adoptium.net/view/Test_grinder/job/Grinder/10678/ passed
@smlambert @sophia-guo @llxia pls reconsider
I am not enamoured with this PR. There was no report of anyone actually encountering difficulties without it.
We tend to not create fixes for problems that are not reported. This change has some potential for side-effects (especially given the quotes are around the entire variable=value part, and there was also no Grinders run against the PR branch, the ones listed were against a different branch.
If I would not hit it, I would not be fixing it. On every other project here, the quoting is mandatory. Why not here? The believe of project maintainers that users do not use spaces (..in filesystem.., not speaking about others) is always misplaced.
The grinder run is correct. I did not now better way then to make custom branch in aqa-tests, where was only single change in get.sh to pull this-chnaged-pr. https://github.com/judovana/aqa-tests/commits/myTkg/ -> https://github.com/judovana/aqa-tests/commit/537126f8daf1f551ea12839cb8f5410533aaeb5e
Sure, feel free to close. But it will just bite later, and waste another developer's time, because the error caused by ill parsed space is appearing later, and is hard to read.
It does not metter if you quote "-param=my value" or -param="my value" or even -param=m"y v"alue. All three are correct. All three used by interpreter. I picked the oen which seems most readable. If you have another preference. I will change.
May be a nit to consider - if it works, it is win win, and if it breaks anythiong, its one click to revert. So in all cases, it should do no harm.
I see, if you hit it, I would have expected to see an issue raised.
I have seen you raise PRs across the project that were not associated with an issue with a description that gives the reviewers context for the change (and suspected severity of the problem). Without an issue, it just looks like fidgeting.
Here we go: https://ci.adoptium.net/view/Test_grinder/job/Grinder/10761/ - failed. PR may be the cause.
I have no reasons to fidget in enterprise ready project. The quoting seemed like so obvious bug with so obvious fix that I was not filling issue.
Unluckily this, or even openjdk, and many other big projects share this behaviour - issues do not get attention unless they are really critical. But PRs do have attention, as there is obvious volunteer to do the work.
All my PRs are based on simple effort to use that, and I keep stumbling around smaller or bigger bugs or missing features. I would say one would be happy that somebody is trying it on non default platforms and in various corenrcases:(
yah, looks better: https://ci.adoptium.net/view/Test_grinder/job/Grinder/10762/console