I think this looks ok. Have you tested it on Windows?
/Erik
On 2020-08-04 01:40, Galder Zamarreno wrote:
Hi all,
Could someone have a look at my updated webrev below?
Thanks
Galder
On Fri, Jul 10, 2020 at 6:48 PM Galder Zamarreno <gal...@redhat.com> wrote:
Hi again,
I've got a new webrev for this:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8248158/02/webrev/
It further expands `type -p` to replace `command -v` uses. It also
replaces $WHICH usages and removes the check for `which` as well.
I tried Simon's suggestions, but getting that right complicated the patch.
Galder
On Tue, Jul 7, 2020 at 4:31 PM Galder Zamarreno <gal...@redhat.com> wrote:
On Mon, Jul 6, 2020 at 10:50 PM Simon Tooke <sto...@redhat.com> wrote:
(Disclaimer: I am not a reviewer, so this is an opinion, not a review)
I have tested this on Windows and it built without issue, although the
submit repo should be the final gate. I'd also like to add my void to
simply redefining 'WHICH' as it leads to less changes in the source
code, which would make life easier should this be backported to 11u
and/or 8u.
So, you would just switch the UTIL_REQUIRE_PROGS call for WHICH to be
`type ...` instead?
-Simon
On 2020-07-02 4:22 a.m., Galder Zamarreno wrote:
On Thu, Jul 2, 2020 at 12:37 AM Magnus Ihse Bursie <
magnus.ihse.bur...@oracle.com> wrote:
On 2020-07-01 12:05, Galder Zamarreno wrote:
Using `which` to check whether commands exist can result in confusing
errors when `which` itself is not installed in the system. This is
the
case
with `autoconf`, where if `autoconf` is present but `which` isn't,
the
build system says that `autoconf` is missing, when in reality it is
`which`
which is missing. The fix switches autoconf uses of `which` for
`type -p`
instead, which is a Bash built-in command.
I've tested the fix with a fedora docker container that had
`autoconf`
installed but `which`. When using `type -p` it correctly detects
`autoconf`
installed and eventually fails saying that `which` is not installed,
which
is the expected behaviour.
`which` is still in use in make/autoconf/util_windows.m4. A possible
future
improvement would be to see if `which` use there could be replaced as
well.
Eventually, when no `which` uses remain, the presence check for
`which`
could be removed.
I agree that we should replace "which" with "type -p" everywhere. The
best way to do this is probably to replace the value of $WHICH with
"type -p". It's a bash built-in, so we can count on its presence. If
you
want to fix that as part of this bug, I'm ok with it, otherwise we
should open a new bug to track this. I think there is also one or two
instances of "command" recently added as (better, but not as good as
"type -p") replacements for which.
I discovered one place in util.m4 where command was being used.
There are other places outside of make/ where command is used but I
feel
those are a bit out of scope here.
My main objective is that from a configure perspective, we'd try to
reduce
the number of dependencies needed to build things.
I'll send an updated webrev shortly.
/Magnus
Bug: https://bugs.openjdk.java.net/browse/JDK-8248158
WebRev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8248158/01/webrev/
Galder
