diff mbox

[dpdk-dev,v3,7/8] mk: sort object files when building deps lists

Message ID 20170623184153.24488-8-lboccass@brocade.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Luca Boccassi June 23, 2017, 6:41 p.m. UTC 
  From: Luca Boccassi <luca.boccassi@gmail.com>

In order to achieve reproducible builds, always use the same
order when listing object files to build dependencies lists.

Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>
---
 mk/rte.app.mk     | 4 ++--
 mk/rte.hostapp.mk | 4 ++--
 mk/rte.shared.mk  | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

Comments

Thomas Monjalon June 26, 2017, 11:20 p.m. UTC | #1 
23/06/2017 20:41, lboccass@brocade.com:
> From: Luca Boccassi <luca.boccassi@gmail.com>
> 
> In order to achieve reproducible builds, always use the same
> order when listing object files to build dependencies lists.
> 
> Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>
> ---
>  mk/rte.app.mk     | 4 ++--
>  mk/rte.hostapp.mk | 4 ++--
>  mk/rte.shared.mk  | 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB
>  
>  # list of found libraries files (useful for deps). If not found, the
>  # library is silently ignored and dep won't be checked
> -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
> -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
> +LDLIBS_FILES := $(sort $(wildcard $(foreach dir,$(LDLIBS_PATH),\
> +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))

You cannot sort libraries.
Check - for instance - this comment above in this file:
	# Eliminate duplicates without sorting, only keep the last occurrence
	filter-libs = \

Why sorting them?
What is random in libraries list?
Luca Boccassi June 27, 2017, 10:43 a.m. UTC | #2 
On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:
> 23/06/2017 20:41, lboccass@brocade.com:

> > From: Luca Boccassi <luca.boccassi@gmail.com>

> > 

> > In order to achieve reproducible builds, always use the same

> > order when listing object files to build dependencies lists.

> > 

> > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>

> > ---

> >  mk/rte.app.mk     | 4 ++--

> >  mk/rte.hostapp.mk | 4 ++--

> >  mk/rte.shared.mk  | 4 ++--

> >  3 files changed, 6 insertions(+), 6 deletions(-)

> > 

> > --- a/mk/rte.app.mk

> > +++ b/mk/rte.app.mk

> > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-

> > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB

> >  

> >  # list of found libraries files (useful for deps). If not found,

> > the

> >  # library is silently ignored and dep won't be checked

> > -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\

> > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))

> > +LDLIBS_FILES := $(sort $(wildcard $(foreach dir,$(LDLIBS_PATH),\

> > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))

> 

> You cannot sort libraries.

> Check - for instance - this comment above in this file:

> 	# Eliminate duplicates without sorting, only keep the last

> occurrence

> 	filter-libs = \


Not sure I follow - what's the reason for avoiding to sort the list of
libs to link against?

> Why sorting them?

> What is random in libraries list?


The issue is that the output of wildcard is not fully deterministic. It
can depend on the filesystem, and even on the locale settings [1].
Before GNU Make 3.82 (2009) it used to automatically sort the output,
but that was removed (to make it faster... sigh). [2]



-- 
Kind regards,
Luca Boccassi

[1] https://reproducible-builds.org/docs/stable-inputs/
[2] http://comments.gmane.org/gmane.comp.gnu.make.bugs/4260
Thomas Monjalon June 27, 2017, 1:52 p.m. UTC | #3 
27/06/2017 12:43, Luca Boccassi:
> On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:
> > 23/06/2017 20:41, lboccass@brocade.com:
> > > From: Luca Boccassi <luca.boccassi@gmail.com>
> > > 
> > > In order to achieve reproducible builds, always use the same
> > > order when listing object files to build dependencies lists.
> > > 
> > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>
> > > ---
> > >  mk/rte.app.mk     | 4 ++--
> > >  mk/rte.hostapp.mk | 4 ++--
> > >  mk/rte.shared.mk  | 4 ++--
> > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-
> > > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB
> > >  
> > >  # list of found libraries files (useful for deps). If not found,
> > > the
> > >  # library is silently ignored and dep won't be checked
> > > -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
> > > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
> > > +LDLIBS_FILES := $(sort $(wildcard $(foreach dir,$(LDLIBS_PATH),\
> > > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))
> > 
> > You cannot sort libraries.
> > Check - for instance - this comment above in this file:
> > 	# Eliminate duplicates without sorting, only keep the last
> > occurrence
> > 	filter-libs = \
> 
> Not sure I follow - what's the reason for avoiding to sort the list of
> libs to link against?

Sorry, the ordering issue is with LDLIBS not LDLIBS_FILES.

> > Why sorting them?
> > What is random in libraries list?
> 
> The issue is that the output of wildcard is not fully deterministic. It
> can depend on the filesystem, and even on the locale settings [1].
> Before GNU Make 3.82 (2009) it used to automatically sort the output,
> but that was removed (to make it faster... sigh). [2]

It is not a true wildcard here. It is just filtering files which
do not exist.
I think you do not need this patch for deterministic build.
Luca Boccassi June 27, 2017, 2:51 p.m. UTC | #4 
On Tue, 2017-06-27 at 15:52 +0200, Thomas Monjalon wrote:
> 27/06/2017 12:43, Luca Boccassi:

> > On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:

> > > 23/06/2017 20:41, lboccass@brocade.com:

> > > > From: Luca Boccassi <luca.boccassi@gmail.com>

> > > > 

> > > > In order to achieve reproducible builds, always use the same

> > > > order when listing object files to build dependencies lists.

> > > > 

> > > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>

> > > > ---

> > > >  mk/rte.app.mk     | 4 ++--

> > > >  mk/rte.hostapp.mk | 4 ++--

> > > >  mk/rte.shared.mk  | 4 ++--

> > > >  3 files changed, 6 insertions(+), 6 deletions(-)

> > > > 

> > > > --- a/mk/rte.app.mk

> > > > +++ b/mk/rte.app.mk

> > > > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-

> > > > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB

> > > >  

> > > >  # list of found libraries files (useful for deps). If not

> > > > found,

> > > > the

> > > >  # library is silently ignored and dep won't be checked

> > > > -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\

> > > > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))

> > > > +LDLIBS_FILES := $(sort $(wildcard $(foreach

> > > > dir,$(LDLIBS_PATH),\

> > > > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))

> > > 

> > > You cannot sort libraries.

> > > Check - for instance - this comment above in this file:

> > > 	# Eliminate duplicates without sorting, only keep the last

> > > occurrence

> > > 	filter-libs = \

> > 

> > Not sure I follow - what's the reason for avoiding to sort the list

> > of

> > libs to link against?

> 

> Sorry, the ordering issue is with LDLIBS not LDLIBS_FILES.

> 

> > > Why sorting them?

> > > What is random in libraries list?

> > 

> > The issue is that the output of wildcard is not fully

> > deterministic. It

> > can depend on the filesystem, and even on the locale settings [1].

> > Before GNU Make 3.82 (2009) it used to automatically sort the

> > output,

> > but that was removed (to make it faster... sigh). [2]

> 

> It is not a true wildcard here. It is just filtering files which

> do not exist.

> I think you do not need this patch for deterministic build.


But then those lists are passed down in the .SECONDEXPANSION rule
right?

I am trying to find out an alternative solution. The problem to solve
is that the build system picks the public headers path (which is
embedded in the object files as notation and in the debug symbol) from
a seemingly random location between the make install path and the
source path (build/include/rte_foo.h vs lib/librte_foo/rte_foo.h) and
this makes the build not reproducible.

Nonetheless, while I work more on the last 4 patches, could you please
have a look and eventually take patch 3 and 4? They are needed to
respectively have a deterministic list of files in the libdpdk.so
linker script and a list of source files in one of the example
documentation files.

Thanks!

-- 
Kind regards,
Luca Boccassi
Thomas Monjalon June 27, 2017, 4:14 p.m. UTC | #5 
27/06/2017 16:51, Luca Boccassi:
> On Tue, 2017-06-27 at 15:52 +0200, Thomas Monjalon wrote:
> > 27/06/2017 12:43, Luca Boccassi:
> > > On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:
> > > > 23/06/2017 20:41, lboccass@brocade.com:
> > > > > From: Luca Boccassi <luca.boccassi@gmail.com>
> > > > > 
> > > > > In order to achieve reproducible builds, always use the same
> > > > > order when listing object files to build dependencies lists.
> > > > > 
> > > > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>
> > > > > ---
> > > > >  mk/rte.app.mk     | 4 ++--
> > > > >  mk/rte.hostapp.mk | 4 ++--
> > > > >  mk/rte.shared.mk  | 4 ++--
> > > > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > > > > 
> > > > > --- a/mk/rte.app.mk
> > > > > +++ b/mk/rte.app.mk
> > > > > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-
> > > > > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB
> > > > >  
> > > > >  # list of found libraries files (useful for deps). If not
> > > > > found,
> > > > > the
> > > > >  # library is silently ignored and dep won't be checked
> > > > > -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
> > > > > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
> > > > > +LDLIBS_FILES := $(sort $(wildcard $(foreach
> > > > > dir,$(LDLIBS_PATH),\
> > > > > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))
> > > > 
> > > > You cannot sort libraries.
> > > > Check - for instance - this comment above in this file:
> > > > 	# Eliminate duplicates without sorting, only keep the last
> > > > occurrence
> > > > 	filter-libs = \
> > > 
> > > Not sure I follow - what's the reason for avoiding to sort the list
> > > of
> > > libs to link against?
> > 
> > Sorry, the ordering issue is with LDLIBS not LDLIBS_FILES.
> > 
> > > > Why sorting them?
> > > > What is random in libraries list?
> > > 
> > > The issue is that the output of wildcard is not fully
> > > deterministic. It
> > > can depend on the filesystem, and even on the locale settings [1].
> > > Before GNU Make 3.82 (2009) it used to automatically sort the
> > > output,
> > > but that was removed (to make it faster... sigh). [2]
> > 
> > It is not a true wildcard here. It is just filtering files which
> > do not exist.
> > I think you do not need this patch for deterministic build.
> 
> But then those lists are passed down in the .SECONDEXPANSION rule
> right?

I do not follow you.
Please explain what is the benefit of the patch in the next version.

> I am trying to find out an alternative solution. The problem to solve
> is that the build system picks the public headers path (which is
> embedded in the object files as notation and in the debug symbol) from
> a seemingly random location between the make install path and the
> source path (build/include/rte_foo.h vs lib/librte_foo/rte_foo.h) and
> this makes the build not reproducible.
> 
> Nonetheless, while I work more on the last 4 patches, could you please
> have a look and eventually take patch 3 and 4? They are needed to
> respectively have a deterministic list of files in the libdpdk.so
> linker script and a list of source files in one of the example
> documentation files.

I think it's better to consider and apply the whole series making
a reproducible build.
The rule is to avoid splitting series without good reason,
so tracking patches is easier.
Luca Boccassi June 28, 2017, 2:07 p.m. UTC | #6 
On Tue, 2017-06-27 at 18:14 +0200, Thomas Monjalon wrote:
> 27/06/2017 16:51, Luca Boccassi:

> > On Tue, 2017-06-27 at 15:52 +0200, Thomas Monjalon wrote:

> > > 27/06/2017 12:43, Luca Boccassi:

> > > > On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:

> > > > > 23/06/2017 20:41, lboccass@brocade.com:

> > > > > > From: Luca Boccassi <luca.boccassi@gmail.com>

> > > > > > 

> > > > > > In order to achieve reproducible builds, always use the

> > > > > > same

> > > > > > order when listing object files to build dependencies

> > > > > > lists.

> > > > > > 

> > > > > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>

> > > > > > ---

> > > > > >  mk/rte.app.mk     | 4 ++--

> > > > > >  mk/rte.hostapp.mk | 4 ++--

> > > > > >  mk/rte.shared.mk  | 4 ++--

> > > > > >  3 files changed, 6 insertions(+), 6 deletions(-)

> > > > > > 

> > > > > > --- a/mk/rte.app.mk

> > > > > > +++ b/mk/rte.app.mk

> > > > > > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-

> > > > > > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB

> > > > > >  

> > > > > >  # list of found libraries files (useful for deps). If not

> > > > > > found,

> > > > > > the

> > > > > >  # library is silently ignored and dep won't be checked

> > > > > > -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\

> > > > > > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))

> > > > > > +LDLIBS_FILES := $(sort $(wildcard $(foreach

> > > > > > dir,$(LDLIBS_PATH),\

> > > > > > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))

> > > > > 

> > > > > You cannot sort libraries.

> > > > > Check - for instance - this comment above in this file:

> > > > > 	# Eliminate duplicates without sorting, only keep the

> > > > > last

> > > > > occurrence

> > > > > 	filter-libs = \

> > > > 

> > > > Not sure I follow - what's the reason for avoiding to sort the

> > > > list

> > > > of

> > > > libs to link against?

> > > 

> > > Sorry, the ordering issue is with LDLIBS not LDLIBS_FILES.

> > > 

> > > > > Why sorting them?

> > > > > What is random in libraries list?

> > > > 

> > > > The issue is that the output of wildcard is not fully

> > > > deterministic. It

> > > > can depend on the filesystem, and even on the locale settings

> > > > [1].

> > > > Before GNU Make 3.82 (2009) it used to automatically sort the

> > > > output,

> > > > but that was removed (to make it faster... sigh). [2]

> > > 

> > > It is not a true wildcard here. It is just filtering files which

> > > do not exist.

> > > I think you do not need this patch for deterministic build.

> > 

> > But then those lists are passed down in the .SECONDEXPANSION rule

> > right?

> 

> I do not follow you.

> Please explain what is the benefit of the patch in the next version.


I thought that these lists are used to determine which files to
recompile - and incidentally, in which order as they are passed in this
snippet in rte.compile-pre.mk:

.SECONDEXPANSION:
%.o: %.c $$(wildcard $$(dep_$$@)) $$(DEP_$$(@)) FORCE
	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
	$(if $(D),\
		@echo -n "$< -> $@ " ; \
		echo -n "file_missing=$(call boolean,$(file_missing)) " ; \
		echo -n "cmdline_changed=$(call boolean,$(call cmdline_changed,$(C_TO_O))) " ; \
		echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \
		echo "depfile_newer=$(call boolean,$(depfile_newer))")
	$(if $(or \
		$(file_missing),\
		$(call cmdline_changed,$(C_TO_O)),\
		$(depfile_missing),\
		$(depfile_newer)),\
		$(C_TO_O_DO))

Did I get that wrong? (It is a bit convoluted :-P )


But nevertheless, I've finally found the root cause for the "wandering
header" - when building the libraries, CFLAGS lists
-I$(RTE_OUTDIR)/include first and then -I$(SRCDIR) last. There is a
race, and sometimes when GCC is called the public header has already
been installed in RTE_OUTDIR and sometimes it has not. This causes the
instability, and causes the expansion of __FILE__ and the directory
listing in the DWARF objects to randomly list the full path to either
RTE_OUTDIR/include or SRCDIR for each library.

By always passing -I$(SRCDIR) first this instability is solved.

I've now dropped this patch and added this new fix in v4.
This might mean that partial rebuilds are not stable, if I understand
correctly how rte.compile-pre.mk works (eg: change 2 files, rebuild,
change them again, rebuild -> output final binary _might_ be
different), but a full rebuild from scratch now seems to be always
reproducible.

> > I am trying to find out an alternative solution. The problem to

> > solve

> > is that the build system picks the public headers path (which is

> > embedded in the object files as notation and in the debug symbol)

> > from

> > a seemingly random location between the make install path and the

> > source path (build/include/rte_foo.h vs lib/librte_foo/rte_foo.h)

> > and

> > this makes the build not reproducible.

> > 

> > Nonetheless, while I work more on the last 4 patches, could you

> > please

> > have a look and eventually take patch 3 and 4? They are needed to

> > respectively have a deterministic list of files in the libdpdk.so

> > linker script and a list of source files in one of the example

> > documentation files.

> 

> I think it's better to consider and apply the whole series making

> a reproducible build.

> The rule is to avoid splitting series without good reason,

> so tracking patches is easier.


Sure, fair enough.

-- 
Kind regards,
Luca Boccassi
Thomas Monjalon June 28, 2017, 2:37 p.m. UTC | #7 
28/06/2017 16:07, Luca Boccassi:
> On Tue, 2017-06-27 at 18:14 +0200, Thomas Monjalon wrote:
> > 27/06/2017 16:51, Luca Boccassi:
> > > On Tue, 2017-06-27 at 15:52 +0200, Thomas Monjalon wrote:
> > > > 27/06/2017 12:43, Luca Boccassi:
> > > > > On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:
> > > > > > 23/06/2017 20:41, lboccass@brocade.com:
> > > > > > > From: Luca Boccassi <luca.boccassi@gmail.com>
> > > > > > > 
> > > > > > > In order to achieve reproducible builds, always use the
> > > > > > > same
> > > > > > > order when listing object files to build dependencies
> > > > > > > lists.
> > > > > > > 
> > > > > > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>
> > > > > > > ---
> > > > > > >  mk/rte.app.mk     | 4 ++--
> > > > > > >  mk/rte.hostapp.mk | 4 ++--
> > > > > > >  mk/rte.shared.mk  | 4 ++--
> > > > > > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > > 
> > > > > > > --- a/mk/rte.app.mk
> > > > > > > +++ b/mk/rte.app.mk
> > > > > > > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst -Wl$(comma)-
> > > > > > > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB
> > > > > > >  
> > > > > > >  # list of found libraries files (useful for deps). If not
> > > > > > > found,
> > > > > > > the
> > > > > > >  # library is silently ignored and dep won't be checked
> > > > > > > -LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
> > > > > > > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
> > > > > > > +LDLIBS_FILES := $(sort $(wildcard $(foreach
> > > > > > > dir,$(LDLIBS_PATH),\
> > > > > > > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))
> > > > > > 
> > > > > > You cannot sort libraries.
> > > > > > Check - for instance - this comment above in this file:
> > > > > > 	# Eliminate duplicates without sorting, only keep the
> > > > > > last
> > > > > > occurrence
> > > > > > 	filter-libs = \
> > > > > 
> > > > > Not sure I follow - what's the reason for avoiding to sort the
> > > > > list
> > > > > of
> > > > > libs to link against?
> > > > 
> > > > Sorry, the ordering issue is with LDLIBS not LDLIBS_FILES.
> > > > 
> > > > > > Why sorting them?
> > > > > > What is random in libraries list?
> > > > > 
> > > > > The issue is that the output of wildcard is not fully
> > > > > deterministic. It
> > > > > can depend on the filesystem, and even on the locale settings
> > > > > [1].
> > > > > Before GNU Make 3.82 (2009) it used to automatically sort the
> > > > > output,
> > > > > but that was removed (to make it faster... sigh). [2]
> > > > 
> > > > It is not a true wildcard here. It is just filtering files which
> > > > do not exist.
> > > > I think you do not need this patch for deterministic build.
> > > 
> > > But then those lists are passed down in the .SECONDEXPANSION rule
> > > right?
> > 
> > I do not follow you.
> > Please explain what is the benefit of the patch in the next version.
> 
> I thought that these lists are used to determine which files to
> recompile - and incidentally, in which order as they are passed in this
> snippet in rte.compile-pre.mk:
> 
> .SECONDEXPANSION:
> %.o: %.c $$(wildcard $$(dep_$$@)) $$(DEP_$$(@)) FORCE
> 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> 	$(if $(D),\
> 		@echo -n "$< -> $@ " ; \
> 		echo -n "file_missing=$(call boolean,$(file_missing)) " ; \
> 		echo -n "cmdline_changed=$(call boolean,$(call cmdline_changed,$(C_TO_O))) " ; \
> 		echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \
> 		echo "depfile_newer=$(call boolean,$(depfile_newer))")
> 	$(if $(or \
> 		$(file_missing),\
> 		$(call cmdline_changed,$(C_TO_O)),\
> 		$(depfile_missing),\
> 		$(depfile_newer)),\
> 		$(C_TO_O_DO))
> 
> Did I get that wrong? (It is a bit convoluted :-P )

LDLIBS_FILES are just used to link the executable AFAIK.
The libraries are built in separate recursive make calls.

> But nevertheless, I've finally found the root cause for the "wandering
> header" - when building the libraries, CFLAGS lists
> -I$(RTE_OUTDIR)/include first and then -I$(SRCDIR) last. There is a
> race, and sometimes when GCC is called the public header has already
> been installed in RTE_OUTDIR and sometimes it has not. This causes the
> instability, and causes the expansion of __FILE__ and the directory
> listing in the DWARF objects to randomly list the full path to either
> RTE_OUTDIR/include or SRCDIR for each library.
> 
> By always passing -I$(SRCDIR) first this instability is solved.

Good news.
Luca Boccassi June 28, 2017, 2:49 p.m. UTC | #8 
On Wed, 2017-06-28 at 16:37 +0200, Thomas Monjalon wrote:
> 28/06/2017 16:07, Luca Boccassi:

> > On Tue, 2017-06-27 at 18:14 +0200, Thomas Monjalon wrote:

> > > 27/06/2017 16:51, Luca Boccassi:

> > > > On Tue, 2017-06-27 at 15:52 +0200, Thomas Monjalon wrote:

> > > > > 27/06/2017 12:43, Luca Boccassi:

> > > > > > On Tue, 2017-06-27 at 01:20 +0200, Thomas Monjalon wrote:

> > > > > > > 23/06/2017 20:41, lboccass@brocade.com:

> > > > > > > > From: Luca Boccassi <luca.boccassi@gmail.com>

> > > > > > > > 

> > > > > > > > In order to achieve reproducible builds, always use the

> > > > > > > > same

> > > > > > > > order when listing object files to build dependencies

> > > > > > > > lists.

> > > > > > > > 

> > > > > > > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com>

> > > > > > > > ---

> > > > > > > >  mk/rte.app.mk     | 4 ++--

> > > > > > > >  mk/rte.hostapp.mk | 4 ++--

> > > > > > > >  mk/rte.shared.mk  | 4 ++--

> > > > > > > >  3 files changed, 6 insertions(+), 6 deletions(-)

> > > > > > > > 

> > > > > > > > --- a/mk/rte.app.mk

> > > > > > > > +++ b/mk/rte.app.mk

> > > > > > > > @@ -263,8 +263,8 @@ LDLIBS_NAMES += $(patsubst

> > > > > > > > -Wl$(comma)-

> > > > > > > > l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB

> > > > > > > >  

> > > > > > > >  # list of found libraries files (useful for deps). If

> > > > > > > > not

> > > > > > > > found,

> > > > > > > > the

> > > > > > > >  # library is silently ignored and dep won't be checked

> > > > > > > > -LDLIBS_FILES := $(wildcard $(foreach

> > > > > > > > dir,$(LDLIBS_PATH),\

> > > > > > > > -	$(addprefix $(dir)/,$(LDLIBS_NAMES))))

> > > > > > > > +LDLIBS_FILES := $(sort $(wildcard $(foreach

> > > > > > > > dir,$(LDLIBS_PATH),\

> > > > > > > > +	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))

> > > > > > > 

> > > > > > > You cannot sort libraries.

> > > > > > > Check - for instance - this comment above in this file:

> > > > > > > 	# Eliminate duplicates without sorting, only keep the

> > > > > > > last

> > > > > > > occurrence

> > > > > > > 	filter-libs = \

> > > > > > 

> > > > > > Not sure I follow - what's the reason for avoiding to sort

> > > > > > the

> > > > > > list

> > > > > > of

> > > > > > libs to link against?

> > > > > 

> > > > > Sorry, the ordering issue is with LDLIBS not LDLIBS_FILES.

> > > > > 

> > > > > > > Why sorting them?

> > > > > > > What is random in libraries list?

> > > > > > 

> > > > > > The issue is that the output of wildcard is not fully

> > > > > > deterministic. It

> > > > > > can depend on the filesystem, and even on the locale

> > > > > > settings

> > > > > > [1].

> > > > > > Before GNU Make 3.82 (2009) it used to automatically sort

> > > > > > the

> > > > > > output,

> > > > > > but that was removed (to make it faster... sigh). [2]

> > > > > 

> > > > > It is not a true wildcard here. It is just filtering files

> > > > > which

> > > > > do not exist.

> > > > > I think you do not need this patch for deterministic build.

> > > > 

> > > > But then those lists are passed down in the .SECONDEXPANSION

> > > > rule

> > > > right?

> > > 

> > > I do not follow you.

> > > Please explain what is the benefit of the patch in the next

> > > version.

> > 

> > I thought that these lists are used to determine which files to

> > recompile - and incidentally, in which order as they are passed in

> > this

> > snippet in rte.compile-pre.mk:

> > 

> > .SECONDEXPANSION:

> > %.o: %.c $$(wildcard $$(dep_$$@)) $$(DEP_$$(@)) FORCE

> > 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)

> > 	$(if $(D),\

> > 		@echo -n "$< -> $@ " ; \

> > 		echo -n "file_missing=$(call boolean,$(file_missing)) "

> > ; \

> > 		echo -n "cmdline_changed=$(call boolean,$(call

> > cmdline_changed,$(C_TO_O))) " ; \

> > 		echo -n "depfile_missing=$(call

> > boolean,$(depfile_missing)) " ; \

> > 		echo "depfile_newer=$(call boolean,$(depfile_newer))")

> > 	$(if $(or \

> > 		$(file_missing),\

> > 		$(call cmdline_changed,$(C_TO_O)),\

> > 		$(depfile_missing),\

> > 		$(depfile_newer)),\

> > 		$(C_TO_O_DO))

> > 

> > Did I get that wrong? (It is a bit convoluted :-P )

> 

> LDLIBS_FILES are just used to link the executable AFAIK.

> The libraries are built in separate recursive make calls.

> 

> > But nevertheless, I've finally found the root cause for the

> > "wandering

> > header" - when building the libraries, CFLAGS lists

> > -I$(RTE_OUTDIR)/include first and then -I$(SRCDIR) last. There is a

> > race, and sometimes when GCC is called the public header has

> > already

> > been installed in RTE_OUTDIR and sometimes it has not. This causes

> > the

> > instability, and causes the expansion of __FILE__ and the directory

> > listing in the DWARF objects to randomly list the full path to

> > either

> > RTE_OUTDIR/include or SRCDIR for each library.

> > 

> > By always passing -I$(SRCDIR) first this instability is solved.

> 

> Good news.


BTW as an example here's the diff between 2 builds, if anyone is
interested:

https://paste.debian.net/973727/

-- 
Kind regards,
Luca Boccassi
diff mbox

Patch

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index bcaf1b382..54134dea4 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -263,8 +263,8 @@  LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB
 
 # list of found libraries files (useful for deps). If not found, the
 # library is silently ignored and dep won't be checked
-LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
-	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
+LDLIBS_FILES := $(sort $(wildcard $(foreach dir,$(LDLIBS_PATH),\
+	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))
 
 #
 # Compile executable file if needed
diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
index 5cb4909cb..f58173c31 100644
--- a/mk/rte.hostapp.mk
+++ b/mk/rte.hostapp.mk
@@ -69,9 +69,9 @@  O_TO_EXE_DO = @set -e; \
 -include .$(HOSTAPP).cmd
 
 # list of .a files that are linked to this application
-LDLIBS_FILES := $(wildcard \
+LDLIBS_FILES := $(sort $(wildcard \
 	$(addprefix $(RTE_OUTPUT)/lib/, \
-	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
+	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))))
 
 #
 # Compile executable file if needed
diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
index 87ccf0ba4..4e680bc03 100644
--- a/mk/rte.shared.mk
+++ b/mk/rte.shared.mk
@@ -85,8 +85,8 @@  LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIB
 
 # list of found libraries files (useful for deps). If not found, the
 # library is silently ignored and dep won't be checked
-LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
-	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
+LDLIBS_FILES := $(sort $(wildcard $(foreach dir,$(LDLIBS_PATH),\
+	$(addprefix $(dir)/,$(LDLIBS_NAMES)))))
 
 #
 # Archive objects in .so file if needed