[Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Bernd Kuhls]

This fix can be backported because the offending line was added in 2009.

Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 5d1c1da128..9d40754939 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -286,7 +286,7 @@ $(BUILD_DIR)/%/.stamp_configured:
 	$(Q)touch $@
 
 # Build
-$(BUILD_DIR)/%/.stamp_built::
+$(BUILD_DIR)/%/.stamp_built:
 	@$(call step_start,build)
 	@$(call MESSAGE,"Building")
 	$(foreach hook,$($(PKG)_PRE_BUILD_HOOKS),$(call $(hook))$(sep))
-- 
2.39.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Thomas Petazzoni via buildroot]

On Fri, 18 Aug 2023 17:16:08 +0200
Bernd Kuhls <bernd@kuhls.net> wrote:

> This fix can be backported because the offending line was added in 2009.
> 
> Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 5d1c1da128..9d40754939 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -286,7 +286,7 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	$(Q)touch $@
>  
>  # Build
> -$(BUILD_DIR)/%/.stamp_built::
> +$(BUILD_DIR)/%/.stamp_built:

Note that double colon rules have a specific meaning in make:

  https://www.gnu.org/software/make/manual/html_node/Double_002dColon.html

I've read a few times the page, but I'm not sure I fully grasp what it
says, so I don't know if the double colon was intended (and needed at
the time) or was a typo.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Yann E. MORIN]

On 2023-08-18 22:03 +0200, Thomas Petazzoni via buildroot spake thusly:
> On Fri, 18 Aug 2023 17:16:08 +0200
> Bernd Kuhls <bernd@kuhls.net> wrote:
> 
> > This fix can be backported because the offending line was added in 2009.

What is actualy broken?

As you noticed, the does has been there for moer than 14 years now, so
if something obvious was broken, we'd have notriced by now.

So, if this is broken, it is not obvious, and we need to udnerstand how
it is broken and why the fix is correct.

Regards,
Yann E. MORIN.

> > Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
> > ---
> >  package/pkg-generic.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 5d1c1da128..9d40754939 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -286,7 +286,7 @@ $(BUILD_DIR)/%/.stamp_configured:
> >  	$(Q)touch $@
> >  
> >  # Build
> > -$(BUILD_DIR)/%/.stamp_built::
> > +$(BUILD_DIR)/%/.stamp_built:
> 
> Note that double colon rules have a specific meaning in make:
> 
>   https://www.gnu.org/software/make/manual/html_node/Double_002dColon.html
> 
> I've read a few times the page, but I'm not sure I fully grasp what it
> says, so I don't know if the double colon was intended (and needed at
> the time) or was a typo.
> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Thomas Petazzoni via buildroot]

On Fri, 18 Aug 2023 22:24:34 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> On 2023-08-18 22:03 +0200, Thomas Petazzoni via buildroot spake thusly:
> > On Fri, 18 Aug 2023 17:16:08 +0200
> > Bernd Kuhls <bernd@kuhls.net> wrote:
> >   
> > > This fix can be backported because the offending line was added in 2009.  
> 
> What is actualy broken?
> 
> As you noticed, the does has been there for moer than 14 years now, so
> if something obvious was broken, we'd have notriced by now.
> 
> So, if this is broken, it is not obvious, and we need to udnerstand how
> it is broken and why the fix is correct.

I don't think Bernd claimed anywhere that something is broken. He
claims there is a typo. I've also seen this double colon several times,
and it isn't consistent across the other similar make targets, and I
always wondered if it was needed or not. If it is not needed, we should
remove it to make this rule consistent with others. If it's needed, it
should be explained in a comment above it because it's not obvious
what's going on.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Bernd Kuhls]

Hi Thomas,

Am Fri, 18 Aug 2023 22:03:14 +0200 schrieb Thomas Petazzoni via buildroot:

> Note that double colon rules have a specific meaning in make:

ah, I did not know that.

> so I don't know if the double colon was intended (and needed at
> the time) or was a typo.

Digging further into what first appeared to me as a typo I found your 
commit adding the double colon to package/Makefile.package.in:
https://git.buildroot.net/buildroot/commit/package?
id=e11fe847b2f545446fc3300dd2ba88fd5da05756

At that time there was another file, package/Makefile.autotools.in, which 
also included a "$(BUILD_DIR)/%/.stamp_built:" single-colon target:
https://git.buildroot.net/buildroot/tree/package/Makefile.autotools.in?
id=e11fe847b2f545446fc3300dd2ba88fd5da05756#n264

Today only one file, package/pkg-generic.mk, contains a .stamp_built 
target.

Maybe, I am only guessing, the double colon was necessary back then, due 
to two Makefiles containing the same target, and lost their justification 
when the single-colon target "$(BUILD_DIR)/%/.stamp_built:" was removed 
with commit https://git.buildroot.net/buildroot/commit/?
id=d8b55b99095e5eed46d2b95bdaab2e69ccbd7170

Regards, Bernd

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Yann E. MORIN]

Thomas, All,

On 2023-08-18 22:44 +0200, Thomas Petazzoni via buildroot spake thusly:
> On Fri, 18 Aug 2023 22:24:34 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > On 2023-08-18 22:03 +0200, Thomas Petazzoni via buildroot spake thusly:
> > > On Fri, 18 Aug 2023 17:16:08 +0200
> > > Bernd Kuhls <bernd@kuhls.net> wrote:
> > >   
> > > > This fix can be backported because the offending line was added in 2009.  
> > What is actualy broken?
> > As you noticed, the does has been there for moer than 14 years now, so
> > if something obvious was broken, we'd have notriced by now.
> > So, if this is broken, it is not obvious, and we need to udnerstand how
> > it is broken and why the fix is correct.
> 
> I don't think Bernd claimed anywhere that something is broken.

Right. But if that had just been a typo, then that would have been
invalid Makefile syntax, which it obviously is not, as this code does
not trigger an error from make.

So it is not a typo (so, see below).

> He
> claims there is a typo. I've also seen this double colon several times,
> and it isn't consistent across the other similar make targets, and I
> always wondered if it was needed or not. If it is not needed, we should
> remove it to make this rule consistent with others. If it's needed, it
> should be explained in a comment above it because it's not obvious
> what's going on.

Exactly.

You expressed the same reasoning I was trying to convey, but which I
failed to express properly.

But I agree that either wway, needed or not, the explanations are going
to be... difficult to come up with, too.

As an aside, we use that :: construct in one other place:

    support/kconfig/Makefile.br:42:$(obj)/%:: $(src)/%_shipped 

which is something we inherited from the 2.6.21.5 kernel in
a665ed34960bf1423cba303276161bf44a9851f4... No need to fix that one.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Thomas Petazzoni via buildroot]

On Fri, 18 Aug 2023 22:47:15 +0200
Bernd Kuhls <bernd@kuhls.net> wrote:

> Digging further into what first appeared to me as a typo I found your 
> commit adding the double colon to package/Makefile.package.in:
> https://git.buildroot.net/buildroot/commit/package?
> id=e11fe847b2f545446fc3300dd2ba88fd5da05756
> 
> At that time there was another file, package/Makefile.autotools.in, which 
> also included a "$(BUILD_DIR)/%/.stamp_built:" single-colon target:
> https://git.buildroot.net/buildroot/tree/package/Makefile.autotools.in?
> id=e11fe847b2f545446fc3300dd2ba88fd5da05756#n264
> 
> Today only one file, package/pkg-generic.mk, contains a .stamp_built 
> target.
> 
> Maybe, I am only guessing, the double colon was necessary back then, due 
> to two Makefiles containing the same target, and lost their justification 
> when the single-colon target "$(BUILD_DIR)/%/.stamp_built:" was removed 
> with commit https://git.buildroot.net/buildroot/commit/?
> id=d8b55b99095e5eed46d2b95bdaab2e69ccbd7170

Thanks for the additional research. However, I don't think your
explanation that the double colon was necessary back then because there
was a $(BUILD_DIR)/%/.stamp_built rule in package/Makefile.package.in
and another one in package/Makefile.autotools.in really makes sense:
this was also the case for all the other
$(BUILD_DIR)/%/.stamp_<something> rules, and only the .stamp_built one
had this double colon added.

I can really only think about this really being a typo.

Here is a highly convincing explanation: up to a few days ago before
you sent this patch, I had no idea what this "double colon" was doing
exactly. And I really doubt that back in 2009 when I contributed this
change I had a much better idea about it. So I don't see how it could
have been intentional from me, and therefore I believe it indeed is a
typo. I looked up the history of this patch, and it was posted only
once to the mailing list by me, I then did a pull request to Peter, who
merged the whole thing. So there was no particular discussion about
this double colon.

So, unless Yann disagrees, I think I'm going to apply Bernd's patch
with a slightly more detailed explanation.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Arnout Vandecappelle via buildroot]

On 18/08/2023 17:16, Bernd Kuhls wrote:
> This fix can be backported because the offending line was added in 2009.
> 
> Signed-off-by: Bernd Kuhls <bernd@kuhls.net>

  Applied to master with a slightly more extended commit message, thanks.

  Regards,
  Arnout

> ---
>   package/pkg-generic.mk | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 5d1c1da128..9d40754939 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -286,7 +286,7 @@ $(BUILD_DIR)/%/.stamp_configured:
>   	$(Q)touch $@
>   
>   # Build
> -$(BUILD_DIR)/%/.stamp_built::
> +$(BUILD_DIR)/%/.stamp_built:
>   	@$(call step_start,build)
>   	@$(call MESSAGE,"Building")
>   	$(foreach hook,$($(PKG)_PRE_BUILD_HOOKS),$(call $(hook))$(sep))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix typo in make target .stamp_built

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle via buildroot <buildroot@buildroot.org> writes:

 > On 18/08/2023 17:16, Bernd Kuhls wrote:
 >> This fix can be backported because the offending line was added in 2009.
 >> Signed-off-by: Bernd Kuhls <bernd@kuhls.net>

 >  Applied to master with a slightly more extended commit message, thanks.

Committed to 2024.02.x and 2024.05.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot