Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v4] rfkill: new package
@ 2015-10-27 15:32 Viacheslav Volkov
  2015-10-27 16:13 ` Yann E. MORIN
  0 siblings, 1 reply; 3+ messages in thread
From: Viacheslav Volkov @ 2015-10-27 15:32 UTC (permalink / raw)
  To: buildroot

Add rfkill package.

Signed-off-by: Viacheslav Volkov <sv99@inbox.ru>
---
Changes v3 -> v4:
  - remove trailing space in the comment  (suggested by yann.morin.1998 at free.fr)
  - remove trailing slash in the URL

Changes v2 -> v3:
  - wrapp comment to 72 characters length  (suggested by Vincent.Riera at imgtec.com)

Changes v1 -> v2:
  - correct Config.in  (suggested by Vincent.Riera at imgtec.com)
  - correct rfkill.mk
---
 package/Config.in          |  1 +
 package/rfkill/Config.in   |  7 +++++++
 package/rfkill/rfkill.hash |  2 ++
 package/rfkill/rfkill.mk   | 20 ++++++++++++++++++++
 4 files changed, 30 insertions(+)
 create mode 100644 package/rfkill/Config.in
 create mode 100644 package/rfkill/rfkill.hash
 create mode 100644 package/rfkill/rfkill.mk

diff --git a/package/Config.in b/package/Config.in
index 10ff94e..9933514 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -403,6 +403,7 @@ endif
 	source "package/pps-tools/Config.in"
 	source "package/pulseview/Config.in"
 	source "package/read-edid/Config.in"
+	source "package/rfkill/Config.in"
 	source "package/rng-tools/Config.in"
 	source "package/rpi-userland/Config.in"
 	source "package/rtl8188eu/Config.in"
diff --git a/package/rfkill/Config.in b/package/rfkill/Config.in
new file mode 100644
index 0000000..3cb7c18
--- /dev/null
+++ b/package/rfkill/Config.in
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_RFKILL
+	bool "rfkill"
+	help
+	  rfkill is a small userspace tool to query the state of the
+	  rfkill switches.
+
+	  https://wireless.wiki.kernel.org/en/users/documentation/rfkill
diff --git a/package/rfkill/rfkill.hash b/package/rfkill/rfkill.hash
new file mode 100644
index 0000000..21be0a2
--- /dev/null
+++ b/package/rfkill/rfkill.hash
@@ -0,0 +1,2 @@
+# From: https://www.kernel.org/pub/software/network/rfkill/sha256sums.asc
+sha256	e0ae3004215e39a6c5c36e0726558740728d16f67ebdb8bea621250f6091d86a  rfkill-0.5.tar.xz
diff --git a/package/rfkill/rfkill.mk b/package/rfkill/rfkill.mk
new file mode 100644
index 0000000..b377c03
--- /dev/null
+++ b/package/rfkill/rfkill.mk
@@ -0,0 +1,20 @@
+################################################################################
+#
+# rfkill
+#
+################################################################################
+
+RFKILL_VERSION = 0.5
+RFKILL_SOURCE = rfkill-$(RFKILL_VERSION).tar.xz
+RFKILL_SITE = https://www.kernel.org/pub/software/network/rfkill
+
+define RFKILL_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
+		VERSION_SUFFIX="-br"
+endef
+
+define RFKILL_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 755 $(@D)/rfkill $(TARGET_DIR)/usr/bin/rfkill
+endef
+
+$(eval $(generic-package))
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [Buildroot] [PATCH v4] rfkill: new package
  2015-10-27 15:32 [Buildroot] [PATCH v4] rfkill: new package Viacheslav Volkov
@ 2015-10-27 16:13 ` Yann E. MORIN
  2015-10-27 16:27   ` Yann E. MORIN
  0 siblings, 1 reply; 3+ messages in thread
From: Yann E. MORIN @ 2015-10-27 16:13 UTC (permalink / raw)
  To: buildroot

Viacheslav, All,

I said in my previous review that you should wait a bit before
resending, in case other may have mor ecomments.

When I said wait a bit, I really meant: wait until tomorrow (at the very
least).

Since the previous review, we discussed on IRC, and we discovered
more issues with your patch, see below...

Be sure to read up until the end of the reply, there ar emore than one
comment to it...

On 2015-10-27 18:32 +0300, Viacheslav Volkov spake thusly:
> Add rfkill package.
> 
> Signed-off-by: Viacheslav Volkov <sv99@inbox.ru>
[--SNIP--]
> diff --git a/package/rfkill/rfkill.hash b/package/rfkill/rfkill.hash
> new file mode 100644
> index 0000000..21be0a2
> --- /dev/null
> +++ b/package/rfkill/rfkill.hash
> @@ -0,0 +1,2 @@
> +# From: https://www.kernel.org/pub/software/network/rfkill/sha256sums.asc
> +sha256	e0ae3004215e39a6c5c36e0726558740728d16f67ebdb8bea621250f6091d86a  rfkill-0.5.tar.xz
> diff --git a/package/rfkill/rfkill.mk b/package/rfkill/rfkill.mk
> new file mode 100644
> index 0000000..b377c03
> --- /dev/null
> +++ b/package/rfkill/rfkill.mk
> @@ -0,0 +1,20 @@
> +################################################################################
> +#
> +# rfkill
> +#
> +################################################################################
> +
> +RFKILL_VERSION = 0.5
> +RFKILL_SOURCE = rfkill-$(RFKILL_VERSION).tar.xz
> +RFKILL_SITE = https://www.kernel.org/pub/software/network/rfkill
> +
> +define RFKILL_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
> +		VERSION_SUFFIX="-br"

Why do you specify the VERSION_SUFFIX?

Don't do it, we do not need to "specialise" the generated binaries just
because they were built with Buldroot.

> +endef
> +
> +define RFKILL_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 755 $(@D)/rfkill $(TARGET_DIR)/usr/bin/rfkill

As I said in my previous review, you should use the install directive
provided by the package's Makefile, like so;

    $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C "$(@D) \
        DESTDIR=$(TARGET_DIR) install

Again, wait a little bit before you resend (like, wait until tomorow).

Thanks! :-)

Regards,
Yann E. MORIN.

> +endef
> +
> +$(eval $(generic-package))
> -- 
> 2.6.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [Buildroot] [PATCH v4] rfkill: new package
  2015-10-27 16:13 ` Yann E. MORIN
@ 2015-10-27 16:27   ` Yann E. MORIN
  0 siblings, 0 replies; 3+ messages in thread
From: Yann E. MORIN @ 2015-10-27 16:27 UTC (permalink / raw)
  To: buildroot

Viacheslav, All,

On 2015-10-27 17:13 +0100, Yann E. MORIN spake thusly:
> Since the previous review, we discussed on IRC, and we discovered
> more issues with your patch, see below...

Vicente had even more comments on my coments. :-)

(Hence the reason why waiting after a review is important.)

> On 2015-10-27 18:32 +0300, Viacheslav Volkov spake thusly:
[--SNIP--]
> > diff --git a/package/rfkill/rfkill.mk b/package/rfkill/rfkill.mk
> > new file mode 100644
> > index 0000000..b377c03
> > --- /dev/null
> > +++ b/package/rfkill/rfkill.mk
> > @@ -0,0 +1,20 @@
> > +################################################################################
> > +#
> > +# rfkill
> > +#
> > +################################################################################
> > +
> > +RFKILL_VERSION = 0.5
> > +RFKILL_SOURCE = rfkill-$(RFKILL_VERSION).tar.xz
> > +RFKILL_SITE = https://www.kernel.org/pub/software/network/rfkill
> > +
> > +define RFKILL_BUILD_CMDS
> > +	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
> > +		VERSION_SUFFIX="-br"
> 
> Why do you specify the VERSION_SUFFIX?
> 
> Don't do it, we do not need to "specialise" the generated binaries just
> because they were built with Buldroot.

OK, Vicente explained that without it, the version.sh script will fail
miserably. I understand now.

This should be explained as a comment above the build command:

    # As part of the build, the Maefile tries to construct a version
    # string. By default, it tries to run git commands to get it, and
    # checks that it gets the same version as the one stored in the
    # Makefile. Since it would (at worst) get Buildroot version (for
    # in-tree builds), or (at best) not version string at all (in case
    # Of out-of-tree build), the version.sh script would fail miserably.
    # To avoid this, we feed it a custom VERSION_SUFFIX, which must not
    # be empty, otherwise the version.sh script is run.

> > +endef
> > +
> > +define RFKILL_INSTALL_TARGET_CMDS
> > +	$(INSTALL) -D -m 755 $(@D)/rfkill $(TARGET_DIR)/usr/bin/rfkill
> 
> As I said in my previous review, you should use the install directive
> provided by the package's Makefile, like so;
> 
>     $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C "$(@D) \
>         DESTDIR=$(TARGET_DIR) install

The $(TARGET_CONFIGURE_OPTS) is not needed at install time. Vicente
believes t should be dropped; I believe it is better for consistency, so
the install command uses the exact same environment as the build
command.

So, please keep it. If a maintainer believes it is superfluous, he can
drop it the moment he applies the patch.

> Again, wait a little bit before you resend (like, wait until tomorow).

Still valid, by the way...

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-10-27 16:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-27 15:32 [Buildroot] [PATCH v4] rfkill: new package Viacheslav Volkov
2015-10-27 16:13 ` Yann E. MORIN
2015-10-27 16:27   ` Yann E. MORIN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox