Skip to content

fix: "Not a Target" logic bug in make helper awk script#1577

Open
rnvannatta wants to merge 1 commit intoscop:mainfrom
rnvannatta:main
Open

fix: "Not a Target" logic bug in make helper awk script#1577
rnvannatta wants to merge 1 commit intoscop:mainfrom
rnvannatta:main

Conversation

@rnvannatta
Copy link
Copy Markdown

In some obscure cases, target: VAR = val can cause make t to autocomplete to the incorrect result.

This is happening in one of my projects with a somewhat complex set of makefiles, such that make w returns windows, webgl, and a random build file. A silly, though minimal repro is:

all: fluff
fluff:
	touch fluff
foobar: OUCH = glitch

where make f will erroneously list completion possibilities all fluff

The root cause is that the foobar 'block' looks like this

# makefile (from 'Makefile', line 4)
foobar: OUCH = glitch
# Not a target:
foobar:
#  Implicit rule search has not been done.
#  Modification time never checked.
#  File has not been updated.
# variable set hash-table stats:
# Load=1/32=3%, Rehash=0, Collisions=0/2=0%

the script matches f against foobar, setting the target flag true, then it encounters # Not a target over and over and over again until it hits the all target, and because the target flag is true, it accepts that. The solution is to make # Not a target reset the flag as it skips.

You can test the script in isolation with: make -npq __BASH_MAKE_COMPLETION__=1 -C . .DEFAULT 2>/dev/null | prefix="f" prefix_replace="f" awk -f make-extract-targets.awk

Tested on Debian Trixie with GNU Make 4.4.1, the latest in Debian.

Here is the output of make -nqp on the minimal repro makefile for your convenience: make-nqp-output.txt

@rnvannatta rnvannatta changed the title Fix "Not a Target" logic issue in make autocompletion awk script fix: "Not a Target" logic issue in make autocompletion awk script Mar 2, 2026
@rnvannatta rnvannatta changed the title fix: "Not a Target" logic issue in make autocompletion awk script fix: "Not a Target" logic bug in make helper awk script Mar 2, 2026
@rnvannatta
Copy link
Copy Markdown
Author

Fixed the lint issues with the commit message

@yedayak
Copy link
Copy Markdown
Collaborator

yedayak commented Mar 21, 2026

How do you feel about adding a test? You could probably just add it to test/fixtures/make/Makefile and change test/t/test_make.py to test it

# should be output.

# only process the targets the user wants.
starts_with($0, prefix) { is_target_block = 1; }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this PR is the right fix for the present issue. In the first place, does anyone have an idea of the reason that we start the block with the matching line and include also unmatching lines in the same block? Isn't this the root cause of the present issue? I translated the original sed script in #1069 into AWK so that the behavior wouldn't be changed, but the behavior seems a mystery. This behavior seems to have existed from the beginning since the analysis was introduced in commit b28d710 by @code5hot.

Copy link
Copy Markdown
Author

@rnvannatta rnvannatta Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of an obscure issue for a less widely used Makefile feature, that for some reason make outputs a slightly different formatted message for.

Normally 'blocks' that aren't targets have their first line be "Not a target:".

In this case, the "Not a target:" comes after the line that passes the starts_with() awk test.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't ask why the present PR fixes the issue. I know that this PR does fix the present specific issue, but that doesn't immediately mean that's the right fix. If the root cause lies in the original approach at all, the root cause should be fixed instead of patching every downstream symptom separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants