diff mbox

[v10,8/8] Makefile: update comment about top-level parallel Makefile

Message ID 1387363007-19846-9-git-send-email-fabio.porcedda@gmail.com
State Superseded
Headers show

Commit Message

Fabio Porcedda Dec. 18, 2013, 10:36 a.m. UTC
After the latest patches top-level parallel Makefile is working but
there is still an issue when a package has an unspecified optional
dependency so change the comment to explain that.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Makefile | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Arnout Vandecappelle Dec. 19, 2013, 5:37 p.m. UTC | #1
On 18/12/13 11:36, Fabio Porcedda wrote:
> After the latest patches top-level parallel Makefile is working but
> there is still an issue when a package has an unspecified optional
> dependency so change the comment to explain that.
>
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>   Makefile | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index ef2582e..27a6a65 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -42,7 +42,20 @@ export HOSTARCH := $(shell uname -m | \
>   	    -e s/macppc/powerpc/\
>   	    -e s/sh.*/sh/)
>
> -# This top-level Makefile can *not* be executed in parallel
> +# Parallel execution of this Makefile is disabled because it could
> +# change the build result if a package has an unspecified optional
> +# dependency.

  ... or if two packages manipulate the same file in the target directory.

> +# Parallel execution changes the packages building order, that can be
> +# a problem when a package has an unspecified optional dependency,
> +# because if that dependency is present when the package is built, it
> +# is used, otherwise it isn't (but compilation happily proceeds). This

  This is a repeat of the first sentence.

> +# means that the packages building order is relevant in that case, and
> +# the end result will differ if the order is swapped due to parallel
> +# building.
> +# Taking in account the above warning, if you still want to execute

  Taking into acount

> +# this top-level Makefile in parrallel comment the following line and

  parallel


  Regards,
  Arnout

> +# execute:
> +#	make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))
>   .NOTPARALLEL:
>
>   # absolute path
>
Fabio Porcedda Dec. 20, 2013, 1:56 p.m. UTC | #2
Hi Arnout,
thanks for reviewing.

On Thu, Dec 19, 2013 at 6:37 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 18/12/13 11:36, Fabio Porcedda wrote:
>>
>> After the latest patches top-level parallel Makefile is working but
>> there is still an issue when a package has an unspecified optional
>> dependency so change the comment to explain that.
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>   Makefile | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index ef2582e..27a6a65 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -42,7 +42,20 @@ export HOSTARCH := $(shell uname -m | \
>>             -e s/macppc/powerpc/\
>>             -e s/sh.*/sh/)
>>
>> -# This top-level Makefile can *not* be executed in parallel
>> +# Parallel execution of this Makefile is disabled because it could
>> +# change the build result if a package has an unspecified optional
>> +# dependency.
>
>
>  ... or if two packages manipulate the same file in the target directory.

Ok, added.

>
>> +# Parallel execution changes the packages building order, that can be
>> +# a problem when a package has an unspecified optional dependency,
>> +# because if that dependency is present when the package is built, it
>> +# is used, otherwise it isn't (but compilation happily proceeds). This
>
>
>  This is a repeat of the first sentence.

So i just remove it?

>
>> +# means that the packages building order is relevant in that case, and
>> +# the end result will differ if the order is swapped due to parallel
>> +# building.
>> +# Taking in account the above warning, if you still want to execute
>
>
>  Taking into acount

Ok fixed.

>
>> +# this top-level Makefile in parrallel comment the following line and
>
>
>  parallel

Ok fixed.

<snip>

