On 2023/05/28 13:16:34 +0200, Marc Espie <marc.espie.open...@gmail.com> wrote:
> Turns out that, due to the search rules we use, make
> is mostly silent in case it couldn't read a Makefile
> 
> The following patch lets it track the makefilenames that
> do exist, but that it wasn't able to open, so that
> a post-mortem can include these.
> 
> 
> Not sure if other use-cases could also use the post-mortem
> 
> The tweak to ReadMakefiles is to avoid pushing the same
> path name twice.
> 
> Using Lst_Pop ensures this diagnostic only ever occurs once,
> in case we find other places we might want to stick this.
> 
> (and realizing that, I should probably add comments in both
> places instead of explaining this through email)

It would indeed be nice to have a "post-mortem" message with the
Makefiles that couldn't be opened.  it would have saved me some
headaches in the past with ports that have too tight permissions.

Diff below reads fine to me, just one question inline, but it doesn't
seem to catch all the situations, consider:

        % mkdir /tmp/foo && cd /tmp/foo
        /tmp/foo
        % touch Makefile
        % chmod 000 Makefile
        % make
        make: no target to make.
        % make all
        make: don't know how to make all
        Stop in /tmp/foo
        Makefile(s) that couldn't be read: Makefile

a bare `make' don't print the nice error message, trying with a target
instead does.

> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/make/main.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 main.c
> --- main.c    17 Jan 2023 13:03:22 -0000      1.129
> +++ main.c    28 May 2023 11:12:48 -0000
> [...]
> +static FILE *
> +open_makefile(const char *fname)
> +{
> +     FILE *stream;
> +     struct stat buffer;
> +
> +     stream = fopen(fname, "r");
> +     if (stream != NULL)
> +             return stream;
> +
> +     if (stat(fname, &buffer) == 0) {
> +             Lst_AtEnd(&unreadable, strdup(fname));
> +     }

Why the stat?  Can't we just check errno for EACCES instead?  EACCES
is also returned when search permission is denied for a component of
the path prefix, which would be a difference but maybe a nice one?

> +     return NULL;
> +}
> +

Reply via email to