make: introduce common BUILD_DIR

[smlng]

Contribution description

This introduces a new environment variable for a common directory
that holds all output of the build process, such as application or
package binaries. This would also allow to easily redirect output
to any other location, e.g. for out-of-source builds.

The 2nd commit adds a build directory with subdirs build/bin and build/pkg. In the (near) future we might add something like build/tests to store test results.

The 3rd commit adds the APPLICATION name as a subdirectory to the bin output folder, this is similar to the pkg subdirectory hierarchy and will also allow to build multiple application in one out-of-source build directory (in the future).

Testing procedure

Everything should still build as usual. However, as some directories changed (see above) it might break something

Issues/PRs references

• needed by testing: introduce RobotFramework tests #10095 and testing: add periph I2C robot tests for HIL #10147

[aabadie]

This would also allow to easily redirect output to any other location, e.g. for out-of-source builds.

I like. But let's wait for build system experts' opinion.

[miri64]

This is more a reminder to my self for when the discussion on this is close to an end, so don't see this as instruction to act immediately: build/ needs to be added to / replace with *bin in .gitignore.

[smlng]

ah good point! Please, remember for all of us 😉

[smlng]

I like. But let's wait for build system experts' opinion.

that's the purpose of this PR, in parts I already talked about this with @cladmi - because we need a place to store test output+results, e.g. from HIL and so on, to they can be found and accessed more easily.

[cladmi]

I was more thinking about a $(RIOTBASE)/build directory.
This would allow put things in common between builds, like tools, pythonlibs instead of currently cloning them directly in the source tree.

The fact that the current build directory is called bin is just a weird naming issue but does not cause any problems.

Also changing it is a Build System API change so would require proper announcement.

[miri64]

I was more thinking about a $(RIOTBASE)/build directory.
This would allow put things in common between builds, like tools, pythonlibs instead of currently cloning them directly in the source tree.

I think waaaaaay back bin/ was actually in $(RIOTBASE) for all applications... I think conflicts in configuration lead to the per application approach we have nowadays.

[cladmi]

If we start change the name of the bin directory and touch this part of the BS API, I would also like to change one thing, remove the override to an absolute path of the directories inside that are board dependent. This is breaking things in the current build system with buildtest for example.

I would replace it by a check it is an absolute path if the user really wants to set it himself.

I would rather say, you can over

[cladmi]

I was more thinking about a $(RIOTBASE)/build directory.
This would allow put things in common between builds, like tools, pythonlibs instead of currently cloning them directly in the source tree.

I think waaaaaay back bin/ was actually in $(RIOTBASE) for all applications... I think conflicts in configuration lead to the per application approach we have nowadays.

That is why I want to still keep a per application bin directory (or build directory). But introducing a common build directory could help migrate parts to a common one.

I would like to be able to build all applications without cleaning, but I would like to not have 120 times the 190M of the u8g2 package.

[cladmi]

To explain a bit more my thought.

I see two parts to this PR.

Rename bin to build with a different hierarchy inside but still as a "per application" basis.
This changes the directory API but could be the occasion of doing a cleanup of the override directive of variables that are inside.
And I could go for this with the proper announcements.
I could also go for keeping the "ugly" name and put "bin/results" for the tests even if they are not binaries to keep the current API if it is preferred.

The second part you propose, is preparing this directory to be common between applications, by having the per APPLICATION subdirectory for example. And I would rather not do this but address it the other way around.
Because until 100% can be done in a common directory, the per application build directory cannot be moved to a common one and if at least one maintainers is not 100% conscious and careful that it can become common, new problems could be introduced.
Even now some packages need to be per application when they are based on external build systems. With ccn-lite for example, the cmake command depends on INCLUDES so on the application.

I would prefer introduce acommon build directory, at $(RIOTBASE)/build or any other name with at the beginning nothing inside, and then, migrate one thing at a time to it. And maybe there will always be a need for per application things in the application bin directory. Or things could be migrated but we do not have enough working power to do the whole migration.

I think we would benefit more by separating it as early as possible to be able to put directly common things in it.
We should also define the rules on how to use it, like not allow having multiple build processes using the directory in parallel could prevent to use a lock for every transactions.

[smlng]

I was also thinking to include the build commit hash (git rev-parse --short HEAD) into the hierarchy at some place, to allow for storage of multiple builds for the app but over time. Could be (at least) useful for CI and testing to store many binaries/results over time for comparison - or just to compare master with some PR.

[smlng]

anyway thanks for your comments @cladmi, I guess you're right and I also like the idea to have $(RIOTBASE)/build - my idea was to simply introduce new name and hierarchy without changing the current structure too much.

Also for packages I wasn't aware (sure) that some might might be build in an app depend way, so maybe it makes sense to checkout/clone/download the package source to build/pkg/<pkg_name>/ and then actually compile in build/pkg/<pkg_name>/<app_name> so each packages source is only needed once - is that what you mean by referring to u8g2?

[cladmi]

As explained in the main comments, I think the ones currently in bin should not been moved for the moment.
It is an API change I would not merge in the last weeks of a release.

Adding a new directory that needs to start being handled for new commands is ok.
The internal organization of this directory will also need some thinking.

Also, there is still some work to make this correctly handled by docker if it is set outside of RIOT. This could help #9646
No idea what the scan-build does.

[cladmi]

I would even add more hierarchy, why building in the 'source' part of the build directory.
It is still building in the source directory. And maybe app_name is a directory in your repository.

Choosing these directory hierarchy should also be a task on itself and I would like to discuss and propose some rough idea of a plan for review by the rest of the community.
I would use the bitbake build directory as a reference for comparison.

[smlng]

@cladmi thanks for your comments, hints and ideas - maybe I'll step down a bit and first introduce the build directory, which I need (want to use) for test output, see #10095.

And I think this is also related to #10100

[smlng]

@cladmi I reduced this to simply introducing a $(RIOTBASE)/build directory without changing any existing directories - which might be done in followup PRs

[smlng]

@cladmi any comments, it should be non-intrusive now but I'd like to have it in place for #10095

One thing I was thinking about was to add the current commit hash (git rev-parse HEAD) to the folder hierarchy, making it $(RIOTBASE)/build/<commit>/ - what do you think? That we can have a history of build results over several commits.

[add]

which could be handy for CI and testing

[smlng]

@cladmi integrated your PR, rebased and squashed

[smlng]

to recap after all those changes back and forth, this PR only introduces a new directory it does not change any existing directories or where things are build and so on. This is only a first step to prepare such things, but also to have a common directory to put test results, i.e. produces by robot framework see related PRs

[smlng]

@cladmi ping?

[cladmi]

Changes look good to me.

Good :)

[cladmi]

Let me just re-test quickly.

Question, do you have any plans/doc for the internal organization of the directory?
Or do we assume the organization is currently on demand and can be changed whenever we want?

Because I see many different usages for this one, like output/artifacts, cache/download/git, compiled_tools, tmp/bin/shared and could be a good idea to sort usages by type somehow.

[cladmi]

ACK, the test from cladmi@cfb0889 still works. The directory is correctly ignored.

Until the internal organization is documented/specified, I will assume the filesystem organization is not an API and can change anytime.

[cladmi]

Just re-running murdock with the last master.

[smlng]

@cladmi: as of now my immediate use case is to store HIL test results of the RobotFramework and therefore I currently was thinking of the following structure: /robot/$(BOARD)/$(APPLICATION) see #10774. But certainly the future goal is to allow for out-of-source builds and to make the RIOT-OS source dir read-only (or at least leave it unchanged), thus it makes sense to use this folder for caching and downloading of external stuff, too.

[cladmi]

@smlng at least I would prefer output/robot. By having a common output or artifacts directory, it could be a directory that gets saved by murdock builders. And if something needs to add other outputs (documentation ?), we do not need to modify murdock code for every directory.

[cladmi]

I will open an issue/PR for discussing an organization inside.

[cladmi]

I noticed an issue, I forgot to grep for BUILD_DIR when reviewing:

RIOT/pkg/jerryscript/Makefile.jerryscript

Line 1 in 7226daf

BUILD_DIR ?= $(BINDIR)/jerryscript

I will do a PR to rename it.