bugmake - Bugs: bug #65273, Potential bug in the info function?

 
 

bug #65273: Potential bug in the info function?

Submitter:  Collin Funk <collinfunk>
Submitted:  Thu 08 Feb 2024 02:01:44 AM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Wont Fix Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Component Version:  SCM Operating System:  POSIX-Based
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 21 Apr 2024 05:42:32 PM UTC, comment #10: 

I have decided, for now, to keep the new behavior.  I'm open to continued discussion.

Paul D. Smith <psmith>
Group administrator
Sun 24 Mar 2024 04:58:48 PM UTC, comment #9: 

Note that the trick in John's book no longer works without warnings in the current Git-based code:


$ ./make
Makefile:3: warning: invalid variable name ' '
Makefile:4: warning: invalid variable reference ' '
Makefile:4: [ ]
make: *** No targets.  Stop.


Paul D. Smith <psmith>
Group administrator
Sat 10 Feb 2024 01:14:06 AM UTC, comment #8: 

comment #3:

> The problem is that (a) there is no way to create a variable name containing, or consisting of, whitespace


Sure there is:

Makefile:
```
SPACE :=
SPACE := $(SPACE) $(SPACE)
$(SPACE) := $(SPACE)
$(warning [$( )])
```

Output:
```
Makefile:4: [ ]
make: * No targets.  Stop.
```

I'm not sure where I learned this trick, but I think it was probably the John Graham-Cumming book, so it's probably relatively widely known.

(Admittedly this will cause a warning with the latest HEAD.)

Ian Prest <ijprest>
Sat 10 Feb 2024 12:14:39 AM UTC, comment #7: 

comment #6:

> Leaving these checks disabled by default is not a good solution since the very people who need this help most, will not benefit from them.


I agree. It took me a while to understand some of the GNU Make extensions a few years ago. I think the warnings would have helped me.

comment #6:

> Of course we COULD add "special case" facilities for various projects' special tricks but I'm not a fan of this approach.


Sounds like it would be a pain to maintain. Paul Eggert said the "$ " trick was just for Emacs.
It was changed to your "$." trick yesterday and seems to be working fine so far.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68996

Thanks again for the help.

Collin Funk <collinfunk>
Fri 09 Feb 2024 11:11:19 PM UTC, comment #6: 

I mean, there IS a reasonable way to get the same effect; I showed one possibility in my response.  Yes, it looks slightly different but the result is the same and there are no warnings.  Alternatively the Emacs makefiles could disable these new warnings, if they are deemed more annoying than helpful.

The problem is that at least 70% of the questions I've been answering on StackOverflow in the last 5 years more or less boil down to misunderstanding how make handles expansion, and a lot of those misunderstandings could be helped if make complained about invalid variable names; for example:

all:
        echo $(git rev-parse HEAD)

will now show a warning (which can be either disabled or converted into an error via options) because "git rev-parse HEAD" is not a valid variable name.

Leaving these checks disabled by default is not a good solution since the very people who need this help most, will not benefit from them.

Of course we COULD add "special case" facilities for various projects' special tricks but I'm not a fan of this approach.  And for this specific case, we would be disabling warnings for situations like:

all:
        echo $ (CURDIR)

which admittedly is probably rare but still seems like it is a bizarre special case to keep.

Paul D. Smith <psmith>
Group administrator
Fri 09 Feb 2024 06:26:30 AM UTC, comment #5: 

FWIW, I think it is very unfortunate that GNU Make started warning about this trick, especially if there is no reasonable way of getting the same effect by other means.

Eli Zaretskii <eliz>
Group Member
Fri 09 Feb 2024 12:30:03 AM UTC, comment #4: 

comment #3:

> It's usually a good idea to check the NEWS file for things that might cause differences in behavior


Thanks. I'll do that next time first. :)
I grep'd for the error message and found it through git blame so it didn't take too long.

comment #3:

> Unfortunately I can't think of a good way to continue to allow this to work, other than simply disabling the warning by default which I'd really prefer to not do.


I agree with your preference. The warning seems useful to catch actual bugs. The Emacs Makefile was just written before it existed.

comment #3:

> I wonder how common this particular little trick is, out in the world.


I'm surprised no one else has ran into it and reported it since the change is a year old. I'll submit a bug or email the Emacs people about it later today. I assume the goal was to make it output in the same way that ./configure --enable-silent-rules would. Removing the whitespace would cause the output to look different but would silence the warnings so they aren't interpreted as a bug like I did. If I manage to think of a decent syntax for adding whitespace I'll propose it on the mailing list. Thanks for the help! Feel free to mark this as "Not a bug" if you'd like. I'm not sure how to on Savannah.

Collin Funk <collinfunk>
Thu 08 Feb 2024 01:55:02 PM UTC, comment #3: 

It's usually a good idea to check the NEWS file for things that might cause differences in behavior; in this case see:

https://git.savannah.gnu.org/cgit/make.git/tree/NEWS#n68

I understand what the Emacs makefile is trying to do: by adding this invalid variable reference, which used to be silently ignored, they are adding indentation.  For example this:

$(info          foo)

shows the string:

foo

because leading whitespace is ignored for all functions and arguments in GNU Make.

But this:

$(info $         foo)

shows the string:

        foo

since the invalid variable "$ " ends the leading whitespace and expands to nothing.

Unfortunately I can't think of a good way to continue to allow this to work, other than simply disabling the warning by default which I'd really prefer to not do.

The problem is that (a) there is no way to create a variable name containing, or consisting of, whitespace, and (b) modifying the warning check to allow a variable of just a single space without comment seems to me to be allowing an extremely common potential error to be silently accepted.

I wonder how common this particular little trick is, out in the world.  Of course it could easily be changed to do something like:

. :=
$(info $.          foo)

but it does involve a change.

Paul D. Smith <psmith>
Group administrator
Thu 08 Feb 2024 08:56:36 AM UTC, comment #2: 

Finally got around to looking into this. From what I can tell it was an intentional change from commit 03ecd94488b85adc38746ec3e7c2a297a522598e. The previous commit doesn't warn and one referenced onward can be controlled with --warn=(ignore|warn|error). Can you confirm that I am not mistaken? If not then the Makefile should be pretty easy to fix for the Emacs people. Thanks.

Collin Funk <collinfunk>
Thu 08 Feb 2024 03:53:04 AM UTC, comment #1: 

I was able to reproduce this on a FreeBSD VM. The GNU Make in ports is version 4.3 and does not output the warnings. When using gmake from master they are outputted. On a separate note, FreeBSD fails when building in maintainer mode due to a #warning preprocecessor directive in sys/timeb.h. I've attached a patch fixing the problem.

(file #55675)

Collin Funk <collinfunk>
Thu 08 Feb 2024 02:01:44 AM UTC, original submission:  

Hello, I believe I found a bug in the way that the $(info ...) function is processed by GNU Make. I noticed it building Emacs from master. With both Make built from master and tag 4.4.1 building Emacs outputs a lot of undefined variable references like so:

../src/verbose.mk:58: warning: invalid variable reference ' '
  CC       print.o
../src/verbose.mk:58: warning: invalid variable reference ' '
  CC       lread.o
../src/verbose.mk:58: warning: invalid variable reference ' '
  CC       emacs-module.o
../src/verbose.mk:58: warning: invalid variable reference ' '
  CC       syntax.o
../src/verbose.mk:58: warning: invalid variable reference ' '

The file that causes the warning can be found here:

https://git.savannah.gnu.org/cgit/emacs.git/tree/src/verbose.mk.in

This does not occur with GNU Make 4.3 packaged in Debian Stable. I'll try to look for the cause a bit later but I am not too sure how the $(info ...) function works. Hopefully I gave enough information to reproduce it. Thanks!

Collin Funk <collinfunk>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ijprest (Posted a comment)
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by collinfunk (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-04-21 psmith StatusNone Wont Fix
        Open/ClosedOpen Closed
    2024-02-08 psmith Component Version4.4.1 SCM
    2024-02-08 collinfunk Attached File- Added 0001-maint-Don-t-include-sys-timeb.h-on-FreeBSD.patch, #55675

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code