Patchwork [RFC,v1,01/14] Add a new "src" directory in the output directory

login
register
mail settings
Submitter Thomas Petazzoni
Date Jan. 20, 2013, 11:52 p.m.
Message ID <1358725943-31485-2-git-send-email-thomas.petazzoni@free-electrons.com>
Download mbox | patch
Permalink /patch/214000/
State Changes Requested
Headers show

Comments

Thomas Petazzoni - Jan. 20, 2013, 11:52 p.m.
This new directory will be used to extract the source directory of the
different packages, as part of the out of tree support.

Note that we need to explicitly re-add write permissions on the
contents of this directory, because write permissions are removed from
the source code of packages in order to ensure that the packages are
not incorrectly modifying their source directory during an out of tree
build. We also re-add write permissions to the build directory since
some packages copy files from their source directory to the build
directory, causing rm to fail due to lack of permissions.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
Jérôme Pouiller - Jan. 21, 2013, 1:47 p.m.
Hello Thomas,

(I repost my remarks there in order to keep all discussion in same thread)

On Monday 21 January 2013 00:52:10 Thomas Petazzoni wrote:
> This new directory will be used to extract the source directory of the
> different packages, as part of the out of tree support.
> 
> Note that we need to explicitly re-add write permissions on the
> contents of this directory, because write permissions are removed from
> the source code of packages in order to ensure that the packages are
> not incorrectly modifying their source directory during an out of tree
> build. We also re-add write permissions to the build directory since
> some packages copy files from their source directory to the build
> directory, causing rm to fail due to lack of permissions.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Makefile |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6f8ed0e..15ce4e4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -260,6 +260,7 @@ GENERATE_LOCALE=$(call qstrip,$(BR2_GENERATE_LOCALE))
>  STAMP_DIR:=$(BASE_DIR)/stamps
> 
>  BINARIES_DIR:=$(BASE_DIR)/images
> +SRC_DIR:=$(BASE_DIR)/src
IMHO, $(SRC_DIR) should be in $(TOPDIR) to be shared between different 
configurations. In add $(SRC_DIR) have to customizable in case I would like to 
clearly separate two projects.

I think, my workspace would look like this:
  br/srcs-projet1
  br/srcs-projet2
  br/output-project1-configA
  br/output-project1-configB
  br/output-project2-configA
  br/output-project2-configB

Finally SRC_DIR should be handled like DL_DIR.

[...]
Thomas Petazzoni - Jan. 21, 2013, 5:16 p.m.
Dear Jérôme Pouiller,

On Mon, 21 Jan 2013 14:47:22 +0100, Jérôme Pouiller wrote:

> IMHO, $(SRC_DIR) should be in $(TOPDIR) to be shared between
> different configurations. In add $(SRC_DIR) have to customizable in
> case I would like to clearly separate two projects.

I have indeed thought about making the source code shareable across
rebuilds and across projects, but since we apply patches to the source
code, it seems a bit complicated to do in a reliable way.

What happens when you bump your Buildroot version and the patches for a
given package change? You would have to manually clean up the source
directories. Not sure we want to do that, at least for now, but the
opinion of others would be interesting here.

Best regards,

Thomas
Jérôme Pouiller - Jan. 22, 2013, 10:15 a.m.
On Monday 21 January 2013 18:16:40 Thomas Petazzoni wrote:
> Dear Jérôme Pouiller,
> 
> On Mon, 21 Jan 2013 14:47:22 +0100, Jérôme Pouiller wrote:
> > IMHO, $(SRC_DIR) should be in $(TOPDIR) to be shared between
> > different configurations. In add $(SRC_DIR) have to customizable in
> > case I would like to clearly separate two projects.
> 
> I have indeed thought about making the source code shareable across
> rebuilds and across projects, but since we apply patches to the source
> code, it seems a bit complicated to do in a reliable way.
Hmm... If source is not shared across builds, what is advantage of build out 
of sources? 

I see two advantages:
  - No duplication of source between host and target. But finaly, only a few 
(and small) package are concerned. Shall we care about this?
  - To make build of a package outside of Buildroot easier. In this case, did 
you consider to build in a subdirectory of source? It does not allow to share 
source between host and target but may be sufficient and may solve a bunch of 
questions.

> What happens when you bump your Buildroot version and the patches for a
> given package change? You would have to manually clean up the source
> directories. Not sure we want to do that, at least for now, but the
> opinion of others would be interesting here.
Currently, when you bump your version of Buildroot, you have to clean all. 
Buildroot may provide a rule to also cleaning source directory. 



Regards,
Thomas Petazzoni - Jan. 22, 2013, 4:49 p.m.
Dear Jérôme Pouiller,

On Tue, 22 Jan 2013 11:15:11 +0100, Jérôme Pouiller wrote:

> Hmm... If source is not shared across builds, what is advantage of build out 
> of sources? 
> 
> I see two advantages:
>   - No duplication of source between host and target. But finaly, only a few 
> (and small) package are concerned. Shall we care about this?
>   - To make build of a package outside of Buildroot easier. In this case, did 
> you consider to build in a subdirectory of source? It does not allow to share 
> source between host and target but may be sufficient and may solve a bunch of 
> questions.

This last point is the main reason. For now, the LINUX_OVERRIDE_SRCDIR
mechanism is barely usable because it does a complete rsync of the
Linux source tree, which is very annoying. So, switching all packages
to out of tree build sounds like an interesting solution.

> > What happens when you bump your Buildroot version and the patches for a
> > given package change? You would have to manually clean up the source
> > directories. Not sure we want to do that, at least for now, but the
> > opinion of others would be interesting here.
> Currently, when you bump your version of Buildroot, you have to clean all. 
> Buildroot may provide a rule to also cleaning source directory. 

In my opinion, it makes the whole thing a lot more complicated to
understand. At the moment:

	make clean = removes the output directory

That's explained in just one easy line.

If we start putting the source trees outside of the output directory,
things get a lot more complicated for new users. They have to
understand there is one tree for the build results, one tree for the
sources, understand when and why to clean which of the trees and so on.
To me, it's not ok.

Remember, simplicity of use is a *key* point of Buildroot. We are very
attentive about this.

Best regards,

Thomas
Jérôme Pouiller - Jan. 22, 2013, 5:12 p.m.
Hello Thomas,

On Tuesday 22 January 2013 17:49:02 Thomas Petazzoni wrote:
> Dear Jérôme Pouiller,
> 
> On Tue, 22 Jan 2013 11:15:11 +0100, Jérôme Pouiller wrote:
> > Hmm... If source is not shared across builds, what is advantage of build
> > out of sources?
> > 
> > I see two advantages:
> >   - No duplication of source between host and target. But finaly, only a
> >   few
> > 
> > (and small) package are concerned. Shall we care about this?
> > 
> >   - To make build of a package outside of Buildroot easier. In this case,
> >   did
> > 
> > you consider to build in a subdirectory of source? It does not allow to
> > share source between host and target but may be sufficient and may solve
> > a bunch of questions.
> 
> This last point is the main reason. For now, the LINUX_OVERRIDE_SRCDIR
> mechanism is barely usable because it does a complete rsync of the
> Linux source tree, which is very annoying. So, switching all packages
> to out of tree build sounds like an interesting solution.
And what about building in a subdirectory of source directory ? 

[...]
> Remember, simplicity of use is a *key* point of Buildroot. We are very
> attentive about this.
Ok, I share your point of view about simplicity. 

Regards,
Thomas Petazzoni - Jan. 23, 2013, 3:45 p.m.
Dear Jérôme Pouiller,

On Tue, 22 Jan 2013 18:12:49 +0100, Jérôme Pouiller wrote:

> > This last point is the main reason. For now, the
> > LINUX_OVERRIDE_SRCDIR mechanism is barely usable because it does a
> > complete rsync of the Linux source tree, which is very annoying.
> > So, switching all packages to out of tree build sounds like an
> > interesting solution.
> And what about building in a subdirectory of source directory ? 

