build: Add tree builds #1001

andreittr · 2023-07-24T21:53:17Z

Description of changes

This PR adds tree builds -- builds that plate the products in a directory tree mirroring the source files.
The root of this source tree relative to which source files are considered is taken from the $(LIBNAME)_SRC make variable, if present.
This feature is most useful in libraries with many files sharing the same basename (e.g. sources named after class names).

It consists of 3 commits:

Add a separate build/ directory to place the build tree in (so source directories won't conflict with unikraft files); this feature is active if $(LIBNAME)_BUILDTREE is set to y.
Adapt path processing functions to retain source hierarchy when $(LIBNAME)_BUILDTREE is set to y.
Add make functions addlib_tree and addlib_tree_s that automate setting $(LIBNAME)_BUILDTREE.

Prerequisite checklist

Read the contribution guidelines regarding submitting new changes to the project;
Tested your changes against relevant architectures and platforms;
Ran the checkpatch.uk on your commit series before opening this PR;
Updated relevant documentation.

Base target

Architecture(s): N/A
Platform(s): N/A
Application(s): N/A

Additional configuration

Use addlib_tree[_s] to register a treebuild library.

razvand · 2023-07-25T21:47:08Z

@andreittr, this fails when I try to build app-helloworld. Does it work on your side? I'm cloning the app-helloworld repository, the unikraft repository, and I'm applying this PR. And I get this error:

$ make
make[1]: Entering directory '/media/razvan/c4f6765a-efa5-4ebd-9cf0-7da9908a0189/razvan/unikraft/scripts/workdir/unikraft'
mkdir: cannot create directory ‘’: No such file or directory
/media/razvan/c4f6765a-efa5-4ebd-9cf0-7da9908a0189/razvan/unikraft/scripts/workdir/unikraft/lib/ukstore/Makefile.uk:3: *** could not create directory "".  Stop.
Makefile:1033: recipe for target 'sub-make' failed
make[2]: *** [sub-make] Error 2
Makefile:32: recipe for target '_all' failed
make[1]: *** [_all] Error 2
make[1]: Leaving directory '/media/razvan/c4f6765a-efa5-4ebd-9cf0-7da9908a0189/razvan/unikraft/scripts/workdir/unikraft'
Makefile:6: recipe for target 'all' failed
make: *** [all] Error 2

StefanJum

Hi @andreittr. Can you give an idea on how the build directory should look like and on what application is this relevant to test?

StefanJum · 2023-07-26T07:54:09Z

@andreittr, this fails when I try to build app-helloworld. Does it work on your side? I'm cloning the app-helloworld repository, the unikraft repository, and I'm applying this PR. And I get this error:

@razvand are you adding anything else in the config menu or something? I tried it on my system and on the remote systems and it seems to work fine.

razvand · 2023-07-27T22:18:05Z

@razvand are you adding anything else in the config menu or something? I tried it on my system and on the remote systems and it seems to work fine.

I get this problem on a Ubuntu 18.04 setup. You could use the Docker-based configuration here.

StefanJum · 2023-07-28T08:42:03Z

I get this problem on a Ubuntu 18.04 setup. You could use the Docker-based configuration here.

Yes, I get the same error in the container. It fails here, tries to create the directory here and does it (build/libukstore/ exists), but the if fails for some reason

andreittr · 2023-08-02T11:08:14Z

Hi @andreittr. Can you give an idea on how the build directory should look like and on what application is this relevant to test?

Hi! Yes, this library PR right here: unikraft/lib-geos#1 makes use of tree building. It uses addlib_tree_s & defines the LIBGEOS_SRC path.

mariasfiraiala

Thanks @andreittr, this works fine!

Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com

StefanJum

Thank you @andreittr. This seems to work fine, but we still have the issue mentioned above for ubuntu 18.04.
Also please rebase the pr on top of the staging branch so the ci/cd tests can run

StefanJum · 2023-08-09T06:36:03Z

This still fails on ubuntu 18.04:

mkdir: cannot create directory '': No such file or directory
/workdir/unikraft/lib/ukstore/Makefile.uk:3: *** could not create directory "".  Stop.
Makefile:1055: recipe for target 'sub-make' failed

It's all good besides that. @razvand do we merge it as it is or do we search for a fix?

razvand · 2023-08-09T06:39:17Z

It's all good besides that. @razvand do we merge it as it is or do we search for a fix?

No, we have to fix that. @andreittr, any idea what's causing this? Could you check on a container running Ubuntu 18.04?

support/build/Makefile.rules

andreittr · 2023-08-09T19:41:44Z

No, we have to fix that. @andreittr, any idea what's causing this? Could you check on a container running Ubuntu 18.04?

@razvand found the issue while rebasing against staging; it was a typo that was excercised by new code.

Pushed rebase w/ fix, should work ok now.

razvand · 2023-08-09T20:29:35Z

@StefanJum, @mschlumpp, please take another look.

mschlumpp · 2023-08-10T08:22:14Z

@StefanJum I think it was just GitHub actions being broken for some reason. I triggered a retry of the failed jobs and they are now successful.

andreittr · 2023-08-10T10:50:38Z

Also build: Add option to place build objects in a tree seems to break the formatting of the build output by inserting a space in front of all filenames:

Hmm, that seems to be due to use of the $(eval ...) line in targrelpath; fixed it by removing the preceding space when invoking verbose_cmd.

mschlumpp · 2023-08-10T10:52:53Z

It's still a problem for commands without a library specifier:

  CC      libkvmplat: irq.o
  CC      libkvmplat: lcpu.common.o
  LD       libkvmplat.ld.o
  OBJCOPY  libkvmplat.o
  CC      libkvmpci: pci_bus.common.o

andreittr · 2023-08-10T11:03:42Z

It's still a problem for commands without a library specifier:

Should be fixed now by judicious application of $(strip ...).

mschlumpp

Reviewed-by: Marco Schlumpp marco@unikraft.io

StefanJum

All good, thanks.

Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

support/build/Makefile.rules

andreittr · 2023-08-11T09:22:37Z

Rebased & addressed comments.

support/build/Makefile.rules

This change adds the option to place build files in a dedicated `.../build/` directory under the library build path. This feature is opt-in per-library and enabled if the $(LIBNAME)__BUILDTREE make variable is set. Signed-off-by: Andrei Tatar <andrei@unikraft.io>

This adds the feature to place build products in a directory tree mirroring the tree their source files are in. This feature piggy-backs on top of the $(LIBNAME)__BUILDTREE flag from 87065c2f (build: Add option for dedicated build directory). In addition, the $(LIBNAME)_SRC make variable must point to the common prefix of a library's source file paths. Any intermediate directories are created automatically. Signed-off-by: Andrei Tatar <andrei@unikraft.io>

This change adds `addlib_tree` and `addlib_tree_s` as library registration functions that enable build products to be placed in a directory tree mirroring the source files. This is useful for code that shares source filenames across multiple directories. Signed-off-by: Andrei Tatar <andrei@unikraft.io>

skuenzer

Thanks a lot for the work!

Reviewed-by: Simon Kuenzer simon@unikraft.io
Approved-by: Simon Kuenzer simon@unikraft.io

unikraft-bot · 2023-08-11T16:05:26Z

✅ Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA	commit	checkpatch
`1ab16fb`	build: Add option for dedicated build directory	✅
`0440c82`	build: Add option to place build objects in a tree	✅
`aa4bead`	build: Add library registration for tree build	✅

This adds the feature to place build products in a directory tree mirroring the tree their source files are in. This feature piggy-backs on top of the $(LIBNAME)__BUILDTREE flag from 87065c2f (build: Add option for dedicated build directory). In addition, the $(LIBNAME)_SRC make variable must point to the common prefix of a library's source file paths. Any intermediate directories are created automatically. Signed-off-by: Andrei Tatar <andrei@unikraft.io> Reviewed-by: Maria Sfiraiala <maria.sfiraiala@gmail.com> Reviewed-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #1001

This change adds `addlib_tree` and `addlib_tree_s` as library registration functions that enable build products to be placed in a directory tree mirroring the source files. This is useful for code that shares source filenames across multiple directories. Signed-off-by: Andrei Tatar <andrei@unikraft.io> Reviewed-by: Maria Sfiraiala <maria.sfiraiala@gmail.com> Reviewed-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #1001

andreittr requested a review from a team as a code owner July 24, 2023 21:53

razvand requested review from StefanJum, mariasfiraiala and mschlumpp and removed request for a team July 24, 2023 21:58

razvand assigned skuenzer Jul 24, 2023

razvand added kind/enhancement New feature or request area/makefile Part of the Unikraft Makefile build system topic/build Topics to do with the build system priority/medium labels Jul 24, 2023

razvand added this to the v0.14.0 (Prometheus) milestone Jul 24, 2023

andreittr mentioned this pull request Jul 25, 2023

Port upstream libgeos 3.11.2 unikraft/lib-geos#1

Closed

StefanJum reviewed Jul 26, 2023

View reviewed changes

andreittr requested a review from StefanJum August 2, 2023 15:10

mariasfiraiala approved these changes Aug 4, 2023

View reviewed changes

StefanJum requested changes Aug 4, 2023

View reviewed changes

andreittr force-pushed the ttr/treebuild branch from 0ee5c37 to f1d5352 Compare August 4, 2023 20:37

razvand requested a review from StefanJum August 9, 2023 06:22

mschlumpp reviewed Aug 9, 2023

View reviewed changes

support/build/Makefile.rules Outdated Show resolved Hide resolved

andreittr force-pushed the ttr/treebuild branch from f1d5352 to 22ffd6d Compare August 9, 2023 19:41

razvand requested a review from mschlumpp August 9, 2023 20:29

andreittr force-pushed the ttr/treebuild branch from 22ffd6d to 74b0a4f Compare August 10, 2023 10:45

andreittr force-pushed the ttr/treebuild branch from 74b0a4f to fa61ec4 Compare August 10, 2023 11:02

andreittr requested review from mschlumpp and skuenzer August 10, 2023 13:16

mschlumpp approved these changes Aug 10, 2023

View reviewed changes

StefanJum approved these changes Aug 10, 2023

View reviewed changes

skuenzer reviewed Aug 10, 2023

View reviewed changes

support/build/Makefile.rules Outdated Show resolved Hide resolved

support/build/Makefile.rules Outdated Show resolved Hide resolved

andreittr force-pushed the ttr/treebuild branch 2 times, most recently from ed7dc05 to 6cc383d Compare August 11, 2023 09:22

razvand requested a review from skuenzer August 11, 2023 09:31

skuenzer reviewed Aug 11, 2023

View reviewed changes

support/build/Makefile.rules Outdated Show resolved Hide resolved

andreittr force-pushed the ttr/treebuild branch 2 times, most recently from ede06d3 to 52f90dd Compare August 11, 2023 10:29

andreittr added 3 commits August 11, 2023 16:51

andreittr force-pushed the ttr/treebuild branch from 52f90dd to aa4bead Compare August 11, 2023 14:53

skuenzer approved these changes Aug 11, 2023

View reviewed changes

unikraft-bot closed this in 5bdc8d9 Aug 11, 2023

unikraft-bot added the ci/merged Merged by CI label Aug 11, 2023

andreittr deleted the ttr/treebuild branch August 11, 2023 16:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Add tree builds #1001

build: Add tree builds #1001

andreittr commented Jul 24, 2023

razvand commented Jul 25, 2023

StefanJum left a comment

StefanJum commented Jul 26, 2023

razvand commented Jul 27, 2023

StefanJum commented Jul 28, 2023

andreittr commented Aug 2, 2023

mariasfiraiala left a comment

StefanJum left a comment

StefanJum commented Aug 9, 2023

razvand commented Aug 9, 2023

andreittr commented Aug 9, 2023

razvand commented Aug 9, 2023

mschlumpp commented Aug 10, 2023

andreittr commented Aug 10, 2023

mschlumpp commented Aug 10, 2023

andreittr commented Aug 10, 2023

mschlumpp left a comment

StefanJum left a comment

andreittr commented Aug 11, 2023

skuenzer left a comment

unikraft-bot commented Aug 11, 2023

build: Add tree builds #1001

build: Add tree builds #1001

Conversation

andreittr commented Jul 24, 2023

Description of changes

Prerequisite checklist

Base target

Additional configuration

razvand commented Jul 25, 2023

StefanJum left a comment

Choose a reason for hiding this comment

StefanJum commented Jul 26, 2023

razvand commented Jul 27, 2023

StefanJum commented Jul 28, 2023

andreittr commented Aug 2, 2023

mariasfiraiala left a comment

Choose a reason for hiding this comment

StefanJum left a comment

Choose a reason for hiding this comment

StefanJum commented Aug 9, 2023

razvand commented Aug 9, 2023

andreittr commented Aug 9, 2023

razvand commented Aug 9, 2023

mschlumpp commented Aug 10, 2023

andreittr commented Aug 10, 2023

mschlumpp commented Aug 10, 2023

andreittr commented Aug 10, 2023

mschlumpp left a comment

Choose a reason for hiding this comment

StefanJum left a comment

Choose a reason for hiding this comment

andreittr commented Aug 11, 2023

skuenzer left a comment

Choose a reason for hiding this comment

unikraft-bot commented Aug 11, 2023