Skip to content

Fix out of tree builddir#5

Open
drok wants to merge 4 commits intomainfrom
fix-out-of-tree-builddir
Open

Fix out of tree builddir#5
drok wants to merge 4 commits intomainfrom
fix-out-of-tree-builddir

Conversation

@drok
Copy link

@drok drok commented Oct 18, 2023

out-of-tree builds allow multiple configurations to be built
simultaneously from one source tree. Each build configuration exists in
a separate builddir, and the srcdir contains no configuration, object
files or build artifacts.

In an autotools project which uses automake, the Makefiles are object
files, ie they are generated specifically for a particular
configuration.

When building an autotools project out-of-tree, the workspace is the
$srcdir, and does not contain a Makefile. Instead, the Makefile is in
$makeDirectory which is the $builddir.

This commit cleans up the detection of makefile location that is done in
the extension so that the correct makefile is found, even if it lives
out-of-tree/out-of-workspace, has a name other than Makefile/makefile,
an whether or not a file named Makefile also exists in the $srcdir.

WIP because something doesn't work and I don't know how to debug (HELP
wanted):

  • if a file named "Makefile" also exists in $srcdir, the extension
    functions correctly.
  • if a file named "Makefile" does NOT exist in the $srcdir, but only in
    $builddir (as is typical), the extension is asynchronously deactivated
    after doing all the dry-run work.

The deactivation is not expected, it is a result of a $srcdir/Makefile not
existing, but the debugger does not pause the Extension Development Host
when the deactivate() method is called. Instead, the Extension
Development Host
quickly reloads before I have a chance to examine the
callstack. The callstack is of course cleared when the Host reloads.

There must be a way to diagnose exactly the chain of calls that leads to deactivate() being called, but I don't know how, because the callstack is prematurely cleared.

Repro:

  • Download the Automake Hello World tarball 1.0. This file is included as documentation on CentOS7 (eg /usr/share/doc/automake-1.13.4/amhello-1.0.tar.gz) but missing on some Debian/Ubuntus.
  • Unpack it in some workspace directory (eg, ~/amhello).
  • create a /tmp/amhello-a directory (this will be the build directory)
  • chdir to /tmp/amhello-a
  • create the 'a' configuration here (run ~/amhello/configure in /tmp/amhello-a)
  • open vscode in the ~/amhello workspace, and set makeDirectory to /tmp/amhello-a
  • Create an empty Makefile in srcdir: touch ~/amhello/Makefile
  • The extension will load and dry run in /tmp/amhello-a using the makefile /tmp/amhello-a/Makefile
  • Remove the empty Makefile in srcdir: rm ~/amhello/Makefile
  • restart vscode; The extension will not activate.
  • To prevent auto-deactivation, you can create a Makefile in srcdir (touch ~/amhello)

Expected: The extension will activate/deactivate based on existence of /tmp/amhello-a/Makefile, without regard to ~/amhello/Makefile

If you know how to get vscode to debug the extension with Extension Development Host and have it stop when receiving the deactivate() call, and prevent the Host from reloading, and thus clearing the stack trace, please tell me how.

Note: this PR builds on top of PR #4. Without it, an autotools+automake project would be unable to complete a dry run, even if building in-tree.

Originally proposed upstream as microsoft#517

drok and others added 4 commits October 13, 2023 15:14
'all' is a well-known name

The default target must be explicitly named on the dry-run command line,
because of [Remaking Makefiles](https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html)
rule of GNU Make. In order to avoid remaking makefiles during a dry-run,
the makefile must also be given as an explicit target.

If the makefile were given as the only target, the dry-run would not
mean 'dry-run the default target', but 'dry-run nothing'. Thus, if the
makefile must be given as a target, the default target must be made
explicit, and thus is must be defined.

Also, previously 'all' was added to the make invokation for listing
targets. This would prevent the .DEFAULT_GOAL to be correctly set as the
Makefile declared, and instead be set to the value given on the command
line. An explicit target is no longer given on the --print-data-base
invocation, so the output .DEFAULT_GOAL will be the genuine default goal.

This commit makes currentTarget always defined, with the default value
'all'; this is a reasonable, well-known default.
The top makefile in projects that use autotools and automake contains a
rule to remake the makefile itself when the configuration changes
(configure.ac).

Even when dry-running, GNU make regenerates the makefile, in a bid to
generate a 'correct' dry-run output.

VScode needs to add --always-make in order to get a complete view of the
dry-run. Without it, it would only get the commands needed for outdated
targets.

These two behaviours combined cause a naive 'make --dry-run --always-make'
to continuously remake the Makefile. In order to avoid this infinite loop,
make must be instructed as in the "Remaking Makefiles" man page, to
avoid remaking the makefile. This is done by adding two options:
  --assume-old=Makefile, and
  Makefile (ie, target).

Make requires the Makefile to be explicitly specified as target, otherwise
it ignores the --assume-old=Makefile option.

Furthermore, Makefiles generated by automake cause the top invocation to
be a recursive sub-make invocation. On recursive makes, make itself calls
submake without passing the --assume-old option, thus breaking the combo
required for the sub-make to avoid remaking the makefile. As a result,
automake Makefiles need one more workaround. The --assume-old option must
be manually passed to sub-make via the AM_MAKEFLAGS, which is always
appended to sub-make's command line.

This commit implements the above incantation to enable automake Makefiles
to be dry-run without an infinite loop.

Additionally, the makefilePath, makeDirectory, updatedMakefilePath and
updatedMakeDirectory variables are made to store the respective setting,
rather then re-purposing them to store the corresponding resolved,
absolute paths. makefilePath cannot be undefined because for dry-running
the name of the makefile must always be supplied on the make commandline.
out-of-tree builds allow multiple configurations to be built
simultaneously from one source tree. Each build configuration exists in
a separate builddir, and the srcdir contains no configuration, object
files or build artifacts. The builddir need not be a subdirectory of
srcdir, and typically it is on another filesystem than the sources.

In an autotools project which uses automake, the Makefiles are object
files, ie they are generated specifically for a particular
configuration, therefore they reside in the builddir.

When building an autotools project out-of-tree, the workspace is the
srcdir, and does not contain a Makefile. instead, the Makefile is in
$makeDirectory which is the builddir.

This commit cleans up the detection of makefile location that is done in
the extension so that the correct makefile is found, even if it lives
out-of-tree/out-of-workspace, has a name other than Makefile/makefile,
an whether or not a file named Makefile also exists in the srcdir.

The activationEvent is changed to "*" because when building out of
tree, there are no makefiles in the workspace. "*" is the better
activation because it means activate whenever the user enabled the
extension; don't second guess. OTOH, the extension does nothing
if a makefile is not found and not configured, so it's safe to
have it activated on workspaces without makefiles, because of the
cleaned-up detection/configuration of makefiles.
When building out of tree, whether the extension is in fullFeatureSet or
not depends on which build configuration is chosen.

The workspace does not have a Makefile, but one of the builddirs in a
configuration does. Allow that configuration to be selected,
unconditionally on the currently selected configuration.
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.

1 participant