All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
@ 2011-02-01 11:18 Bjørn Forsman
  2011-02-01 20:34 ` Thomas Petazzoni
  0 siblings, 1 reply; 8+ messages in thread
From: Bjørn Forsman @ 2011-02-01 11:18 UTC (permalink / raw)
  To: buildroot

The Buildroot documentation says $(PKG)_SITE_METHOD can be set to upper
case names like SVN or GIT, but the actual implementation (currently)
accepts lower case. This patch changes lower case to upper case in the
implementation to stay in sync with the documentation.

Signed-off-by: Bj?rn Forsman <bjorn.forsman@gmail.com>
---
 package/Makefile.package.in |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/Makefile.package.in b/package/Makefile.package.in
index 92ce4e2..c3e7dd1 100644
--- a/package/Makefile.package.in
+++ b/package/Makefile.package.in
@@ -189,9 +189,9 @@ define DOWNLOAD
 	fi ; \
 	if test -n "$(1)" ; then \
 		case "$($(PKG)_SITE_METHOD)" in \
-			git) $($(DL_MODE)_GIT) && exit ;; \
-			svn) $($(DL_MODE)_SVN) && exit ;; \
-			bzr) $($(DL_MODE)_BZR) && exit ;; \
+			GIT) $($(DL_MODE)_GIT) && exit ;; \
+			SVN) $($(DL_MODE)_SVN) && exit ;; \
+			BZR) $($(DL_MODE)_BZR) && exit ;; \
 			*) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
 		esac ; \
 	fi ; \
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 11:18 [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD Bjørn Forsman
@ 2011-02-01 20:34 ` Thomas Petazzoni
  2011-02-01 20:51   ` Daniel Nyström
  2011-02-01 21:30   ` Bjørn Forsman
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas Petazzoni @ 2011-02-01 20:34 UTC (permalink / raw)
  To: buildroot

On Tue,  1 Feb 2011 12:18:49 +0100
Bj?rn Forsman <bjorn.forsman@gmail.com> wrote:

>  		case "$($(PKG)_SITE_METHOD)" in \
> -			git) $($(DL_MODE)_GIT) && exit ;; \
> -			svn) $($(DL_MODE)_SVN) && exit ;; \
> -			bzr) $($(DL_MODE)_BZR) && exit ;; \
> +			GIT) $($(DL_MODE)_GIT) && exit ;; \
> +			SVN) $($(DL_MODE)_SVN) && exit ;; \
> +			BZR) $($(DL_MODE)_BZR) && exit ;; \
>  			*) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
>  		esac ; \
>  	fi ; \

