Fix infinite loop when --dry-running autoconf projects.

[drok]

When --dry-running autoconf projects, the default make invocation flags, along causes the dry-run process to enter an infinite loop where the Makefile is updated, by running config.status, which requires reconfiguration.

The problem

The infinite loop when dry-running autoconf projects is due to GNU Make's documented behaviour. It's not a bug, or version-specific behaviour:

1. GNU Make remakes files makefiles unconditionally (defined as files it reads rules from, eg, given a statement "include blah.inc", the file blah.inc is a "makefile"). Remaking Makefiles says "--dry-run does not prevent updating of makefiles, because an out-of-date makefile would result in the wrong output for other targets"

2. Automake creates rules for updating the Makefile when Makefile.am changes, and when configure.ac changes.

3. In recursive builds, when make spawns a sub-make process, it does not pass "--assume-old" arguments to child make via the MAKEFLAGS mechanism. This is described in Communicating Options to a Sub-make

Together, these three behaviours guarantee that an autoconf+automake project will end up in an infinite loop when "--dry-run" as the vscode "Makefile Tools" extension does by default when projects are loaded.

Solution

The solution is to change the command line options as follows (settings.json excerpt):

"makefile.dryrunSwitches": [
        "--always-make",
        "--print-directory",
        "--dry-run",
        "--assume-old=Makefile",
        "Makefile",
        "all",
        "AM_MAKEFLAGS=--assume-old=Makefile Makefile"
],

If using the GUI, each of these strings must appear on one line. Specifically, AM_MAKEFLAGS=--assume-old=Makefile Makefile is one argument. In a bash command line, this would be quoted like AM_MAKEFLAGS="--assume-old=Makefile Makefile"

Explanation

The --assume-old=Makefile means the file named 'Makefile', which is also a target, will not be considered for remake, in spite of --always-make, per Avoiding Compilation.

Since the file Makefile is also a file (ie, used to read rules from), it would be re-made in spite of "--dry-run". This behaviour is overriden by specifying Makefile as a target, in addition to all, as documented in Remaking Makefiles: "on occasion you might actually wish to prevent updating of even the makefiles. You can do this by specifying the makefiles as goals in the command line as well as specifying them as makefiles. When the makefile name is specified explicitly as a goal, the options ‘-t’ and so on do apply to them."

Since on recursive builds the --assume-old argument would not be passed to the sub-make via make's own MAKEFLAGS environment variable mechanism, the AM_MAKEFLAGS variable is used to perform override this behaviour. This works because automake projects add the value of this variable on every make invocation.

Note 1. If your project defines the AM_MAKEFLAGS variable in Makefile.am, you'll need to adjust it to include the needed --assume-old incantation. One way would be:

AM_MAKEFLAGS="--project-specific-make-flags $(DRYRUN_MAKEFLAGS)"

Then, use use DRYRUN_MAKEFLAGS="--assume-old=Makefile Makefile" in settings.json.

Note 2. Which of the settings.json files you put the "makefile.dryrunSwitches" settings matters. If you work on autoconf as well as cmake projects, you might want it in each project's settings.json. I have it set at the user level, as I work on very few cmake projects, and then I can override in at the project level.

Marked WIP because of these issues:

1. adding "all" as a target makes the assumption that "all" is the default target, which may not be true, especially in non-automake projects.
2. The workaround for recursive sub-makes only works if automake is in use. Otherwise, another way to pass --assume-old to sub-makes should be found.

The problem was reported at StackOverflow, and this is my solution to that question.

This commit changes the default makefile.dryrunSwitches flags as documented above.

[andreeis]

@drok, this is very cool. Even if you don't finish the PR, this is a great pointer for us to continue. I wasn't aware of this possible solution to the infinite loop problem which I considered by design and I thought only removing always-make could somewhat help (with other negative consequences in some scenarios).

Regarding passing the flags to the sub-makes. I will test soon. Did I understand right from the recursion link you provided, that we can define MAKEFLAGS in the environment of the main process and so all the sub-make process will see it? The extension can inject this variable into the environment before we spawn the first dry-run. So maybe we don't need AM_MAKEFLAGS at all?

Regarding target "all". Why is that needed? How about we let the default target or whichever target the user defines via the makefile UI? Just let me know what would happen to the command line if "all" would be missing or replaced by some other "build1" for example target.

Regarding the makefile specified as target and as assume_old. Sometimes the user points to a makefile somewhere else (or even named differently) via "makefile.makefilePath". Would work the same if instead of the simple most common case of "Makefile" we would have "dir1/dir2/MyMakefile"? I'll test but you can also answer if you happen to know or it's quick to try.

[drok]

Hi Andrea, thank you for taking up this issue; I will answer your questions below:

MAKEFLAGS injected in the top make environment

Did I understand right from the recursion link you provided, that we can define MAKEFLAGS in the environment of the main process and so all the sub-make process will see it

This would not work. MAKEFLAGS is a special environment variable that make itself sets in order to communicate with the sub-makes. If you define it the main process's environment it will only have an effect in the top make, but not the sub-makes. The top main process does not propagate it, but overrides it before forking the sub-make.

The newly sub-make's MAKEFLAGS inherits most of the intent of the top make's MAKEFLAGS. Specifically, as documented in Communicating Options to a Sub-make:

The options ‘-C’, ‘-f’, ‘-o’, and ‘-W’ are not put into MAKEFLAGS; these options are not passed down.

-o is a synonym for --assume-old (I use the full-length options because they are self-documenting)
-f means --makefile

Without these arguments, the sub-make will attempt to remake its makefile, and you're back in an infinite loop, eg, if the subdir is a nested autoconf project.

Besides, it would not be correct for the IDE to override MAKEFLAGS to pass arguments. This variable may be set to enforce local administrative policy, like --load-average, --include-dir or --no-builtin-rules. These affect the output of the dry-run, so at best you'd have to doctor an existing setting to preserve the intent.

Why is AM_MAKEFLAGS needed?

So maybe we don't need AM_MAKEFLAGS at all?

On the contrary. AM_MAKEFLAGS is the secret sauce that makes this solution work, and in fact makes it work reliably only when the makefiles are generated by automake.

The contents of AM_MAKEFLAGS are propagated to submakes, unlike the contents of MAKEFLAGS which are re-written to filter out a few essential flags as shown above. The incantation automake generates is $(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS) $$local_target

automake-generated makefiles explicitly give every sub-make the contents of AM_MAKEFLAGS as additional command-line arguments.

In the case of manually-written makefiles, where submakes are typically invoked with the incantation $(MAKE) -C subdir target, there is no straightforward way to pass the --assume-old flag from top to sub-make. In fact, the makefile in the subdirectory could be called other-Makefile, which would require the incantation $(MAKE) -C subdir --makefile=other-Makefile --assume-old=other-Makefile other-Makefile target. The --always-make and --dry-run flags would be propagated via the MAKEFLAGS mechanism

The solution in this PR assumes that all makefiles along the hierarchy of subdirectories are named Makefile, which is a good assumption most of the time but clearly would not work if the makefile in a subdirectory is not named Makefile, or if makefiles at different depths of the hierarchy have different filenames.

In the case where the top makefile is named something other than Makefile (eg, as defined in makefile.makefilePath), you'd have to feed that filename to the top make process, but how would you know what the makefiles in the subdirectories are called?

Why is the .DEFAULT_GOAL given explicitly on the make command line?

Regarding target "all". Why is that needed?

all is of course the well-known default target name for automake projects. Generally, the first target in the makefile is the default target, and is exposed in the special variable .DEFAULT_GOAL

The reason it's specified explicitly is itself recursive:

1. In order that --assume-old=Makefile be respected, GNU make requires Makefile to also be given as an explicit target, as explained in the PR, and documented at Remaking Makefiles
2. Since Makefile is an explicit target on the command line, it would be the only target attempted; the .DEFAULT_GOAL would not be considered. Make would not perform the dry-run, and would return without doing anything at all.
3. In order to avoid doing nothing at all, make needs to be given an additional target to evaluate. Ie, the additional target is the target make would have considered if Makefile was not given as a target.

Ideally, the top level incantation would be make --assume-old=${makefile.makefilePath} --dry-run "${makefile.makefilePath}" $(.DEFAULT_GOAL), but sadly there's no obvious way to implement the $(.DEFAULT_GOAL) substitution; this variable is known to make once the makefile is read in, but not known prior to invocation.

Instead of hardcoding the word all, you could extract this value after the project is configured (see How to discover default target in make?)

In short, without the explicit all, dry-running would not be done at all. Make would return without doing anything.

Is there a better way?

My solution would make the makefile tools work much better by avoiding the infinite loop in the majority of automake projects, but it is brittle for a small minority, due to hardcoding the default target name and makefile filename, and due to relying on a fortuitous automake behaviour which is not present for manually written makefiles.

I think a far better solution would be to fork GNU make into a special-purpose tool for dry-running complex projects to be shipped with the makefile-tools extension, and used only for dry-running. The behaviour of the dry-running-make would differ from GNU make in the following ways:

1. Avoid Remaking Makefiles if an additional flag --inspect is given with --dry-run
2. Fail (return non-zero) in --dry-run --inspect mode if a makefile is genuinely out-of-date, ignoring the effect of --always-make
3. Pass --inspect to sub-makes via the MAKEFLAGS mechanism

