TUP_VARIANTDIR with output outside cwd

[Christopher Corsi]

Hi,

I am trying to convert my pre-explicit variant project Tupfiles and running into a problem.

Currently there are two basic stages in the build: Generate libraries, then generate binaries that link against those libraries. The libraries are stored in a directory different from their source directory for two reasons:

 - So all libraries can be referenced with -lmylib by just adding the library output directory to the link path with -L$(LIB_DIR)

 - So the final build's directory structure matches the installation directory structure (which is essentially a sysroot)

The structure looks like this:

src/

    Tuprules.tup

    libA/

        library.cc

        Tupfile

    libs/

        # empty

    targets/

        target.cc

        Tupfile

Tuprules is something like this:

DIR_LIBS = $(TUP_CWD)/libs

NEEDS_LIBA = $(TUP_CWD)/<needs_liba>

The first Tupfile builds a library like this:

include_rules

: library.cc |> g++ %f -o %o |> %B.o {objs}

: {objs} |> ar rs %o %f |> $(DIRLIBS)/libA.a

The second Tupfile tries to build and link the target like this:

include_rules

: target.cc |> g++ %f -L$(DIR_LIBS) -lA -o %o |> %B

This obviously breaks with the new explicit variants as-is, but I can't figure out how to make it work. Changing the DIR_LIBS definition to $(TUP_VARIANTDIR)/libs doesn't work, since $(TUP_VARIANTDIR) seems to be expanding based on the path it's included from (i.e. it expands to ../build/targets/ inside the targets folder). So far the only option I've found is to explicitly mess with the path by changing the target build line to something like this:

: target.cc |> g++ %f -L$(TUP_VARIANTDIR)/../libs -lA -o %o |> %B

Which isn't very scalable or robust with refactoring. Is there a better solution?

Could TUP_VARIANTDIR also be added to the official documentation? All of the above was discovered by manual experimentation and a git bisect on tup.

Thanks!

[Mike Shal]

Hi Christopher,

On Wed, Sep 15, 2021 at 2:58 AM Christopher Corsi <cjm...@gmail.com> wrote:

DIR_LIBS = $(TUP_CWD)/libs

NEEDS_LIBA = $(TUP_CWD)/<needs_liba>

Unrelated to your issue, but it looks like you might be creating a separate group for each library (presumably since you have a needs_liba group, you would also have needs_libb, needs_libc groups?). You should just be able to use a single group, like $(TUP_CWD)/<libs> that corresponds to the "generate libraries" stage, so all libraries can also list this <libs> group as an output, and all binaries can list the same group as an input.

 

The first Tupfile builds a library like this:

include_rules

: library.cc |> g++ %f -o %o |> %B.o {objs}

: {objs} |> ar rs %o %f |> $(DIRLIBS)/libA.a

The second Tupfile tries to build and link the target like this:

include_rules

: target.cc |> g++ %f -L$(DIR_LIBS) -lA -o %o |> %B

This obviously breaks with the new explicit variants as-is, but I can't figure out how to make it work. Changing the DIR_LIBS definition to $(TUP_VARIANTDIR)/libs doesn't work, since $(TUP_VARIANTDIR) seems to be expanding based on the path it's included from (i.e. it expands to ../build/targets/ inside the targets folder). So far the only option I've found is to explicitly mess with the path by changing the target build line to something like this:

Thanks for reporting this, this is definitely an issue. The problem is that tup is expecting all paths to point to the srcdir, and then if a variant is enabled, it will convert the output paths to point to the variant dir. So when you have an output path that already points inside the variantdir, tup doesn't realize it and makes it try to point inside the variantdir again and it ends up creating the library in build/build/libs/ instead of build/libs/. I'm working on a fix but it's not a quick one unfortunately.

 

: target.cc |> g++ %f -L$(TUP_VARIANTDIR)/../libs -lA -o %o |> %B

Which isn't very scalable or robust with refactoring. Is there a better solution?

One way it can work now until I get the above issue fixed is to have two separate variables, so that the outputs use $(TUP_CWD) based paths and things like -L use $(TUP_VARIANTDIR) based paths. Something like this patch applied to your example:

diff --git a/Tuprules.tup b/Tuprules.tup
index 04f2ccf..3bad3d7 100644
--- a/Tuprules.tup
+++ b/Tuprules.tup
@@ -1,2 +1,3 @@
 DIR_LIBS = $(TUP_CWD)/libs
+VARIANT_LIBS = $(TUP_VARIANTDIR)/libs
 NEEDS_LIBA = $(TUP_CWD)/<needs_liba>
diff --git a/targets/Tupfile b/targets/Tupfile
index 8db422e..4ec1623 100644
--- a/targets/Tupfile
+++ b/targets/Tupfile
@@ -1,2 +1,2 @@
 include_rules
-: target.c | $(DIR_LIBS)/<libs> |> g++ %f -L$(DIR_LIBS) -lA -o %o |> %B
+: target.c | $(DIR_LIBS)/<libs> |> g++ %f -L$(VARIANT_LIBS) -lA -o %o |> %B

 

Could TUP_VARIANTDIR also be added to the official documentation? All of the above was discovered by manual experimentation and a git bisect on tup.

Ugh, sorry about that. I really appreciate you taking the time to work through this and reporting your findings.

-Mike

[Mike Shal]

On Wed, Sep 15, 2021 at 2:58 AM Christopher Corsi <cjm...@gmail.com> wrote:

This obviously breaks with the new explicit variants as-is, but I can't figure out how to make it work. Changing the DIR_LIBS definition to $(TUP_VARIANTDIR)/libs doesn't work, since $(TUP_VARIANTDIR) seems to be expanding based on the path it's included from (i.e. it expands to ../build/targets/ inside the targets folder). So far the only option I've found is to explicitly mess with the path by changing the target build line to something like this:

Can you try out this branch and see if it helps? https://github.com/gittup/tup/tree/variantdir-output

That should work with your example when setting DIR_LIBS to $(TUP_VARIANTDIR)/libs, but you might hit other issues after that.

I tried a couple of different fixes, but the only one I got to work is to see if a filename given as an input/output enters into the variant directory, and if so, to skip auto-adding the variant path. One potential caveat with this is if you have a Tupfile like this:

: |> touch %o |> foo.txt
: |> touch %o |> build/bar.txt

If you run this without a variant, you get foo.txt and build/bar.txt as files. If you run it with a variant called 'objdir', you get objdir/foo.txt and objdir/build/bar.txt. But if you run it with a variant called 'build', then you get objdir/foo.txt and objdir/bar.txt since it thinks the second rule is trying to point into the variant rather than creating a subdir. I haven't found a way to distinguish between when you're intentionally trying to put a file into a variant (like your example when using $(LIBS_DIR)), and when the output dir happens to have the same name as the variant dir. So I'm not sure how to address this other than to say "don't name a variant the same as a top-level generated directory" in the manual somewhere. Your thoughts? Or can you think of a better approach?

 

Could TUP_VARIANTDIR also be added to the official documentation? All of the above was discovered by manual experimentation and a git bisect on tup.

I added this to the manual; let me know if it is insufficient.

Thanks again,

-Mike

[Christopher Corsi]

Sorry for the delayed response - replies were being sent to spam and I barely caught your most recent reply before deleting.

> You should just be able to use a single group, like $(TUP_CWD)/<libs> that corresponds to the "generate libraries" stage, so all libraries can also list this <libs> group as an output, and all binaries can list the same group as an input.

Yep, that was how it was done originally. The actual project is substantially more complex (15 minute build times) than the example given here and we wanted the ability to build small portions of the tree to save time. The actual order dependencies are being generated with lua scripting by iterating over the linker parameters and looking for "-l".

> One way it can work now until I get the above issue fixed is to have two separate variables, so that the outputs use $(TUP_CWD) based paths and things like -L use $(TUP_VARIANTDIR) based paths. Something like this patch applied to your example:

I tested the patch on the example and it works here as well. I haven't tried expanding it to the full project yet.

> If you run this without a variant, you get foo.txt and build/bar.txt as files. If you run it with a variant called 'objdir', you get objdir/foo.txt and objdir/build/bar.txt. But if you run it with a variant called 'build', then you get objdir/foo.txt and objdir/bar.txt since it thinks the second rule is trying to point into the variant rather than creating a subdir. I haven't found a way to distinguish between when you're intentionally trying to put a file into a variant (like your example when using $(LIBS_DIR)), and when the output dir happens to have the same name as the variant dir. So I'm not sure how to address this other than to say "don't name a variant the same as a top-level generated directory" in the manual somewhere. Your thoughts? Or can you think of a better approach?

One idea comes to mind but it would be a bit more complex. Right now it seems like output paths have the variant directory prefix added unconditionally. The parser could track expansions that started with $(TUP_VARIANTDIR) (i.e. $(TUP_VARIANTDIR) itself or recursively, some other variable that started with this). If the output path uses one of these values it can avoid adding the prefix again. This would be fragile if there was a use case for including $(TUP_VARIANTDIR) in a path where it wasn't the first path component but I can't think of a reason why someone might do that.

> I added this to the manual; let me know if it is insufficient.

Thanks! I think this is good. I might try to write some additional instructions for the variant section based on my experience. If so, I'll share it here.

Thanks,

Christopher

[Mike Shal]

On Thu, Sep 23, 2021 at 12:42 AM Christopher Corsi <cjm...@gmail.com> wrote:

> If you run this without a variant, you get foo.txt and build/bar.txt as files. If you run it with a variant called 'objdir', you get objdir/foo.txt and objdir/build/bar.txt. But if you run it with a variant called 'build', then you get objdir/foo.txt and objdir/bar.txt since it thinks the second rule is trying to point into the variant rather than creating a subdir. I haven't found a way to distinguish between when you're intentionally trying to put a file into a variant (like your example when using $(LIBS_DIR)), and when the output dir happens to have the same name as the variant dir. So I'm not sure how to address this other than to say "don't name a variant the same as a top-level generated directory" in the manual somewhere. Your thoughts? Or can you think of a better approach?

One idea comes to mind but it would be a bit more complex. Right now it seems like output paths have the variant directory prefix added unconditionally. The parser could track expansions that started with $(TUP_VARIANTDIR) (i.e. $(TUP_VARIANTDIR) itself or recursively, some other variable that started with this). If the output path uses one of these values it can avoid adding the prefix again. This would be fragile if there was a use case for including $(TUP_VARIANTDIR) in a path where it wasn't the first path component but I can't think of a reason why someone might do that.

Oh, that's a good idea. Initially I thought it wouldn't work since you could have TUP_VARIANTDIR-based paths and regular paths in the same variable, so tracking which variables are TUP_VARIANTDIR-based would be tricky. Eg:

libs = $(TUP_VARIANTDIR)/lib1.a build/lib2.a

Presumably we would want lib1.a to be treated as if it's already in a variant, but build/lib2.a to still get put in a variant directory (even if it's also named "build"). But thinking about it some more, I thought it could work if TUP_VARIANTDIR basically expanded to a node variable (not commonly used, but these are like database references during the parsing stage). I updated how node variables work, and now $(TUP_VARIANTDIR) isn't expanded into its full string until later, which means if it's used in an output section, we can know that it is already intending to point into the variant directory.

Would you be able to do another test with https://github.com/gittup/tup/tree/variantdir-nodevar and see if you find any issues? I think it resolves the issue pretty nicely, and doesn't rely on string-matching the variant directory or cause problems if a generated directory has the same name as the variant directory. If that solves your use-case I'll merge it in.

Thanks for your help,

-Mike

[Mike Shal]

Well I ended up merging this. If you run into any issues with it, let me know!

Thanks,

-Mike