Bug 246615

Summary:

devel/gmake: PATH search may match a directory instead of a file

Product:

Ports & Packages

Reporter:

Jung-uk Kim <jkim>

Component:

Individual Port(s)

Assignee:

Tijl Coosemans <tijl>

Status:

Closed FIXED

Severity:

Affects Only Me

CC:

jkim

Priority:

---

Flags:

bugzilla: maintainer-feedback? (tijl)

Version:

Latest

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
Work around PATH search bug	none
Add an upstream patch	none

Description Jung-uk Kim freebsd_committer

2020-05-20 20:16:14 UTC

Test case:

% ls -ld foo
drwxr-xr-x  2 jkim  staff  512 May 20 19:43 foo
% cat bin/foo
#!/bin/sh
echo This is ~/bin/foo.
echo \$PATH is \'$PATH\'.
% cat Makefile
all:
	foo

Expected behavior (before GNU make 4.3):

% env PATH=.:$PATH gmake
foo
This is /home/jkim/bin/foo.
$PATH is '.:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/home/jkim/bin'.

Current behavior (after GNU make 4.3):

% env PATH=.:$PATH gmake
foo
gmake: foo: Permission denied
gmake: *** [Makefile:2: all] Error 127

Note GNU make 4.3 uses posix_spawn(3) instead of fork()/exec().

http://git.savannah.gnu.org/cgit/make.git/commit/?id=749a54d7a458dc6779936138caf40ce600a80052

Now they implemented a built-in PATH search.

http://git.savannah.gnu.org/cgit/make.git/commit/?id=60905a8afb012aa38ac6d56cee24754cc678947c

This new find_in_given_path() function seems to be the culprit.  A workaround is using "--diasable-posix-spawn" configure option.

Comment 1 Jung-uk Kim freebsd_committer

2020-05-20 20:18:35 UTC

(In reply to Jung-uk Kim from comment #0)
I meant "--disable-posix-spawn".  Sorry for the typo.

Comment 2 Jung-uk Kim freebsd_committer

2020-05-20 20:26:41 UTC

Created attachment 214701 [details]
Work around PATH search bug

Comment 3 Jung-uk Kim freebsd_committer

2020-05-20 21:55:57 UTC

Interestingly, gnulib has added this check to findprog.c recently but not to findprog_in.c.

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=4c1009ec93e12ee34acd27f6d7e25442bedc16f2

Comment 4 Jung-uk Kim freebsd_committer

2020-05-20 22:46:17 UTC

I found the upstream PR:

https://savannah.gnu.org/bugs/index.php?57962

Comment 5 Jung-uk Kim freebsd_committer

2020-05-20 22:47:29 UTC

Created attachment 214702 [details]
Add an upstream patch

This upstream patch works, too.

Comment 6 commit-hook freebsd_committer

2020-05-21 09:51:24 UTC

A commit references this bug:

Author: tijl
Date: Thu May 21 09:50:37 UTC 2020
New revision: 536096
URL: https://svnweb.freebsd.org/changeset/ports/536096

Log:
  Add backport of gnulib git commit 4c1009ec93e12ee34acd27f6d7e25442bedc16f2.

  When the file found in a PATH element is a directory, continue searching.

  PR:		246615
  Submitted by:	jkim
  Obtained from:	https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=4c1009ec93e12ee34acd27f6d7e25442bedc16f2

Changes:
  head/devel/gmake/Makefile
  head/devel/gmake/files/patch-10-4c1009ec

Comment 7 Jung-uk Kim freebsd_committer

2020-05-21 19:30:30 UTC

(In reply to commit-hook from comment #6)
Note it is not exactly "backport".  The attached patch was initial submission and the committer modified the logic to "reject directories" rather than "only accept regular files".  IMHO, it is wrong, though.
Also. the initial patch was just for findprog_in.c and the commit just fixed findprog.c and progreloc.c.

Comment 8 Tijl Coosemans freebsd_committer

2020-05-21 21:54:43 UTC

(In reply to Jung-uk Kim from comment #7)
Thanks, I've notified upstream about findprog vs findprog-in.  The patch I committed has !S_ISDIR like the gnulib commit.

Comment 9 Tijl Coosemans freebsd_committer

2020-06-01 16:16:43 UTC

(In reply to Tijl Coosemans from comment #8)
Fixed in https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=6e6abd0cdfe4bb96f6412aebc511f10bf254a820.

Comment 10 Jung-uk Kim freebsd_committer

2020-06-09 16:50:44 UTC

(In reply to Tijl Coosemans from comment #9)
Cool!  Can you please import this version?

Comment 11 Jung-uk Kim freebsd_committer

2020-06-09 18:27:40 UTC

(In reply to commit-hook from comment #6)
Actually, it didn't work for me and I found this patch is slightly broken.

# patch < ../../files/patch-10-4c1009ec
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Backport of gnulib git commit 4c1009ec93e12ee34acd27f6d7e25442bedc16f2.
|
|When the file found in a PATH element is a directory, continue searching.
|
|--- lib/findprog-in.c.orig     2020-01-19 20:34:01 UTC
|+++ lib/findprog-in.c
--------------------------
Patching file lib/findprog-in.c using Plan A...
Hunk #1 succeeded at 26.
Hunk #2 succeeded at 191.
Hmm...  Ignoring the trailing garbage.
done

That's because '@' is missing, i.e.,

--- patch-10-4c1009ec.orig
+++ patch-10-4c1009ec
@@ -20,7 +20,7 @@
          for (i = 0; i < sizeof (suffixes) / sizeof (suffixes[0]); i++)
            {
              const char *suffix = suffixes[i];
-@ -208,7 +210,8 @@ find_in_given_path (const char *progname, const char *
+@@ -208,7 +210,8 @@ find_in_given_path (const char *progname, const char *
                     use it.  On other systems, let's hope that this program
                     is not installed setuid or setgid, so that it is ok to
                     call access() despite its design flaw.  */

Comment 12 commit-hook freebsd_committer

2020-06-10 10:55:02 UTC

A commit references this bug:

Author: tijl
Date: Wed Jun 10 10:54:25 UTC 2020
New revision: 538386
URL: https://svnweb.freebsd.org/changeset/ports/538386

Log:
  Fix broken patch (missing @) by using the upstream version now that it's
  available.

  PR:		246615
  Reported by:	jkim
  Obtained from:	https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=6e6abd0cdfe4bb96f6412aebc511f10bf254a820

Changes:
  head/devel/gmake/Makefile
  head/devel/gmake/files/patch-10-4c1009ec
  head/devel/gmake/files/patch-10-6e6abd0c

Comment 13 Tijl Coosemans freebsd_committer

2020-06-10 11:01:40 UTC

Apologies for the breakage.  No idea how that happened.  Must have been manually editing your patch.