Since the alternate Remaking Makefiles behaviour is enabled by a new flag, it would not break backward compatibility, and could be contributed back to GNU as a worthwhile feature.

This alternate solution would be more reliable, and take much less effort to implement correctly. My solution based on AM_MAKEFLAGS will need revisiting due to the aforementioned corner cases.

EDIT: Replaced .DEFAULT_TARGET with .DEFAULT_GOAL, the correct name of this special variable.

[andreeis]

@drok , thank you for the additional info, very useful and we will consider it all. Lucky that indeed the most common cases can be addressed with the simpler approach. I will open a work item for the cases that we will not be able to address, including an eventual fork of make or even a contribution into the main one, if it will be desired.

Brainstorm: I am not familiar yet with automake to write myself or play much with it other than just run the usual commands on an existing framework. Can you see a solution where the Makefile Tools extension would use the original makefile of the project as source-input, we call automake ourselves to generate a new makefile which will be identical in content but at least can inherit the variables we need? Then the dry-run will be invoked on that newly generated makefile. I'll be thinking and maybe you can also add on top of this idea or let me know if you see this wouldn't help at all.
Or... We could also with some easy preprocess string manipulation change all the recursive MAKE invocations from the source-input makefiles so that all the destination-output makefiles automake (the one ran by us) have the right flags inserted at the end of the command... This is just brainstorm for future...

[drok]

I am not familiar yet with automake to write myself or play much

I would recommend Hello World(automake) as a good place to start.

call automake ourselves to generate a new makefile which will be identical in content but at least can inherit the variables we need

Fortunately, this behaviour is built in. automake always adds the AM_MAKEFLAGS variable on every sub-make invocation. In fact I don't think it's possible to invoke automake in such a way that this variable is not inherited.

See the crafted search below, of the automake source code, which shows that in the Makefile templates that automake uses, every time $(MAKE) is called, the AM_MAKEFLAGS variable is prepended to the list of arguments. This variable is propagated by make in the environment of sub-makes, like any other environment variable. make itself does not treat it specially, as it does with the distinctly different MAKEFLAGS variable.

https://github.com/search?q=repo%3Aautotools-mirror%2Fautomake+%24%28MAKE%29+path%3Alib%2Fam%2F&type=code

Maybe I misunderstood the idea. How, specifically, would the alternate makefile you envision, be different than the makefile that automake generates by default?

preprocess string manipulation change all the recursive MAKE invocations from the source-input makefiles

This would be very error prone. Complex projects often include makefiles into makefiles to avoid re-writing common rules or definitions multiple times, in a similar way to how C source code uses #include statements. String manipulation of a C source code file, without rigorous expansion of the inclusions and preprocessor definitions can only end in tears. It's the same with Makefiles.

I do believe that the Makefiles that are normally generated by default have all the features needed to solve this dry-run problem (namely, the AM_MAKEFLAGS environment variable), and there's nothing I can see to be gained from processing the makefiles, or even re-generating them with alternate settings.

[andreeis]

@drok

Maybe I misunderstood the idea. How, specifically, would the alternate makefile you envision, be different than the makefile that automake generates by default?

That idea was for projects that don't use automake. You listed this case as one of the future problems to solve. I thought we could invoke automake ourselves when it's not part of the process, but without changing any file belonging to the user (that is why I proposed we generate a different new makefile - even if identical - the user doesn't need to worry about and we point the extension towards that and invoking which will also have all the flags).

And yes, no string preprocessing to inject flags for sub-make calls.

[drok]

we generate a different new makefile

Generating a makefile from an existing makefile makes no sense to me, so I feel I need to describe what automake is, and what it is not:

What is automake ?

automake is a compiler. It takes as source a high-level description of how a project is put together by defining what the _SOURCES, _PROGRAMS, _LIBRARIES involved are, and any tool options that must be used, like _CFLAGS, _CPPFLAGS, _LDFLAGS, etc. The output of automake is an object file which can be run by any make (GNU make, solaris make, HP-UX make, etc), and it contains instructions for exactly how to invoke other tools like C compilers, Pascal compilers, Linkers, etc, to build the project's targets. The object file that automake outputs is a makefile, and it happens to be text. It would be a mistake to treat this object file as a source by editing it, just as it would be a mistake to open an .EXE object file with a hex editor or some other tool to modify how it interacts with the OS when run. Such editing can be successful in some cases, but cannot easily and reliably automated (such as vscode would have to do).

In fact all makefiles are object files, even those which manually written by a person. They can be run under an interpreter (eg one of the make utilities), but are not meant to be compiled into an object file (eg, a batch file). We are concerned with the general case, where any syntactically correct makefile, and not specific cases where relatively simple makefiles can be approximated by batch files.

The idea of generating a makefile from another in order to force some behaviour detail from the make interpreter is akin to generating an EXE from another EXE in order to force some behaviour from the CPU (eg, to do something specific before calling a subroutine). In the CPU/subroutine call analogy, the process of generating an alternate EXE is onerous because the original EXE may contain inlined, optimized subroutines. The difficulty in inserting the specific action before subroutine calls is the same as the difficulty of detecting inlined subroutines, and inferring their unoptimized counterparts (since some irrelevant arguments were optimized away, but may be relevant to the hook we're trying to insert).

What to solve now, what to solve later?

future problems to solve

I thought it would be a future problem to solve the general case, where any makefile can be converted into the equivalent list of commands, because it seems to require modifying the behaviour of the interpreter (the make utility). The make --dry-run option is not the magic solution it appears to be, because of Remaking Makefiles behaviour, which requires more care to avoid.

Even then, if you consider a project where a Makefile contains instructions to invoke another build system, like CMake or MSBuild for a sub-project, it's not clear how a correct list of commands can be obtained. It's not clear to me that a general solution is possible until there is some standardized way of querying build systems.

A general solution is at the bottom of a rabbit hole. Maybe in the future we'll find it, but for the time being, the AM_MAKEFLAGS solution would work in "most" makefile projects.

Forking GNU make would make a cleaner, more elegant solution, and would address the corner case of non-automake recursive makes, but as you said, that idea is 'for the future'. Even that, better idea would not be a general solution, but it might be good enough for vscode's purposes.

[drok]

@andreeis, I have (force-pushed) updated this fork with a complete implementation of the infinite loop fix. Compared with my original implementation (562ea62), the following improvements were made:

• Instead of hardcoding the filename of the makefile, the config variable ${makefilePath} is used.
• ${makefilePath} defaults to "Makefile" instead of empty string.
• instead of hardcoding the target name to all, the config variable ${currentTarget} is used.
• ${currentTarget} defaults to "all" instead of empty string.
• the target name is appended rather than prepended to the dry-run command line for consistency with the make manpage. Ie, make --dry-run --always-make ... ${currentTarget} instead of make all --dry-run --always-make...

The current PR is also tested on a project that uses recursive make, and contains one nested auto-tools sub-project.

Regarding the original reasons for marking it WIP:

• item 1 is resolved (ie, all is not hardcoded)
• item 2 remains a wish-list item for later. (propagating --dry-run in recursive but non-automake projects)

Thus, I am now removing the draft marker from this PR. It is ready.

Future work: I plan to fork GNU make and include it as the dry-running tool, as I describe above - Is there a better way?, eventually, and with no particular rush. I currently am not affected by the non-automake recursive dry-running corner case in any of my projects.

@microsoft-github-policy-service agree

[andreeis]

Thank you for the last long explanation. Very useful. I see now :).
Reviewing the last changes of the PR now and will look at the failed tests briefly as well.

[andreeis]

Not sure. Before this change, if the user does not specify a build target, we invoke "make" (eventually with other args, coming via other switches) but no target mentioned which will result in "make" looking only at the first (if I understand correctly). Now for such scenario suddenly "make" would build "all" even if the user did not specify "all".

[drok]

The issue is that in order to avoid the infinite loop, you must add Makefile as a target, no matter what. Giving no other target is not an option, because make would not attempt to build anything, there would be no compilers invoked, and thus no useful information to be parsed (defines and includes).

The target "all" is assumed all over the code, so that's why I thought it's already baked in. If you want to do it more correctly, you'd have to call make twice. The first time with --print-data-base --dry-run --assume-old=Makefile Makefile, from which you'd extract the .DEFAULT_GOAL variable, and the second time with --dry-run --assume-old=Makefile Makefile ${.DEFAULT_GOAL} - then no assumptions are made about the default target, and you get useful tool invocations from the 2nd --dry-run.

In fact, the output of the first run would be useful to extract the targets in the current directory. (so you'd get the targets first, then the tool invocations)

[andreeis]

I see the common parts with the other PRs. I mentioned some questions for the common code in the other PRs. Did not test yet but I'll get back to this soon. It's possible you'll have a chance to reply and look at test failures by then.

[drok]

It's possible you'll have a chance to reply and look at test failures

the existing tests fail to be meaningful, and reviewing failures is not a good use of time; likewise, putting any faith in "passing" tests is unwise.

I have taken this as the drok#1 issue with this project. The lack of meaningful testing discourages contribution from the community.