Best regards
Arnout Vandecappelle Dec. 20, 2013, 3:48 p.m. UTC | #3
On 20/12/13 14:56, Fabio Porcedda wrote:
> On Thu, Dec 19, 2013 at 6:37 PM, Arnout Vandecappelle<arnout@mind.be>  wrote:
>> >On 18/12/13 11:36, Fabio Porcedda wrote:
>>> >>
>>> >>After the latest patches top-level parallel Makefile is working but
>>> >>there is still an issue when a package has an unspecified optional
>>> >>dependency so change the comment to explain that.
>>> >>
>>> >>Signed-off-by: Fabio Porcedda<fabio.porcedda@gmail.com>
>>> >>---
>>> >>   Makefile | 15 ++++++++++++++-
>>> >>   1 file changed, 14 insertions(+), 1 deletion(-)
>>> >>
>>> >>diff --git a/Makefile b/Makefile
>>> >>index ef2582e..27a6a65 100644
>>> >>--- a/Makefile
>>> >>+++ b/Makefile
>>> >>@@ -42,7 +42,20 @@ export HOSTARCH := $(shell uname -m | \
>>> >>             -e s/macppc/powerpc/\
>>> >>             -e s/sh.*/sh/)
>>> >>
>>> >>-# This top-level Makefile can*not*  be executed in parallel
>>> >>+# Parallel execution of this Makefile is disabled because it could
>>> >>+# change the build result if a package has an unspecified optional
>>> >>+# dependency.
>> >
>> >
>> >  ... or if two packages manipulate the same file in the target directory.
> Ok, added.
>
>> >
>>> >>+# Parallel execution changes the packages building order, that can be
>>> >>+# a problem when a package has an unspecified optional dependency,
>>> >>+# because if that dependency is present when the package is built, it
>>> >>+# is used, otherwise it isn't (but compilation happily proceeds). This
>> >
>> >
>> >  This is a repeat of the first sentence.
> So i just remove it?
>

  Probably better to remove the first sentence, the second one is more 
descriptive.

  Regards,
  Arnout
Fabio Porcedda Jan. 7, 2014, 10:05 a.m. UTC | #4
On Fri, Dec 20, 2013 at 4:48 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 20/12/13 14:56, Fabio Porcedda wrote:
>>
>> On Thu, Dec 19, 2013 at 6:37 PM, Arnout Vandecappelle<arnout@mind.be>
>> wrote:
>>>
>>> >On 18/12/13 11:36, Fabio Porcedda wrote:
>>>>
>>>> >>
>>>> >>After the latest patches top-level parallel Makefile is working but
>>>> >>there is still an issue when a package has an unspecified optional
>>>> >>dependency so change the comment to explain that.
>>>> >>
>>>> >>Signed-off-by: Fabio Porcedda<fabio.porcedda@gmail.com>
>>>> >>---
>>>> >>   Makefile | 15 ++++++++++++++-
>>>> >>   1 file changed, 14 insertions(+), 1 deletion(-)
>>>> >>
>>>> >>diff --git a/Makefile b/Makefile
>>>> >>index ef2582e..27a6a65 100644
>>>> >>--- a/Makefile
>>>> >>+++ b/Makefile
>>>> >>@@ -42,7 +42,20 @@ export HOSTARCH := $(shell uname -m | \
>>>> >>             -e s/macppc/powerpc/\
>>>> >>             -e s/sh.*/sh/)
>>>> >>
>>>> >>-# This top-level Makefile can*not*  be executed in parallel
>>>>
>>>> >>+# Parallel execution of this Makefile is disabled because it could
>>>> >>+# change the build result if a package has an unspecified optional
>>>> >>+# dependency.
>>>
>>> >
>>> >
>>> >  ... or if two packages manipulate the same file in the target
>>> > directory.
>>
>> Ok, added.
>>
>>> >
>>>>
>>>> >>+# Parallel execution changes the packages building order, that can be
>>>> >>+# a problem when a package has an unspecified optional dependency,
>>>> >>+# because if that dependency is present when the package is built, it
>>>> >>+# is used, otherwise it isn't (but compilation happily proceeds).
>>>> >> This
>>>
>>> >
>>> >
>>> >  This is a repeat of the first sentence.
>>
>> So i just remove it?
>>
>
>  Probably better to remove the first sentence, the second one is more
> descriptive.

This is the fixed comment:

# Parallel execution of this Makefile is disabled because it changes
# the packages building order, that can be a problem when a package
# has an unspecified optional dependency, because if that dependency
# is present when the package is built, it is used, otherwise it isn't
# (but compilation happily proceeds). This means that the packages
# building order is relevant in that case, and the end result will
# differ if the order is swapped due to parallel building.  Also
# changing the building order can be a problem if two packages
# manipulate the same file in the target directory.
# Taking into account the above warnings, if you still want to execute
# this top-level Makefile in parallel comment the following line and
# execute:
#    make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))

Regards
Mike Zick Jan. 7, 2014, 1:17 p.m. UTC | #5
On Tue, 7 Jan 2014 11:05:52 +0100
Fabio Porcedda <fabio.porcedda@gmail.com> wrote:

> This is the fixed comment:
> 
> # Parallel execution of this Makefile is disabled because it changes
> # the packages building order, that can be a problem when a package
> # has an unspecified optional dependency, because if that dependency
> # is present when the package is built, it is used, otherwise it isn't
> # (but compilation happily proceeds). This means that the packages
> # building order is relevant in that case, and the end result will
> # differ if the order is swapped due to parallel building.  Also
> # changing the building order can be a problem if two packages
> # manipulate the same file in the target directory.
> # Taking into account the above warnings, if you still want to execute
> # this top-level Makefile in parallel comment the following line and
> # execute:
> #    make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))
> 
> Regards
>

Been a silent reader for years here now, but I just must make a
suggestion:

# Parallel execution of this top-level Makefile is disabled.
#
# Single execution is forced here for two reasons:
#
# Maintaining reproducible builds:
# Packages with an unspecified, optional, dependency could
# otherwise change the build order among multiple builds.
#
# Prevent file modification collisions:
# This can happen if two packages manipulate the same file.
#
# After taking into account the above considerations;
# If you still want to execute this top-level Makefile in 
# parallel, comment out the following line and then execute:
#    make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))

Just clarify which line is "the following line" in the context
of this comment. 

Add styling to first line of each paragraph to match the context.

- - - -

I will return now to my read-only mode.
Mike
Fabio Porcedda Jan. 9, 2014, 8:10 a.m. UTC | #6
On Tue, Jan 7, 2014 at 2:17 PM, Mike Zick <minimod@morethan.org> wrote:
> On Tue, 7 Jan 2014 11:05:52 +0100
> Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
>
>> This is the fixed comment:
>>
>> # Parallel execution of this Makefile is disabled because it changes
>> # the packages building order, that can be a problem when a package
>> # has an unspecified optional dependency, because if that dependency
>> # is present when the package is built, it is used, otherwise it isn't
>> # (but compilation happily proceeds). This means that the packages
>> # building order is relevant in that case, and the end result will
>> # differ if the order is swapped due to parallel building.  Also
>> # changing the building order can be a problem if two packages
>> # manipulate the same file in the target directory.
>> # Taking into account the above warnings, if you still want to execute
>> # this top-level Makefile in parallel comment the following line and
>> # execute:
>> #    make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))
>>
>> Regards
>>
>
> Been a silent reader for years here now, but I just must make a
> suggestion:

Hi Mike,
welcome as a writer :)


> # Parallel execution of this top-level Makefile is disabled.
> #
> # Single execution is forced here for two reasons:
> #
> # Maintaining reproducible builds:
> # Packages with an unspecified, optional, dependency could
> # otherwise change the build order among multiple builds.
> #
> # Prevent file modification collisions:
> # This can happen if two packages manipulate the same file.
> #
> # After taking into account the above considerations;
> # If you still want to execute this top-level Makefile in
> # parallel, comment out the following line and then execute:
> #    make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))
>
> Just clarify which line is "the following line" in the context
> of this comment.
>
> Add styling to first line of each paragraph to match the context.

Thanks for the suggestion, maybe a mix of the two:

# Parallel execution of this Makefile is disabled because it changes
# the packages building order, that can be a problem for two reasons:
# - If a package has an unspecified optional dependency and that
#   dependency is present when the package is built, it is used,
#   otherwise it isn't (but compilation happily proceeds) so the end
#   result will differ if the order is swapped due to parallel
#   building.
# - Also changing the building order can be a problem if two packages
#   manipulate the same file in the target directory.
#
# Taking into account the above considerations, if you still want to execute
# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and
# build using the following command:
#    make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))

Regards
diff mbox

Patch

diff --git a/Makefile b/Makefile
index ef2582e..27a6a65 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,20 @@  export HOSTARCH := $(shell uname -m | \
 	    -e s/macppc/powerpc/\
 	    -e s/sh.*/sh/)
 
-# This top-level Makefile can *not* be executed in parallel
+# Parallel execution of this Makefile is disabled because it could
+# change the build result if a package has an unspecified optional
+# dependency.
+# Parallel execution changes the packages building order, that can be
+# a problem when a package has an unspecified optional dependency,
+# because if that dependency is present when the package is built, it
+# is used, otherwise it isn't (but compilation happily proceeds). This
+# means that the packages building order is relevant in that case, and
+# the end result will differ if the order is swapped due to parallel
+# building.
+# Taking in account the above warning, if you still want to execute
+# this top-level Makefile in parrallel comment the following line and
+# execute:
+#	make BR2_JLEVEL= -j$((`getconf _NPROCESSORS_ONLN`+1))
 .NOTPARALLEL:
 
 # absolute path