From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-1?Q?J=E9r=F4me?= Pouiller <jezz@sysmic.org>
Date: Fri, 10 Feb 2017 18:40:04 +0100
Subject: [Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt'
	with TLP
In-Reply-To: <20170209232429.0c133424@free-electrons.com>
References: <20161114132238.6569-1-jezz@sysmic.org>
 <20161114132238.6569-4-jezz@sysmic.org>
 <20170209232429.0c133424@free-electrons.com>
Message-ID: <18141971.6S5PrfofuZ@sagittae>
List-Id: <buildroot.busybox.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: buildroot@busybox.net

Hello Thomas,

On Thursday 9 February 2017 23:24:29 CET Thomas Petazzoni wrote:
> Hello,
> 
> Adding Gustavo in Cc. Gustavo, you are working on TLP support. Could
> you comment on the below patch?
> 
> See also my comments below.
> 
> On Mon, 14 Nov 2016 14:22:35 +0100, J?r?me Pouiller wrote:
> > Until now, `$(BUILD_DIR)/packages-file-list.txt' was not filled properly
> > when top level parallelization is enabled. Therefore, all scripts that
> > rely on packages-file-list.txt did not work.
> > 
> > In order to fix it,this patch place target installation task in a critical
> > section.
> > 
> > Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> > ---
> > 
> >  package/pkg-generic.mk | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 987efa6..c5f70e0 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -62,6 +62,9 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
> > 
> >  # files currently installed in the target. Note that the MD5 is also
> >  # stored, in order to identify if the files are overwritten.
> >  define step_pkg_size_start
> > 
> > +	while ! flock $(BUILD_DIR) -c "[ ! -e $(BUILD_DIR)/.target_lock ] &&
> > touch $(BUILD_DIR)/.target_lock"; do \ +		sleep 0.5; \
> > +	done
> 
> I personally don't really like this retry loop around flock, but since
> package-file-list.txt is a global file, I don't really see how to do
> otherwise. Unless storing a per-package file with just the time/date of
> the start/end of each step for this package, and then have a
> post-processing logic at the very end of the build that regroups all
> those per-package files into a single global file, ordering the entries
> by their timestamp. Don't know if it's really better.

I think that any tool involving parallel processing need one day or another
to protect code inside a critical section. Even if we find another way to 
solve current problem, I am pretty sure we will need it later, anyway.

However, I think I am going to modify my patch in order to provide a
generic mutex implementation that would be enabled iff TLP is enable.

I wonder where I should place lock file. In add, tools like fakedate,
check-shlibs-deps and other QA tools could also generate log or
temporary files. Currently, we place all of them in build/ without any
specific rules. Maybe we should have a naming policy for these files?
(prefix them with "BR_"?) or place them elsewhere than in build/?

[...]

-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr