Skip to contentSkip to conversation

Add dvd+rw-tools as package dvdrw-tools

PATCH New Steve Kenton 2 participants 2 messages Feb 21, 2015 – Mar 15, 2015

lore
Share & tools
lore

Steve Kenton

author
#0 lore raw

PATCH 1/1 Add dvd+rw-tools as package dvdrw-tools

Signed-off-by Steve Kenton <[email protected]>
---
 package/dvdrw-tools/0001-limits.h.patch | 19 ++++++++++++
 package/dvdrw-tools/Config.in           | 53 +++++++++++++++++++++++++++++++++
 package/dvdrw-tools/dvdrw-tools.hash    |  3 ++
 package/dvdrw-tools/dvdrw-tools.mk      | 38 +++++++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 package/dvdrw-tools/0001-limits.h.patch
 create mode 100644 package/dvdrw-tools/Config.in
 create mode 100644 package/dvdrw-tools/dvdrw-tools.hash
 create mode 100644 package/dvdrw-tools/dvdrw-tools.mk
package/dvdrw-tools/0001-limits.h.patch new file +19 -0
new file mode 100644
index 0000000..a5c7484
@@ -0,0 +1,19 @@
+diff -pruN dvd+rw-tools-7.1.ori/transport.hxx dvd+rw-tools-7.1/transport.hxx
+--- dvd+rw-tools-7.1.ori/transport.hxx	2008-03-01 04:34:43.000000000 -0600
++++ dvd+rw-tools-7.1/transport.hxx	2015-01-18 15:47:24.245863631 -0600
+@@ -9,6 +9,7 @@
+ #if defined(__unix) || defined(__unix__)
+ #include <stdio.h>
+ #include <stdlib.h>
++#include <limits.h>
+ #include <unistd.h>
+ #include <string.h>
+ #include <sys/types.h>
+@@ -40,6 +41,7 @@ inline long getmsecs()
+ #include <stddef.h>
+ #include <stdio.h>
+ #include <stdlib.h>
++#include <limits.h>
+ #define ssize_t		LONG_PTR
+ #define off64_t		__int64
+
package/dvdrw-tools/Config.in new file +53 -0

53 lines not on this page · raw message

package/dvdrw-tools/dvdrw-tools.hash new file +3 -0
new file mode 100644
index 0000000..5d79709
@@ -0,0 +1,3 @@
+# Locally computed using sha256sum
+sha256	f8d60f822e914128bcbc5f64fbe3ed131cbff9045dca7e12c5b77b26edde72ca	dvd+rw-tools-7.1.tar.gz
+
package/dvdrw-tools/dvdrw-tools.mk new file +38 -0
new file mode 100644
index 0000000..cc704d6
@@ -0,0 +1,38 @@
+################################################################################
+#
+# dvdrw-tools
+#
+################################################################################
+
+DVDRW_TOOLS_VERSION = 7.1
+DVDRW_TOOLS_SOURCE = dvd+rw-tools-$(DVDRW_TOOLS_VERSION).tar.gz
+DVDRW_TOOLS_SITE = http://fy.chalmers.se/~appro/linux/DVD+RW/tools
+DVDRW_TOOLS_LICENSE = GPLv2
+DVDRW_TOOLS_LICENSE_FILES = LICENSE
+
+define DVDRW_TOOLS_BUILD_CMDS
+	$(MAKE) -C $(@D)
+endef
+
+BACKEND =
+
+ifneq ($(BR2_PACKAGE_CDRTOOLS),y)
+ifeq ($(BR2_PACKAGE_DVDRW_CDRKIT),y)
+	BACKEND = genisoimage
+endif
+
+ifeq ($(BR2_PACKAGE_DVDRW_XORRISO),y)
+	BACKEND = xorrisofs
+endif
+endif
+
+define DVDRW_TOOLS_INSTALL_TARGET_CMDS
+	$(INSTALL) -m 0755 -D $(@D)/dvd-ram-control $(TARGET_DIR)/usr/bin/dvd-ram-control
+	$(INSTALL) -m 0755 -D $(@D)/dvd+rw-booktype $(TARGET_DIR)/usr/bin/dvd+rw-booktype
+	$(INSTALL) -m 0755 -D $(@D)/dvd+rw-format $(TARGET_DIR)/usr/bin/dvd+rw-format
+	$(INSTALL) -m 0755 -D $(@D)/dvd+rw-mediainfo $(TARGET_DIR)/usr/bin/dvd+rw-mediainfo
+	$(INSTALL) -m 0755 -D $(@D)/growisofs $(TARGET_DIR)/usr/bin/growisofs
+	test -z "$(BACKEND)" || ln -s -f /usr/bin/$(BACKEND) $(TARGET_DIR)/usr/bin/mkisofs
+endef
+
+$(eval $(generic-package))
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 Add dvd+rw-tools as package dvdrw-tools

Dear Steve Kenton,

Quoted 11 lines · Steve Kenton
Signed-off-by Steve Kenton <[email protected]>
---
 package/dvdrw-tools/0001-limits.h.patch | 19 ++++++++++++
 package/dvdrw-tools/Config.in           | 53 +++++++++++++++++++++++++++++++++
 package/dvdrw-tools/dvdrw-tools.hash    |  3 ++
 package/dvdrw-tools/dvdrw-tools.mk      | 38 +++++++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 package/dvdrw-tools/0001-limits.h.patch
 create mode 100644 package/dvdrw-tools/Config.in
 create mode 100644 package/dvdrw-tools/dvdrw-tools.hash
 create mode 100644 package/dvdrw-tools/dvdrw-tools.mk

Thanks for this patch, and sorry for the slow response time. However, I have a number of comments, see below.

package/dvdrw-tools/0001-limits.h.patch

Quoted 5 lines
package/dvdrw-tools/0001-limits.h.patch new file +0 -0
new file mode 100644
index 0000000..a5c7484

All patches need a description + a Signed-off-by line. See
http://buildroot.uclibc.org/downloads/manual/manual.html#_format_and_licensing_of_the_package_patches.

Quoted 27 lines
package/dvdrw-tools/Config.in new file +21 -0
new file mode 100644
index 0000000..48254b3
@@ -0,0 +1,53 @@
+config BR2_PACKAGE_DVDRW_TOOLS
+	bool "dvdrw-tools"
+	depends on BR2_USE_WCHAR
+	depends on BR2_LARGEFILE
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	depends on BR2_USE_MMU # fork()
+	help
+	  The dvd+rw-tools are used to master Blu-ray Disc
+	  and DVD Disc media, both +RW/+R and -RW/-R. The
+	  +RW in the name is a historical artifact. This
+	  package contains the widely used growisofs program.
+
+	  Buildroot does not support packages with a '+' sign
+	  in their name, which explains why it is named
+	  dvdrw-tools and not dvd+rw-tools.
+
+	  Because dvd+rw-tools does not directly interact with
+	  disc media it uses a separate media backend program.
+	  The usual backend is mkisofs from the cdrtools package.
+	  However, cdrtools is not currently part of buildroot.
+

package/dvdrw-tools/Config.in line 31

Quoted 10 lines
package/dvdrw-tools/Config.in new file +10 -0
+	  The Linux From Scratch project uses uses xorriso for the
+	  media backend and Ubuntu uses cdrkit for the backend.
+	  Choose the one which seems most appropriate for your use.
+	  NOTE: xorriso does not currently support UDF.
+	  
+	  http://fy.chalmers.se/~appro/linux/DVD+RW/tools
+
+comment "dvdrw-tools needs a toolchain w/ wchar, largefile"
+	depends on !BR2_USE_WCHAR || !BR2_LARGEFILE \
+		|| !BR2_TOOLCHAIN_HAS_THREADS || !BR2_USE_MMU

This is not properly written. It should be:

comment "dvdrw-tools needs a toolchain w/ wchar, largefile, threads"

	depends on BR2_USE_MMU
	depends on !BR2_USE_WCHAR || !BR2_LARGEFILE || \
		!BR2_TOOLCHAIN_HAS_THREADS

The two issues with your proposal was:

- The thread dependency was not mentioned in the comment.

- The BR2_USE_MMU dependency must be handled specially: we don't want to see the comment if BR2_USE_MMU is disabled.

Also, since there is some C++ code, you probably need a dependency on BR2_INSTALL_LIBSTDCPP and the corresponding comment update.

package/dvdrw-tools/Config.in line 33

Quoted 2 lines
package/dvdrw-tools/Config.in new file +2 -0
+
+if BR2_PACKAGE_DVDRW_TOOLS && !BR2_PACKAGE_CDRTOOLS

BR2_PACKAGE_CDRTOOLS does not exist in Buildroot, so why do we have a reference to this non exiting variable?

package/dvdrw-tools/Config.in line 40

Quoted 1 line
package/dvdrw-tools/Config.in new file +1 -0
+config BR2_PACKAGE_DVDRW_CDRKIT

All Config.in variables of a package must start with the Config.in option of the package itself. I.e, this should be:

BR2_PACKAGE_DVDRW_TOOLS_CDRKIT

or maybe even better:

BR2_PACKAGE_DVDRW_TOOLS_CDRKIT_BACKEND

package/dvdrw-tools/Config.in line 37

Quoted 3 lines
package/dvdrw-tools/Config.in new file +3 -0
+define DVDRW_TOOLS_BUILD_CMDS
+	$(MAKE) -C $(@D)
+endef

How can this even cross-compile? You're not specifying the cross-compiler path. Most likely, it at least needs to be:

	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D)

package/dvdrw-tools/Config.in line 38

Quoted 1 line
package/dvdrw-tools/Config.in new file +1 -0
+BACKEND =

Not needed, variables are empty by default.

And BACKEND is not a proper variable name: the namespace of variables in Buildroot is global. So all variables defined by a package must be prefixed by the package name, to avoid conflicts with variables defined by other packages.

package/dvdrw-tools/Config.in line 39

Quoted 1 line
package/dvdrw-tools/Config.in new file +1 -0
+ifneq ($(BR2_PACKAGE_CDRTOOLS),y)

Not needed, BR2_PACKAGE_CDRTOOLS does not exist in Buildroot.

package/dvdrw-tools/Config.in line 47

Quoted 8 lines
package/dvdrw-tools/Config.in new file +8 -0
+ifeq ($(BR2_PACKAGE_DVDRW_CDRKIT),y)
+	BACKEND = genisoimage
+endif
+
+ifeq ($(BR2_PACKAGE_DVDRW_XORRISO),y)
+	BACKEND = xorrisofs
+endif
+endif

This should probably be:

ifeq ($(BR2_PACKAGE_DVDRW_TOOLS_CDRKIT_BACKEND),y) DVDRW_TOOLS_BACKEND = genisoimage
else ifeq ($(BR2_PACKAGE_DVDRW_TOOLS_XORRISO_BACKEND),y) DVDRW_TOOLS_BACKEND = xorrisofs
endif

package/dvdrw-tools/Config.in

Quoted 8 lines
package/dvdrw-tools/Config.in new file +8 -0
+
+define DVDRW_TOOLS_INSTALL_TARGET_CMDS
+	$(INSTALL) -m 0755 -D $(@D)/dvd-ram-control $(TARGET_DIR)/usr/bin/dvd-ram-control
+	$(INSTALL) -m 0755 -D $(@D)/dvd+rw-booktype $(TARGET_DIR)/usr/bin/dvd+rw-booktype
+	$(INSTALL) -m 0755 -D $(@D)/dvd+rw-format $(TARGET_DIR)/usr/bin/dvd+rw-format
+	$(INSTALL) -m 0755 -D $(@D)/dvd+rw-mediainfo $(TARGET_DIR)/usr/bin/dvd+rw-mediainfo
+	$(INSTALL) -m 0755 -D $(@D)/growisofs $(TARGET_DIR)/usr/bin/growisofs
+	test -z "$(BACKEND)" || ln -s -f /usr/bin/$(BACKEND) $(TARGET_DIR)/usr/bin/mkisofs

No need for the test -z test, since you're using a choice for the backend, there is no way the BACKEND variable can be empty.

I've marked your patch as Changes Requested in our patch tracking system, so please resubmit an updated version of your patch!

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com