I ran this patch through our internal system (corresponds to the submit
repo), so it's working for us at least.
/Erik
On 2020-08-04 06:25, Galder Zamarreno wrote:
Hi Erik,
I've been in contact with Simon (cc) who has been testing the patch on
Windows and he said it all looked good.
@Simon Tooke <mailto:sto...@redhat.com>, do you need to verify again?
Galder
On Tue, Aug 4, 2020 at 2:40 PM Erik Joelsson <erik.joels...@oracle.com
<mailto:erik.joels...@oracle.com>> wrote:
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 <mailto: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 <mailto:gal...@redhat.com>> wrote:
>>
>>>
>>> On Mon, Jul 6, 2020 at 10:50 PM Simon Tooke <sto...@redhat.com
<mailto: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
<mailto: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
>>>>
