On Tue, 25 Mar 2025 11:36:01 GMT, Magnus Ihse Bursie <[email protected]> wrote:
>>> Also, the addition of the `executable` test was good! Please re-instantiate >>> it. >>> >>> I'm not sure if @tstuefe meant that it should be removed? If so, why? To be >>> able to pass in a shell script that does not have the executable bit set? >>> That's kind of weird. How do we even know it is a bash script then? It >>> might just as well be a ksh or csh script, so just assuming bash would be >>> strange. >> >> I removed the check because I noticed that `UTIL_FIXUP_EXECUTABLE` already >> performs such a check and so it was redundant. > >> I removed the check because I noticed that UTIL_FIXUP_EXECUTABLE already >> performs such a check and so it was redundant. > > I see. > > The benefit of your solution was that the error message is tied to the > configure argument and is more clear to the user. The check in > UTIL_FIXUP_EXECUTABLE is more of a fallback insurance, and if it is > triggered, the error message written might not be clear to the user what went > wrong. So I still think it serves a purpose, to verify incoming arguments > up-front, and it fit very nicely into the UTIL_ARG_WITH framework. @magicus is the leading build engineer. Please feel free to ignore anything I suggest if he argues against it :-) ------------- PR Comment: https://git.openjdk.org/jdk/pull/23807#issuecomment-2751136369