No. This is going to break:

        # Try automatic detection using the scheme part of the URI
        $(2)_SITE_METHOD = $(firstword $(subst ://, ,$(call qstrip,$($(2)_SITE))))

is used to detect the site method from the URI, like :

	git://....

or

	svn://....

so if only upper case site methods are accepted, it's not going to work.

And more generally, I'd prefer to keep the existing lower-case writing
of the site method, since it is coherent with how it's written in the
URI.

Regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 20:34 ` Thomas Petazzoni
@ 2011-02-01 20:51   ` Daniel Nyström
  2011-02-01 21:00     ` Thomas Petazzoni
  2011-02-01 21:30   ` Bjørn Forsman
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Nyström @ 2011-02-01 20:51 UTC (permalink / raw)
  To: buildroot

011/2/1 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> On Tue, ?1 Feb 2011 12:18:49 +0100
> Bj?rn Forsman <bjorn.forsman@gmail.com> wrote:
>
>> ? ? ? ? ? ? ? case "$($(PKG)_SITE_METHOD)" in \
>> - ? ? ? ? ? ? ? ? ? ? git) $($(DL_MODE)_GIT) && exit ;; \
>> - ? ? ? ? ? ? ? ? ? ? svn) $($(DL_MODE)_SVN) && exit ;; \
>> - ? ? ? ? ? ? ? ? ? ? bzr) $($(DL_MODE)_BZR) && exit ;; \
>> + ? ? ? ? ? ? ? ? ? ? GIT) $($(DL_MODE)_GIT) && exit ;; \
>> + ? ? ? ? ? ? ? ? ? ? SVN) $($(DL_MODE)_SVN) && exit ;; \
>> + ? ? ? ? ? ? ? ? ? ? BZR) $($(DL_MODE)_BZR) && exit ;; \
>> ? ? ? ? ? ? ? ? ? ? ? *) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
>> ? ? ? ? ? ? ? esac ; \
>> ? ? ? fi ; \
>
> No. This is going to break:
>
> ? ? ? ?# Try automatic detection using the scheme part of the URI
> ? ? ? ?$(2)_SITE_METHOD = $(firstword $(subst ://, ,$(call qstrip,$($(2)_SITE))))
>
> is used to detect the site method from the URI, like :
>
> ? ? ? ?git://....
>
> or
>
> ? ? ? ?svn://....
>
> so if only upper case site methods are accepted, it's not going to work.
>
> And more generally, I'd prefer to keep the existing lower-case writing
> of the site method, since it is coherent with how it's written in the
> URI.

Maybe, after all, this is a special case where both upper and lower
case should work?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 20:51   ` Daniel Nyström
@ 2011-02-01 21:00     ` Thomas Petazzoni
  2011-02-01 21:19       ` Bjørn Forsman
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Petazzoni @ 2011-02-01 21:00 UTC (permalink / raw)
  To: buildroot

On Tue, 1 Feb 2011 21:51:38 +0100
Daniel Nystr?m <daniel.nystrom@timeterminal.se> wrote:

> Maybe, after all, this is a special case where both upper and lower
> case should work?

Hum, why would it be necessary to accept upper case spelling ?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 21:00     ` Thomas Petazzoni
@ 2011-02-01 21:19       ` Bjørn Forsman
  0 siblings, 0 replies; 8+ messages in thread
From: Bjørn Forsman @ 2011-02-01 21:19 UTC (permalink / raw)
  To: buildroot

On 1 February 2011 22:00, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Tue, 1 Feb 2011 21:51:38 +0100
> Daniel Nystr?m <daniel.nystrom@timeterminal.se> wrote:
>
>> Maybe, after all, this is a special case where both upper and lower
>> case should work?
>
> Hum, why would it be necessary to accept upper case spelling ?

The BR documentation uses upper case spelling. So currently
implementation != documentation.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 20:34 ` Thomas Petazzoni
  2011-02-01 20:51   ` Daniel Nyström
@ 2011-02-01 21:30   ` Bjørn Forsman
  2011-02-01 21:36     ` Bjørn Forsman
  1 sibling, 1 reply; 8+ messages in thread
From: Bjørn Forsman @ 2011-02-01 21:30 UTC (permalink / raw)
  To: buildroot

On 1 February 2011 21:34, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Tue, ?1 Feb 2011 12:18:49 +0100
> Bj?rn Forsman <bjorn.forsman@gmail.com> wrote:
>
>> ? ? ? ? ? ? ? case "$($(PKG)_SITE_METHOD)" in \
>> - ? ? ? ? ? ? ? ? ? ? git) $($(DL_MODE)_GIT) && exit ;; \
>> - ? ? ? ? ? ? ? ? ? ? svn) $($(DL_MODE)_SVN) && exit ;; \
>> - ? ? ? ? ? ? ? ? ? ? bzr) $($(DL_MODE)_BZR) && exit ;; \
>> + ? ? ? ? ? ? ? ? ? ? GIT) $($(DL_MODE)_GIT) && exit ;; \
>> + ? ? ? ? ? ? ? ? ? ? SVN) $($(DL_MODE)_SVN) && exit ;; \
>> + ? ? ? ? ? ? ? ? ? ? BZR) $($(DL_MODE)_BZR) && exit ;; \
>> ? ? ? ? ? ? ? ? ? ? ? *) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
>> ? ? ? ? ? ? ? esac ; \
>> ? ? ? fi ; \
>
> No. This is going to break:
>
> ? ? ? ?# Try automatic detection using the scheme part of the URI
> ? ? ? ?$(2)_SITE_METHOD = $(firstword $(subst ://, ,$(call qstrip,$($(2)_SITE))))
>
> is used to detect the site method from the URI, like :
>
> ? ? ? ?git://....
>
> or
>
> ? ? ? ?svn://....
>
> so if only upper case site methods are accepted, it's not going to work.

Oh, didn't see that one.

> And more generally, I'd prefer to keep the existing lower-case writing
> of the site method, since it is coherent with how it's written in the
> URI.

How about my first patch then, allowing both upper and lower case (to
stay in sync with doc)? Or reword the doc?

Having doc != implementation is confusing so one of them should get patched :-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 21:30   ` Bjørn Forsman
@ 2011-02-01 21:36     ` Bjørn Forsman
  2011-02-01 21:41       ` Daniel Nyström
  0 siblings, 1 reply; 8+ messages in thread
From: Bjørn Forsman @ 2011-02-01 21:36 UTC (permalink / raw)
  To: buildroot

2011/2/1 Bj?rn Forsman <bjorn.forsman@gmail.com>:
> On 1 February 2011 21:34, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> On Tue, ?1 Feb 2011 12:18:49 +0100
>> Bj?rn Forsman <bjorn.forsman@gmail.com> wrote:
>>
>>> ? ? ? ? ? ? ? case "$($(PKG)_SITE_METHOD)" in \
>>> - ? ? ? ? ? ? ? ? ? ? git) $($(DL_MODE)_GIT) && exit ;; \
>>> - ? ? ? ? ? ? ? ? ? ? svn) $($(DL_MODE)_SVN) && exit ;; \
>>> - ? ? ? ? ? ? ? ? ? ? bzr) $($(DL_MODE)_BZR) && exit ;; \
>>> + ? ? ? ? ? ? ? ? ? ? GIT) $($(DL_MODE)_GIT) && exit ;; \
>>> + ? ? ? ? ? ? ? ? ? ? SVN) $($(DL_MODE)_SVN) && exit ;; \
>>> + ? ? ? ? ? ? ? ? ? ? BZR) $($(DL_MODE)_BZR) && exit ;; \
>>> ? ? ? ? ? ? ? ? ? ? ? *) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
>>> ? ? ? ? ? ? ? esac ; \
>>> ? ? ? fi ; \
>>
>> No. This is going to break:
>>
>> ? ? ? ?# Try automatic detection using the scheme part of the URI
>> ? ? ? ?$(2)_SITE_METHOD = $(firstword $(subst ://, ,$(call qstrip,$($(2)_SITE))))
>>
>> is used to detect the site method from the URI, like :
>>
>> ? ? ? ?git://....
>>
>> or
>>
>> ? ? ? ?svn://....
>>
>> so if only upper case site methods are accepted, it's not going to work.
>
> Oh, didn't see that one.
>
>> And more generally, I'd prefer to keep the existing lower-case writing
>> of the site method, since it is coherent with how it's written in the
>> URI.
>
> How about my first patch then, allowing both upper and lower case (to
> stay in sync with doc)? Or reword the doc?
>
> Having doc != implementation is confusing so one of them should get patched :-)

Hm, I see other places where lower case names are used:

ifeq ($$($(2)_SITE_METHOD),svn)
DL_TOOLS_DEPENDENCIES += svn
else ifeq ($$($(2)_SITE_METHOD),git)
DL_TOOLS_DEPENDENCIES += git
else ifeq ($$($(2)_SITE_METHOD),bzr)
DL_TOOLS_DEPENDENCIES += bzr
endif # SITE_METHOD

So the best thing is probably to patch the documentation.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD
  2011-02-01 21:36     ` Bjørn Forsman
@ 2011-02-01 21:41       ` Daniel Nyström
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Nyström @ 2011-02-01 21:41 UTC (permalink / raw)
  To: buildroot

2011/2/1 Bj?rn Forsman <bjorn.forsman@gmail.com>:
> So the best thing is probably to patch the documentation.

While on it; maybe change YES/NO into yes/no will make all parameters
lower case?

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-02-01 21:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-01 11:18 [Buildroot] [PATCH] Makefile.package.in: fix upper case $(PKG)_SITE_METHOD Bjørn Forsman
2011-02-01 20:34 ` Thomas Petazzoni
2011-02-01 20:51   ` Daniel Nyström
2011-02-01 21:00     ` Thomas Petazzoni
2011-02-01 21:19       ` Bjørn Forsman
2011-02-01 21:30   ` Bjørn Forsman
2011-02-01 21:36     ` Bjørn Forsman
2011-02-01 21:41       ` Daniel Nyström

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.