Makefile: Allow external Makefile.build and Provide GO 1.18 support #1005
Conversation
unikraft-bot
added
area/lib
razvand
requested review from
StefanJum
and removed request for
a team and
dragosargint
| @@ -2,7 +2,6 @@ menuconfig LIBPOSIX_MMAP | |||
| bool "posix-mmap: Memory mapping related functions" | |||
| default n | |||
| depends on LIBUKVMEM | |||
| depends on !LIBUKMMAP | |||
There was a problem hiding this comment.
Why change this to be the other way around? (have ukmmap depend on !LIBPOSIX_MMAP instead of posix-mmap depend on !LIBUKMMAP)?
There was a problem hiding this comment.
@StefanJum, the reason is that most of the external libraries don't have mentions of LIBPOSIX_MMAP, but rather LIBUKMMAP.
For example, without this commit, if you first select compiler-rt in the menuconfig, ukmmap gets selected because the library's Config.uk file includes the statement imply UKMMAP. If you afterwards select libgo, which has the select LIBPOSIX_MMAP requirement, it would indeed select posix-mmap, but for some reason it also keeps ukmmap selected as well, which leads to a double definition conflict.
The dependency switch that I've introduced prevents that from happening, but I do not have a perfect explanation for this behavior, unfortunately.
There was a problem hiding this comment.
Hum... one additional thing to do in order to avoid the problem is by patching compiler_rt with
imply LIB_UKMMAP if !LIBPOSIX_MMAP.
Additionally, we probably should add a warning or error with support/scripts/configupdate as soon as ukmmap and posix_mmap are both. I think this would be a reasonable workaround as long as ukmmap exists. Longer term the plan was to migrate the left functionality from ukmmap to posix-mmap anyways.
Overall, I will approve this PR and will open an issue for updating the configupdate script.
The current method of building libraries is to iterate over each library's source files and invoke a file type specific build rule. For libgo this is insufficient as we have to do more complex operations for dependency resolution before starting the actual build. This commit allows libraries to supply an own `Makefile.build` that enables custom build mechanics. Co-authored-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io> Signed-off-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io> Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
The current Makefile refers to gccgo via gccgo-7. This commit removes the version suffix. Co-authored-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io> Signed-off-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io> Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
This ensures that both libraries cannot be selected at the same time. Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
eduardvintila
force-pushed
the
go-support-updated
branch
from
6f14861
to
379a9f3
Compare
There was a problem hiding this comment.
All good, thanks.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com
There was a problem hiding this comment.
All good from my side. Tested with gcc12 on x86_64 and it works.
Reviewed-by: Radu Nichita radunichita99@gmail.com
|
✅ Checkpatch passed Beep boop! I ran Unikraft's
|
The current Makefile refers to gccgo via gccgo-7. This commit removes the version suffix. Co-authored-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io> Signed-off-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io> Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Radu Nichita <radunichita99@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #1005
This ensures that both libraries cannot be selected at the same time. Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Radu Nichita <radunichita99@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #1005
This commit ensures that the inclusion of sub-'Makefile.build' is done with the main Makefile. This is done for consistency reasons. This basically adopts commit db20b80 ("Makefile: Allow external Makefile.build") from GitHub PR unikraft#1005. Signed-off-by: Simon Kuenzer <simon@unikraft.io>
This commit ensures that the inclusion of sub-'Makefile.build' is done with the main Makefile. This is done for consistency reasons. This basically adopts commit db20b80 ("Makefile: Allow external Makefile.build") from GitHub PR #1005. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #1028
Prerequisite checklist
checkpatch.ukon your commit series before opening this PR;Base target
x86_64or N/A]kvm,xenor N/A]app-python3or N/A]Additional configuration
Description of changes
This PR is part of a series of PRs that work in conjunction for the update of Go support to 1.18:
For testing, make sure to pass to QEMU the
-cpu hostargument (this is necessary because the paging API and ukvmem are required bylibgoas part of Virtual Address space management, and somehow the default CPU just won't handle 1GB pages).Only GCC12 and x86 is supported for the moment.