| Message ID | 1464539299-3853-1-git-send-email-nerv@dawncrow.de |
|---|---|
| State | Superseded |
| Headers | show |
André, All, On 2016-05-29 18:28 +0200, André Hentschel spake thusly: > Signed-off-by: André Hentschel <nerv@dawncrow.de> > --- [--SNIP--] > diff --git a/package/p7zip/Config.in b/package/p7zip/Config.in > new file mode 100644 > index 0000000..1a6a4df > --- /dev/null > +++ b/package/p7zip/Config.in > @@ -0,0 +1,15 @@ > +config BR2_PACKAGE_P7ZIP > + bool "p7zip" > + depends on BR2_INSTALL_LIBSTDCPP > + depends on BR2_i386 || BR2_x86_64 || \ > + BR2_aarch64 || BR2_arm || BR2_armeb Please explain why this is limited to those architectures. You can put it in the commit log, or you may add a terse comment just above the depends-on line. If the explanations is not straightforward, prefer the commit log. (Yes, I read the previous reply that it was broken on NIOSII. Limiting to a small set of architectures wihtout an explanation is not enough.) Thanks! :-) Regards, Yann E. MORIN. > + help > + p7zip is a quick port of the command line version of 7-zip for Unix. > + (see http://www.7-zip.org) > + > + 7-Zip is a file archiver with highest compression ratio. > + > + http://sourceforge.net/projects/p7zip > + > +comment "p7zip needs a toolchain w/ C++" > + depends on !BR2_INSTALL_LIBSTDCPP > diff --git a/package/p7zip/p7zip.hash b/package/p7zip/p7zip.hash > new file mode 100644 > index 0000000..ead6d87 > --- /dev/null > +++ b/package/p7zip/p7zip.hash > @@ -0,0 +1,3 @@ > +# From https://sourceforge.net/projects/p7zip/files/p7zip/ > +md5 92cca093312b5a71a7be7dc7d1d32509 p7zip_15.14.1_src_all.tar.bz2 > +sha1 9b15a79f94230fab9b9d4f9f532c723117145c7a p7zip_15.14.1_src_all.tar.bz2 > diff --git a/package/p7zip/p7zip.mk b/package/p7zip/p7zip.mk > new file mode 100644 > index 0000000..95d47d2 > --- /dev/null > +++ b/package/p7zip/p7zip.mk > @@ -0,0 +1,25 @@ > +################################################################################ > +# > +# p7zip > +# > +################################################################################ > + > +P7ZIP_VERSION = 15.14.1 > +P7ZIP_SOURCE = p7zip_$(P7ZIP_VERSION)_src_all.tar.bz2 > +P7ZIP_SITE = http://downloads.sourceforge.net/project/p7zip/p7zip/$(P7ZIP_VERSION) > +P7ZIP_LICENSE = LGPLv2.1+ > +P7ZIP_LICENSE_FILES = DOC/License.txt > + > +# Note that the build system of p7zip is a mess, so we can't use TARGET_CONFIGURE_OPTS > +define P7ZIP_BUILD_CMDS > + $(MAKE) CC="$(TARGET_CC)" ALLFLAGS_C="$(TARGET_CFLAGS)" \ > + CXX="$(TARGET_CXX)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ > + LDFLAGS="$(TARGET_LDFLAGS)" \ Well, I at least see three variables in there (CC, CXX and LDFLAGS) that are in TARGET_CONFIGURE_OPTS. Can you do: $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \ ALLFLAGS_C="$(TARGET_CFLAGS)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ 7zr ... instead? Regards, Yann E. MORIN. > + -C $(@D) 7zr > +endef > + > +define P7ZIP_INSTALL_TARGET_CMDS > + cp -dpf $(@D)/bin/7zr $(TARGET_DIR)/usr/bin/ > +endef > + > +$(eval $(generic-package)) > -- > 1.9.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Am 29.05.2016 um 19:14 schrieb Yann E. MORIN: > André, All, > Hi > On 2016-05-29 18:28 +0200, André Hentschel spake thusly: >> Signed-off-by: André Hentschel <nerv@dawncrow.de> >> --- > [--SNIP--] >> diff --git a/package/p7zip/Config.in b/package/p7zip/Config.in >> new file mode 100644 >> index 0000000..1a6a4df >> --- /dev/null >> +++ b/package/p7zip/Config.in >> @@ -0,0 +1,15 @@ >> +config BR2_PACKAGE_P7ZIP >> + bool "p7zip" >> + depends on BR2_INSTALL_LIBSTDCPP >> + depends on BR2_i386 || BR2_x86_64 || \ >> + BR2_aarch64 || BR2_arm || BR2_armeb > > Please explain why this is limited to those architectures. > > You can put it in the commit log, or you may add a terse comment just > above the depends-on line. If the explanations is not straightforward, > prefer the commit log. > > (Yes, I read the previous reply that it was broken on NIOSII. Limiting > to a small set of architectures wihtout an explanation is not enough.) > > Thanks! :-) Can I start with a small set of archs until it is tested on more? (with a comment then anyway of course) > >> + help >> + p7zip is a quick port of the command line version of 7-zip for Unix. >> + (see http://www.7-zip.org) >> + >> + 7-Zip is a file archiver with highest compression ratio. >> + >> + http://sourceforge.net/projects/p7zip >> + >> +comment "p7zip needs a toolchain w/ C++" >> + depends on !BR2_INSTALL_LIBSTDCPP >> diff --git a/package/p7zip/p7zip.hash b/package/p7zip/p7zip.hash >> new file mode 100644 >> index 0000000..ead6d87 >> --- /dev/null >> +++ b/package/p7zip/p7zip.hash >> @@ -0,0 +1,3 @@ >> +# From https://sourceforge.net/projects/p7zip/files/p7zip/ >> +md5 92cca093312b5a71a7be7dc7d1d32509 p7zip_15.14.1_src_all.tar.bz2 >> +sha1 9b15a79f94230fab9b9d4f9f532c723117145c7a p7zip_15.14.1_src_all.tar.bz2 >> diff --git a/package/p7zip/p7zip.mk b/package/p7zip/p7zip.mk >> new file mode 100644 >> index 0000000..95d47d2 >> --- /dev/null >> +++ b/package/p7zip/p7zip.mk >> @@ -0,0 +1,25 @@ >> +################################################################################ >> +# >> +# p7zip >> +# >> +################################################################################ >> + >> +P7ZIP_VERSION = 15.14.1 >> +P7ZIP_SOURCE = p7zip_$(P7ZIP_VERSION)_src_all.tar.bz2 >> +P7ZIP_SITE = http://downloads.sourceforge.net/project/p7zip/p7zip/$(P7ZIP_VERSION) >> +P7ZIP_LICENSE = LGPLv2.1+ >> +P7ZIP_LICENSE_FILES = DOC/License.txt >> + >> +# Note that the build system of p7zip is a mess, so we can't use TARGET_CONFIGURE_OPTS >> +define P7ZIP_BUILD_CMDS >> + $(MAKE) CC="$(TARGET_CC)" ALLFLAGS_C="$(TARGET_CFLAGS)" \ >> + CXX="$(TARGET_CXX)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ >> + LDFLAGS="$(TARGET_LDFLAGS)" \ > > Well, I at least see three variables in there (CC, CXX and LDFLAGS) that > are in TARGET_CONFIGURE_OPTS. Can you do: > > $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \ > ALLFLAGS_C="$(TARGET_CFLAGS)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ > 7zr > > ... instead? We can't touch CFLAGS and CXXFLAGS, other option would be to patch p7zip, but it's already too messy, no intention to touch that...
André, All, On 2016-05-29 19:22 +0200, André Hentschel spake thusly: > Am 29.05.2016 um 19:14 schrieb Yann E. MORIN: > > On 2016-05-29 18:28 +0200, André Hentschel spake thusly: > >> Signed-off-by: André Hentschel <nerv@dawncrow.de> > >> --- > > [--SNIP--] > >> diff --git a/package/p7zip/Config.in b/package/p7zip/Config.in > >> new file mode 100644 > >> index 0000000..1a6a4df > >> --- /dev/null > >> +++ b/package/p7zip/Config.in > >> @@ -0,0 +1,15 @@ > >> +config BR2_PACKAGE_P7ZIP > >> + bool "p7zip" > >> + depends on BR2_INSTALL_LIBSTDCPP > >> + depends on BR2_i386 || BR2_x86_64 || \ > >> + BR2_aarch64 || BR2_arm || BR2_armeb > > > > Please explain why this is limited to those architectures. > > > > You can put it in the commit log, or you may add a terse comment just > > above the depends-on line. If the explanations is not straightforward, > > prefer the commit log. > > > > (Yes, I read the previous reply that it was broken on NIOSII. Limiting > > to a small set of architectures wihtout an explanation is not enough.) > > > > Thanks! :-) > > Can I start with a small set of archs until it is tested on more? > (with a comment then anyway of course) Yes, that's perfectly OK for me. Just say so, like: Only enabled on a subset of known-working architectures. It fails to build on some archs, like NIOSII. [--SNIP--] > >> diff --git a/package/p7zip/p7zip.mk b/package/p7zip/p7zip.mk > >> new file mode 100644 > >> index 0000000..95d47d2 > >> --- /dev/null > >> +++ b/package/p7zip/p7zip.mk > >> @@ -0,0 +1,25 @@ > >> +################################################################################ > >> +# > >> +# p7zip > >> +# > >> +################################################################################ > >> + > >> +P7ZIP_VERSION = 15.14.1 > >> +P7ZIP_SOURCE = p7zip_$(P7ZIP_VERSION)_src_all.tar.bz2 > >> +P7ZIP_SITE = http://downloads.sourceforge.net/project/p7zip/p7zip/$(P7ZIP_VERSION) > >> +P7ZIP_LICENSE = LGPLv2.1+ > >> +P7ZIP_LICENSE_FILES = DOC/License.txt > >> + > >> +# Note that the build system of p7zip is a mess, so we can't use TARGET_CONFIGURE_OPTS > >> +define P7ZIP_BUILD_CMDS > >> + $(MAKE) CC="$(TARGET_CC)" ALLFLAGS_C="$(TARGET_CFLAGS)" \ > >> + CXX="$(TARGET_CXX)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ > >> + LDFLAGS="$(TARGET_LDFLAGS)" \ > > > > Well, I at least see three variables in there (CC, CXX and LDFLAGS) that > > are in TARGET_CONFIGURE_OPTS. Can you do: > > > > $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \ > > ALLFLAGS_C="$(TARGET_CFLAGS)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ > > 7zr > > > > ... instead? > > We can't touch CFLAGS and CXXFLAGS, OK, I guess we should also say so: # p7zip buildsystem is a mess: it plays dirty tricks with CFLAGS and # CXXFLAGS, so we can't pass them. Instead, it accepts ALLFLAGS_C # and ALLFLAGS_CPP as variables to pass the CFLAGS and CXXFLAGS. > other option would be to patch > p7zip, but it's already too messy, no intention to touch that... Meh... ;-) Thanks! Regards, Yann E. MORIN.
Am Sun, 29 May 2016 18:28:19 +0200 schrieb André Hentschel: > + depends on BR2_i386 || BR2_x86_64 || \ > + BR2_aarch64 || BR2_arm || BR2_armeb Hi André, I also tested with xtensa and microblazeel with success, please do not get confused by the list of archs in C/CpuArch.h. Lines 69 || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)) and 85 || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)) are a major catch-all which however does not work out for blackfin. Adding || defined(__BLACKFIN__) around line 68 in C/CpuArch.h should be enough to fix the problem because blackfin is little endian: https://git.busybox.net/buildroot/tree/arch/Config.in.bfin#n69 Regards, Bernd
Hello, On Sun, 29 May 2016 19:22:50 +0200, André Hentschel wrote: > >> +config BR2_PACKAGE_P7ZIP > >> + bool "p7zip" > >> + depends on BR2_INSTALL_LIBSTDCPP > >> + depends on BR2_i386 || BR2_x86_64 || \ > >> + BR2_aarch64 || BR2_arm || BR2_armeb > > > > Please explain why this is limited to those architectures. > > > > You can put it in the commit log, or you may add a terse comment just > > above the depends-on line. If the explanations is not straightforward, > > prefer the commit log. > > > > (Yes, I read the previous reply that it was broken on NIOSII. Limiting > > to a small set of architectures wihtout an explanation is not enough.) > > > > Thanks! :-) > > Can I start with a small set of archs until it is tested on more? > (with a comment then anyway of course) I will disagree with Yann here. We typically do not limit the architectures based simply on what has been tested or not. A dependency on specific architectures should only be added if the package is either 1/ contains some architecture-specific code or 2/ is known to not build/work properly on some architectures. In any case, the fact that you could only test on i386/AArch64/ARM is not a proper reason to restrict the package to those architectures only. Thomas
Am 30.05.2016 um 15:20 schrieb Thomas Petazzoni: > Hello, > > On Sun, 29 May 2016 19:22:50 +0200, André Hentschel wrote: > >>>> +config BR2_PACKAGE_P7ZIP >>>> + bool "p7zip" >>>> + depends on BR2_INSTALL_LIBSTDCPP >>>> + depends on BR2_i386 || BR2_x86_64 || \ >>>> + BR2_aarch64 || BR2_arm || BR2_armeb >>> >>> Please explain why this is limited to those architectures. >>> >>> You can put it in the commit log, or you may add a terse comment just >>> above the depends-on line. If the explanations is not straightforward, >>> prefer the commit log. >>> >>> (Yes, I read the previous reply that it was broken on NIOSII. Limiting >>> to a small set of architectures wihtout an explanation is not enough.) >>> >>> Thanks! :-) >> >> Can I start with a small set of archs until it is tested on more? >> (with a comment then anyway of course) > > I will disagree with Yann here. We typically do not limit the > architectures based simply on what has been tested or not. > > A dependency on specific architectures should only be added if the > package is either 1/ contains some architecture-specific code or 2/ is > known to not build/work properly on some architectures. > > In any case, the fact that you could only test on i386/AArch64/ARM is > not a proper reason to restrict the package to those architectures only. I simply used my autobuild machine now to test on almost all archs, bfin is the only troublemaker. > Building p7zip for blackfin using > > http://autobuild.buildroot.net/toolchains/configs/bfin-uclinux.config > > as defconfig fails with > > ../../../../CPP/myWindows/mySplitCommandLine.cpp:99:8: error: #error ENDIANNESS > > Regards, Bernd I tried it, the compiler doesn't expose ENDIANess defines... reason unknown...
diff --git a/package/Config.in b/package/Config.in index 9d668bf..073b88e 100644 --- a/package/Config.in +++ b/package/Config.in @@ -59,6 +59,7 @@ endif source "package/lz4/Config.in" source "package/lzip/Config.in" source "package/lzop/Config.in" + source "package/p7zip/Config.in" source "package/unrar/Config.in" if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/unzip/Config.in" diff --git a/package/p7zip/Config.in b/package/p7zip/Config.in new file mode 100644 index 0000000..1a6a4df --- /dev/null +++ b/package/p7zip/Config.in @@ -0,0 +1,15 @@ +config BR2_PACKAGE_P7ZIP + bool "p7zip" + depends on BR2_INSTALL_LIBSTDCPP + depends on BR2_i386 || BR2_x86_64 || \ + BR2_aarch64 || BR2_arm || BR2_armeb + help + p7zip is a quick port of the command line version of 7-zip for Unix. + (see http://www.7-zip.org) + + 7-Zip is a file archiver with highest compression ratio. + + http://sourceforge.net/projects/p7zip + +comment "p7zip needs a toolchain w/ C++" + depends on !BR2_INSTALL_LIBSTDCPP diff --git a/package/p7zip/p7zip.hash b/package/p7zip/p7zip.hash new file mode 100644 index 0000000..ead6d87 --- /dev/null +++ b/package/p7zip/p7zip.hash @@ -0,0 +1,3 @@ +# From https://sourceforge.net/projects/p7zip/files/p7zip/ +md5 92cca093312b5a71a7be7dc7d1d32509 p7zip_15.14.1_src_all.tar.bz2 +sha1 9b15a79f94230fab9b9d4f9f532c723117145c7a p7zip_15.14.1_src_all.tar.bz2 diff --git a/package/p7zip/p7zip.mk b/package/p7zip/p7zip.mk new file mode 100644 index 0000000..95d47d2 --- /dev/null +++ b/package/p7zip/p7zip.mk @@ -0,0 +1,25 @@ +################################################################################ +# +# p7zip +# +################################################################################ + +P7ZIP_VERSION = 15.14.1 +P7ZIP_SOURCE = p7zip_$(P7ZIP_VERSION)_src_all.tar.bz2 +P7ZIP_SITE = http://downloads.sourceforge.net/project/p7zip/p7zip/$(P7ZIP_VERSION) +P7ZIP_LICENSE = LGPLv2.1+ +P7ZIP_LICENSE_FILES = DOC/License.txt + +# Note that the build system of p7zip is a mess, so we can't use TARGET_CONFIGURE_OPTS +define P7ZIP_BUILD_CMDS + $(MAKE) CC="$(TARGET_CC)" ALLFLAGS_C="$(TARGET_CFLAGS)" \ + CXX="$(TARGET_CXX)" ALLFLAGS_CPP="$(TARGET_CXXFLAGS)" \ + LDFLAGS="$(TARGET_LDFLAGS)" \ + -C $(@D) 7zr +endef + +define P7ZIP_INSTALL_TARGET_CMDS + cp -dpf $(@D)/bin/7zr $(TARGET_DIR)/usr/bin/ +endef + +$(eval $(generic-package))
Signed-off-by: André Hentschel <nerv@dawncrow.de> --- package/Config.in | 1 + package/p7zip/Config.in | 15 +++++++++++++++ package/p7zip/p7zip.hash | 3 +++ package/p7zip/p7zip.mk | 25 +++++++++++++++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 package/p7zip/Config.in create mode 100644 package/p7zip/p7zip.hash create mode 100644 package/p7zip/p7zip.mk