I am not sure why you think it would solve any problem, but it is
anyway not possible for packages for which <pkg>_OVERRIDE_SRCDIR is
used: we certainly do *not* want to build in a subdirectory of the
source directory.

Best regards,

Thomas
Jérôme Pouiller - Jan. 24, 2013, 10:34 a.m.
Hello Thomas,

On Wednesday 23 January 2013 16:45:36 Thomas Petazzoni wrote:
> Dear Jérôme Pouiller,
> 
> On Tue, 22 Jan 2013 18:12:49 +0100, Jérôme Pouiller wrote:
> > > This last point is the main reason. For now, the
> > > LINUX_OVERRIDE_SRCDIR mechanism is barely usable because it does a
> > > complete rsync of the Linux source tree, which is very annoying.
> > > So, switching all packages to out of tree build sounds like an
> > > interesting solution.
> > 
> > And what about building in a subdirectory of source directory ?
> 
> I am not sure why you think it would solve any problem, but it is
> anyway not possible for packages for which <pkg>_OVERRIDE_SRCDIR is
> used: we certainly do *not* want to build in a subdirectory of the
> source directory.
Mainly because packages not compiling out-of-source won't need to be rsynced 
between SRCDIR ans BUILDDIR.

Next it may solve questions about of dirclean behaviour.

Finally because I like idea that a directory contains all files related to a 
package.

Sure, overrided packages have be to handled differently. 

But it is only an idea, not sure it is a good idea :-) .

Patch

diff --git a/Makefile b/Makefile
index 6f8ed0e..15ce4e4 100644
--- a/Makefile
+++ b/Makefile
@@ -260,6 +260,7 @@  GENERATE_LOCALE=$(call qstrip,$(BR2_GENERATE_LOCALE))
 STAMP_DIR:=$(BASE_DIR)/stamps
 
 BINARIES_DIR:=$(BASE_DIR)/images
+SRC_DIR:=$(BASE_DIR)/src
 TARGET_DIR:=$(BASE_DIR)/target
 TOOLCHAIN_DIR=$(BASE_DIR)/toolchain
 TARGET_SKELETON=$(TOPDIR)/system/skeleton
@@ -383,7 +384,7 @@  TARGETS_LEGAL_INFO:=$(patsubst %,%-legal-info,\
 $(TARGETS_ALL): __real_tgt_%: $(BASE_TARGETS) %
 
 dirs: $(TOOLCHAIN_DIR) $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
-	$(HOST_DIR) $(BINARIES_DIR) $(STAMP_DIR)
+	$(HOST_DIR) $(BINARIES_DIR) $(SRC_DIR) $(STAMP_DIR)
 
 $(BASE_TARGETS): dirs $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
 
@@ -409,7 +410,7 @@  world: toolchain $(TARGETS_ALL)
 # dependencies anywhere else
 #
 #############################################################
-$(TOOLCHAIN_DIR) $(BUILD_DIR) $(HOST_DIR) $(BINARIES_DIR) $(STAMP_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR):
+$(TOOLCHAIN_DIR) $(BUILD_DIR) $(HOST_DIR) $(BINARIES_DIR) $(SRC_DIR) $(STAMP_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR):
 	@mkdir -p $@
 
 $(STAGING_DIR):
@@ -704,9 +705,10 @@  ifeq ($(NEED_WRAPPER),y)
 endif
 
 clean:
+	-chmod u+w -R $(SRC_DIR)/* $(BUILD_DIR)/*
 	rm -rf $(STAGING_DIR) $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
 		$(STAMP_DIR) $(BUILD_DIR) $(TOOLCHAIN_DIR) $(BASE_DIR)/staging \
-		$(LEGAL_INFO_DIR)
+		$(LEGAL_INFO_DIR) $(SRC_DIR)
 
 distclean: clean
 ifeq ($(DL_DIR),$(TOPDIR)/dl)