pkg: store packages sources in a global package directory#14289
pkg: store packages sources in a global package directory#14289kaspar030 merged 9 commits intoRIOT-OS:masterfrom
Conversation
|
Isn't this leading to problem, when different applications have different compile time configurations? |
If the original source code is unchanged, this is not a problem since all intermediate build files should go in the build directory of each application/board. Think of it as |
|
Ah, I did not realize you put the two apart. This however then has the implication, that if the |
|
For cmake, it's already difficult IMHO, this change doesn't make thing harder, just cleaner: out-of-source builds are forced (see #14286) |
"while you're at it", how about changing to |
git-cache is still used instead of a direct clone from the network, right? |
The logic is the same, so if gitcache is initialized, it will be used as usual. But for the developer, using gitcache becomes less useful: you would have the repo duplicated (in git-cache and in the global package directory). |
It should be ok(tm). Murdock never builds in parallel in a single checkout. Previously, it would remove BINDIR after each build, so the checkout doesn't grow too much. What needs to be clear is that with shared (re-used) source directories, the logic must be rock solid even on failure / aborts. Before, each build would get their own fresh checkout and patch. Now, following builds on the same worker might fail, if something in the download step goes wrong in a previous build. This can all be solved by making it possible to override the base directory for PKG_BUILD_DIR so Murdock can move it into BINDIR.
Checking out from git-cache is fast. There's not much to gain, why download at the beginning? |
859bf55 to
77860f8
Compare
Is it really an issue ? During the package download phase, several steps are performed:
Then Murdock won't behave the same as a developer would do locally, hiding potential problems during CI. By the way, for most of the packages this can already be done by overriding PKGDIRBASE but I won't recommend to use this for Murdock. |
it should be almost the same, if both are still using out-of-tree builds. just that the source download folder is not shared between builds, but local to bindir. |
maybe not. it definitely is not an issue until it is. if it shows up, it shows up in the wrong build. |
So setting "PKGDIRBASE=$(BINDIRBASE)/pkg" variable to the build should work for packages using out-of-source builds: source code will be cloned in "$(BINDIRBASE)/pkg/$(PKG_NAME)" and files generated by the build (for cmake mainly) will go in "$(BINDIR)/$(PKG_NAME)". So for example with
This I don't get. If the package couldn't be fetched with git (which is done per worker only when building the package the first time it's added to RIOT or the version is changed), all builds related to this package would fail but this happen "rarely" and only during the initial development phase. If the package cannot be patched because the patches cannot be applied, all builds related to this package will fail anyway. |
yes! |
yes, you're right, that would show up only in builds for that package. I got confused with the increased build folder sizes. Those would show up in random builds. |
|
Let's test this one on Murdock |
It doesn't seem to cause major problems on Murdock. There were some tools that were initially missed: kconfig, cc2538-bsl-py, setsid, pic32prog, teensyloader-cli. I pushed a commit to fix them. cc2538-bsl.pysetsiddiff --git a/Makefile.include b/Makefile.include
index 2c9bc19ab7..0fcf2dba66 100644
--- a/Makefile.include
+++ b/Makefile.include
@@ -648,7 +648,7 @@ endef
# Check if setsid command is available on the system for debug target
# This is not the case on MacOSX, so it must be built on the fly
ifneq (,$(filter debug, $(MAKECMDGOALS)))
- ifneq (0,$(shell which setsid 2>&1 > /dev/null ; echo $$?))
+ ifneq (0,$(shell which invalid 2>&1 > /dev/null ; echo $$?))
SETSID = $(RIOTTOOLS)/setsid/setsid
$(call target-export-variables,debug,$(SETSID))
DEBUGDEPS += $(SETSID)pic32progteensyloader-cli |
|
@kaspar030, I think we are good here. May I squash ? |
yes, please squash! |
openthread, micropython and nordic_softdevice cannot be built out-of-source
ea3a0cd to
42912b4
Compare
|
squashed ! |
|
Any chance of this PR getting merged until noon-ish? |
yes, likely! |
|
All green here @kaspar030 ! |
|
ACK. |
|
Awesome! Thanks a lot for reviewing and merging @kaspar030 ! |
Contribution description
Please don't trigger Murdock before there's consensus about this PR
This PR is a POC that reworks the location where the fetched source code of packages is stored. The main objective is to have a central place for packages sources (in this PR it is
$(RIOTBASE)/packagesbut that can be adapted) and avoid to fetch them all the time when building an application for different boards when the application needs some packages. When developing or testing, this saves a lot of time and a lot of disk space.To achieve this, several things are required:
PKG_BUILDDIRvariable is very ambiguous because most of the time it corresponds to the place where package sources are located. To make things more clear, this PR renames this variable toPKG_SOURCE_DIRwhich contains the same value as the actualPKG_BUILDDIR:$(PKGDIRBASE)/$(PKG_NAME). A newPKG_BUILD_DIRvariable is introduced and corresponds to the place where files generated by the build are stored. This location is the same as the current one:$(BINDIRBASE)/pkg/$(BOARD)/$(PKG_NAME)PKG_SOURCE_DIRis the same asPKG_BUILD_DIR. Note that for openthread, the next release will use CMake so it will be possible to switch to the new organization.This PR is not meant to be a replacement for git-cache. git-cache can still be useful on the CI, to avoid fetching all packages between each Murdock run and also because 2 packages are still cloned in each application/board build directory.
Open questions:
pkg/<package name>/sources?Testing procedure
A successful Murdock run should be enough.
Example outputs with this PR:
tests/pkg_flatbuffers
tests/pkg_libcose
Issues/PRs references
This PR is based on
#14250#14251#14272#14273#14274#14280#14282#14286#14287