Skip to contentSkip to conversation

Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

PATCH New Bjørn Forsman 3 participants 8 messages Feb 1, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

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.

---
 package/Makefile.package.in |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
package/Makefile.package.in +3 -3
index 92ce4e2..c3e7dd1 100644
@@ -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 ; \
Signature · 1 line
1.7.1
#1 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

package/Makefile.package.in define DOWNLOAD · line 197

Quoted 10 lines · On Tue, 1 Feb 2011 12:18:49 +0100 Bj?rn Forsman <[email protected]>…
package/Makefile.package.in +3 -3
 		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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#2 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

011/2/1 Thomas Petazzoni <[email protected]>:

Quoted 25 lines
Quoted 10 lines · On Tue, ?1 Feb 2011 12:18:49 +0100 Bj?rn Forsman <[email protected]>…

? ? ? ? ? ? ? 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

Quoted 7 lines

? ? ? ?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?

#3 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

Quoted 2 lines · On Tue, 1 Feb 2011 21:51:38 +0100 Daniel Nystr?m…

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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#4 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

Quoted 7 lines · Thomas Petazzoni
Quoted 2 lines · On Tue, 1 Feb 2011 21:51:38 +0100 Daniel Nystr?m…

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.

#5 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

In reply to Thomas Petazzoni
Quoted 21 lines · Thomas Petazzoni
Quoted 10 lines · On Tue, ?1 Feb 2011 12:18:49 +0100 Bj?rn Forsman <[email protected]>…

? ? ? ? ? ? ? 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 :

Quoted 7 lines

? ? ? ?git://....

or

? ? ? ?svn://....

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

Oh, didn't see that one.

Quoted 3 lines

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 :-)

#6 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

2011/2/1 Bj?rn Forsman <[email protected]>:

Quoted 33 lines
Quoted 21 lines · Thomas Petazzoni
Quoted 10 lines · On Tue, ?1 Feb 2011 12:18:49 +0100 Bj?rn Forsman <[email protected]>…

? ? ? ? ? ? ? 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 :

Quoted 7 lines

? ? ? ?git://....

or

? ? ? ?svn://....

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

Oh, didn't see that one.

Quoted 8 lines
Quoted 3 lines

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.

#7 lore raw

PATCH 1/1 Makefile.package.in: fix upper case $(PKG)_SITE_METHOD

2011/2/1 Bj?rn Forsman <[email protected]>:

Quoted 1 line

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?