| Message ID | 1413129607-9134-1-git-send-email-maxime.hadjinlian@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Maxime, All, On 2014-10-12 18:00 +0200, Maxime Hadjinlian spake thusly: [--SNIP--] > diff --git a/package/libevent/0002-Remove-usage-of-top_srcdir.patch b/package/libevent/0002-Remove-usage-of-top_srcdir.patch > new file mode 100644 > index 0000000..34c90f0 > --- /dev/null > +++ b/package/libevent/0002-Remove-usage-of-top_srcdir.patch > @@ -0,0 +1,64 @@ > +From 8fe87a7dd2179ae6a3697af5c2a43143adba3c82 Mon Sep 17 00:00:00 2001 > +From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> > +Date: Sun, 12 Oct 2014 17:41:41 +0200 > +Subject: [PATCH 2/2] Remove usage of top_srcdir > + > +This is already upstream, waiting for a release. For an upstream patch, we like yto have an URL pointing to the upstream commit or mailing-post about it. [--SNIP--] > diff --git a/package/libevent/libevent.mk b/package/libevent/libevent.mk > index 73be502..fbc7c54 100644 > --- a/package/libevent/libevent.mk > +++ b/package/libevent/libevent.mk > @@ -5,11 +5,11 @@ > ################################################################################ > > LIBEVENT_VERSION = 2.0.21 > -LIBEVENT_SOURCE = libevent-$(LIBEVENT_VERSION)-stable.tar.gz > -LIBEVENT_SITE = https://github.com/downloads/libevent/libevent > +LIBEVENT_SITE = $(call github,libevent,libevent,release-$(LIBEVENT_VERSION)-stable) > LIBEVENT_INSTALL_STAGING = YES > LIBEVENT_LICENSE = BSD-3c, OpenBSD > LIBEVENT_LICENSE_FILES = LICENSE > +LIBEVENT_AUTORECONF = YES Please, add a small comment why we autoreconf: # Straight from the repository, need to generate autotools files Regards, Yann E. MORIN.
Dear Maxime Hadjinlian, Sorry, but the title of your patch is wrong: it does *much* more than use the github call. The title of the commit should summarize the entirety of the patch, and then the rest of the description should go in the details of what the patch does. On Sun, 12 Oct 2014 18:00:07 +0200, Maxime Hadjinlian wrote: > Add AUTORECONF as theses releases don't contains 'configure' > and such. > > Two patchs are added to make the autotools happy. > Theses patchs are already in upstream, waiting for a new release. Missing SoB line. > diff --git a/package/libevent/0001-Disable-building-tests-programs.patch b/package/libevent/0001-Disable-building-tests-programs.patch > new file mode 100644 > index 0000000..65fd928 > --- /dev/null > +++ b/package/libevent/0001-Disable-building-tests-programs.patch > @@ -0,0 +1,30 @@ > +From a4fb64af00b76600f85416471a513c89dc89d769 Mon Sep 17 00:00:00 2001 > +From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> > +Date: Sun, 12 Oct 2014 17:12:22 +0200 > +Subject: [PATCH 1/2] Disable building tests programs > + > +We are not really interested in building test programs. > +Moreover, these programs use fork() function that is not available on > +MMU-less architectures. > + > +Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> You are not the author of this patch, it was written by Gilles Tallis. Please use git format-patch -M to enable rename detection so we see that it's actually the other patch that has only be changed slightly and renamed. > LIBEVENT_VERSION = 2.0.21 > -LIBEVENT_SOURCE = libevent-$(LIBEVENT_VERSION)-stable.tar.gz > -LIBEVENT_SITE = https://github.com/downloads/libevent/libevent > +LIBEVENT_SITE = $(call github,libevent,libevent,release-$(LIBEVENT_VERSION)-stable) > LIBEVENT_INSTALL_STAGING = YES > LIBEVENT_LICENSE = BSD-3c, OpenBSD > LIBEVENT_LICENSE_FILES = LICENSE > +LIBEVENT_AUTORECONF = YES Why use the github function when a tarball is provided? Sorry, but patch unclear. Thomas
Hi all On Sun, Oct 12, 2014 at 6:17 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Maxime Hadjinlian, > > Sorry, but the title of your patch is wrong: it does *much* more than > use the github call. > > The title of the commit should summarize the entirety of the patch, and > then the rest of the description should go in the details of what the > patch does. Yes, forgot to amend my commit. > > On Sun, 12 Oct 2014 18:00:07 +0200, Maxime Hadjinlian wrote: >> Add AUTORECONF as theses releases don't contains 'configure' >> and such. >> >> Two patchs are added to make the autotools happy. >> Theses patchs are already in upstream, waiting for a new release. > > Missing SoB line. > >> diff --git a/package/libevent/0001-Disable-building-tests-programs.patch b/package/libevent/0001-Disable-building-tests-programs.patch >> new file mode 100644 >> index 0000000..65fd928 >> --- /dev/null >> +++ b/package/libevent/0001-Disable-building-tests-programs.patch >> @@ -0,0 +1,30 @@ >> +From a4fb64af00b76600f85416471a513c89dc89d769 Mon Sep 17 00:00:00 2001 >> +From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> >> +Date: Sun, 12 Oct 2014 17:12:22 +0200 >> +Subject: [PATCH 1/2] Disable building tests programs >> + >> +We are not really interested in building test programs. >> +Moreover, these programs use fork() function that is not available on >> +MMU-less architectures. >> + >> +Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> > > You are not the author of this patch, it was written by Gilles Tallis. Sorry about that, some git am/rebase going awry. Will fix right away. > > Please use git format-patch -M to enable rename detection so we see > that it's actually the other patch that has only be changed slightly > and renamed. > >> LIBEVENT_VERSION = 2.0.21 >> -LIBEVENT_SOURCE = libevent-$(LIBEVENT_VERSION)-stable.tar.gz >> -LIBEVENT_SITE = https://github.com/downloads/libevent/libevent >> +LIBEVENT_SITE = $(call github,libevent,libevent,release-$(LIBEVENT_VERSION)-stable) >> LIBEVENT_INSTALL_STAGING = YES >> LIBEVENT_LICENSE = BSD-3c, OpenBSD >> LIBEVENT_LICENSE_FILES = LICENSE >> +LIBEVENT_AUTORECONF = YES > > Why use the github function when a tarball is provided? Only because we were using the github repository before, but we could uses the tarball from the servers ? I don't know which is prefered ? > > Sorry, but patch unclear. I'll respin right away. > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Dear Maxime Hadjinlian, On Sun, 12 Oct 2014 18:30:14 +0200, Maxime Hadjinlian wrote: > > Why use the github function when a tarball is provided? > Only because we were using the github repository before, but we could > uses the tarball from the servers ? I don't know which is prefered ? My personal point of view is that if there's a tarball, then we should use it. Don't know if that opinion is shared by the community as a whole, please ask right now during the meeting :-) Thomas
Hi Maxime, Thomas, all, On Sun, Oct 12, 2014 at 6:46 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Maxime Hadjinlian, > > On Sun, 12 Oct 2014 18:30:14 +0200, Maxime Hadjinlian wrote: > >> > Why use the github function when a tarball is provided? >> Only because we were using the github repository before, but we could >> uses the tarball from the servers ? I don't know which is prefered ? > > My personal point of view is that if there's a tarball, then we should > use it. Don't know if that opinion is shared by the community as a > whole, please ask right now during the meeting :-) I agree. > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
On Sun, Oct 12, 2014 at 6:58 PM, Samuel Martin <s.martin49@gmail.com> wrote: > Hi Maxime, Thomas, all, > > On Sun, Oct 12, 2014 at 6:46 PM, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: >> Dear Maxime Hadjinlian, >> >> On Sun, 12 Oct 2014 18:30:14 +0200, Maxime Hadjinlian wrote: >> >>> > Why use the github function when a tarball is provided? >>> Only because we were using the github repository before, but we could >>> uses the tarball from the servers ? I don't know which is prefered ? >> >> My personal point of view is that if there's a tarball, then we should >> use it. Don't know if that opinion is shared by the community as a >> whole, please ask right now during the meeting :-) > > I agree. It seems that the tarball available on the website, is the tarball generated by github from the tag. > >> >> Thomas >> -- >> Thomas Petazzoni, CTO, Free Electrons >> Embedded Linux, Kernel and Android engineering >> http://free-electrons.com >> _______________________________________________ >> buildroot mailing list >> buildroot@busybox.net >> http://lists.busybox.net/mailman/listinfo/buildroot > > > > -- > Samuel
Dear Maxime Hadjinlian, On Sun, 12 Oct 2014 19:06:23 +0200, Maxime Hadjinlian wrote: > >> My personal point of view is that if there's a tarball, then we should > >> use it. Don't know if that opinion is shared by the community as a > >> whole, please ask right now during the meeting :-) > > > > I agree. > It seems that the tarball available on the website, is the tarball > generated by github from the tag. Yes, and? Thomas
On Sun, Oct 12, 2014 at 7:08 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Maxime Hadjinlian, > > On Sun, 12 Oct 2014 19:06:23 +0200, Maxime Hadjinlian wrote: > >> >> My personal point of view is that if there's a tarball, then we should >> >> use it. Don't know if that opinion is shared by the community as a >> >> whole, please ask right now during the meeting :-) >> > >> > I agree. >> It seems that the tarball available on the website, is the tarball >> generated by github from the tag. > > Yes, and? To put everything cleary in a mail: When a maintainer do a release from github, there's two possibilities: Either you don't upload a file, then the tarball you download is generated from the tag. Or you upload a file, and then the tarball, is *NOT* generated, it's the file that you uploaded. Since the hepers, will call the http URL of github (and not doing a git clone), for me the github helper is used for any generated tarball. *BUT* if the releases is a manually uploaded file, then we have to uses the http link and not the helper. I'd recommend to put a comments on each one theses cases. In the case of libevent, the link from the website points to the release on github, which is a generated tarball, not an uploaded one. So the uses of the helper is perfectly fine here. Hope I gave enough details here, maybe this should be put in the manual ? Don't know if it's really usefull. > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Dear Maxime Hadjinlian, On Sun, 12 Oct 2014 19:22:43 +0200, Maxime Hadjinlian wrote: > To put everything cleary in a mail: > > When a maintainer do a release from github, there's two possibilities: > Either you don't upload a file, then the tarball you download is > generated from the tag. > > Or you upload a file, and then the tarball, is *NOT* generated, it's > the file that you uploaded. > > Since the hepers, will call the http URL of github (and not doing a > git clone), for me the github helper is used for any generated > tarball. > *BUT* if the releases is a manually uploaded file, then we have to > uses the http link and not the helper. I'd recommend to put a comments > on each one theses cases. > > In the case of libevent, the link from the website points to the > release on github, which is a generated tarball, not an uploaded one. > So the uses of the helper is perfectly fine here. > > Hope I gave enough details here, maybe this should be put in the > manual ? Don't know if it's really usefull. Makes sense, especially when you add the information that the github helper doesn't provide a git URL that will lead to a git clone, but a http URL that will lead to wget-ing a tarball. So, fine with using the github helper here. Thanks, Thomas
diff --git a/package/libevent/0001-Disable-building-tests-programs.patch b/package/libevent/0001-Disable-building-tests-programs.patch new file mode 100644 index 0000000..65fd928 --- /dev/null +++ b/package/libevent/0001-Disable-building-tests-programs.patch @@ -0,0 +1,30 @@ +From a4fb64af00b76600f85416471a513c89dc89d769 Mon Sep 17 00:00:00 2001 +From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> +Date: Sun, 12 Oct 2014 17:12:22 +0200 +Subject: [PATCH 1/2] Disable building tests programs + +We are not really interested in building test programs. +Moreover, these programs use fork() function that is not available on +MMU-less architectures. + +Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> +--- + Makefile.am | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/Makefile.am b/Makefile.am +index 42879a3..dc90359 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -126,7 +126,7 @@ else + noinst_LTLIBRARIES = $(LIBEVENT_LIBS_LA) + endif + +-SUBDIRS = . include sample test ++SUBDIRS = . include sample + + if BUILD_WIN32 + +-- +2.1.1 + diff --git a/package/libevent/0002-Remove-usage-of-top_srcdir.patch b/package/libevent/0002-Remove-usage-of-top_srcdir.patch new file mode 100644 index 0000000..34c90f0 --- /dev/null +++ b/package/libevent/0002-Remove-usage-of-top_srcdir.patch @@ -0,0 +1,64 @@ +From 8fe87a7dd2179ae6a3697af5c2a43143adba3c82 Mon Sep 17 00:00:00 2001 +From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> +Date: Sun, 12 Oct 2014 17:41:41 +0200 +Subject: [PATCH 2/2] Remove usage of top_srcdir + +This is already upstream, waiting for a release. + +Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> +--- + Makefile.am | 4 +++- + test/Makefile.am | 10 +++++++++- + 2 files changed, 12 insertions(+), 2 deletions(-) + +diff --git a/Makefile.am b/Makefile.am +index dc90359..c34576d 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -128,6 +128,8 @@ endif + + SUBDIRS = . include sample + ++DISTCLEANFILES= ++ + if BUILD_WIN32 + + SYS_LIBS = -lws2_32 -lshell32 -ladvapi32 +@@ -239,5 +241,5 @@ doxygen: FORCE + doxygen $(srcdir)/Doxyfile + FORCE: + +-DISTCLEANFILES = *~ libevent.pc ./include/event2/event-config.h ++DISTCLEANFILES += *~ libevent.pc ./include/event2/event-config.h + +diff --git a/test/Makefile.am b/test/Makefile.am +index b10c41a..ea468b3 100644 +--- a/test/Makefile.am ++++ b/test/Makefile.am +@@ -5,6 +5,7 @@ + # See LICENSE for copying information. + + AUTOMAKE_OPTIONS = foreign ++DISTCLEANFILES = + + AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/compat -I$(top_srcdir)/include -I../include -DTINYTEST_LOCAL + +@@ -19,7 +20,14 @@ endif + EXTRA_PROGRAMS = regress + noinst_HEADERS = tinytest.h tinytest_macros.h regress.h tinytest_local.h + +-TESTS = $(top_srcdir)/test/test.sh ++# We need to copy this file, since automake doesn't want us to use top_srcdir ++# in TESTS. ++TESTS = test/test-script.sh ++ ++test/test-script.sh: test/test.sh ++ cp $< $@ ++ ++DISTCLEANFILES += test/test-script.sh + + BUILT_SOURCES = + if BUILD_REGRESS +-- +2.1.1 + diff --git a/package/libevent/libevent-disable-building-test-programs.patch b/package/libevent/libevent-disable-building-test-programs.patch deleted file mode 100644 index 4b0f0d9..0000000 --- a/package/libevent/libevent-disable-building-test-programs.patch +++ /dev/null @@ -1,30 +0,0 @@ -From e932c8864e1bb8b6a7901d4b049a1100c4becba5 Mon Sep 17 00:00:00 2001 -From: Gilles Talis <gilles.talis@gmail.com> -Date: Fri, 21 Jun 2013 15:25:11 -0700 -Subject: [PATCH] Disable building test programs - -We are not really interested in building test programs. -Moreover, these programs use fork() function that is -not available on MMU-less architectures. - -Signed-off-by: Gilles Talis <gilles.talis@gmail.com> ---- - Makefile.in | 2 +- - 1 files changed, 1 insertions(+), 1 deletions(-) - -diff --git a/Makefile.in b/Makefile.in -index 2ebefa2..4fba1ff 100644 ---- a/Makefile.in -+++ b/Makefile.in -@@ -487,7 +487,7 @@ LIBEVENT_LIBS_LA = libevent.la libevent_core.la libevent_extra.la \ - @INSTALL_LIBEVENT_TRUE@lib_LTLIBRARIES = $(LIBEVENT_LIBS_LA) - @INSTALL_LIBEVENT_TRUE@pkgconfig_DATA = $(LIBEVENT_PKGCONFIG) - @INSTALL_LIBEVENT_FALSE@noinst_LTLIBRARIES = $(LIBEVENT_LIBS_LA) --SUBDIRS = . include sample test -+SUBDIRS = . include sample - @BUILD_WIN32_FALSE@SYS_LIBS = - @BUILD_WIN32_TRUE@SYS_LIBS = -lws2_32 -lshell32 -ladvapi32 - @BUILD_WIN32_FALSE@SYS_SRC = $(am__append_5) $(am__append_6) \ --- -1.7.4.1 - diff --git a/package/libevent/libevent.mk b/package/libevent/libevent.mk index 73be502..fbc7c54 100644 --- a/package/libevent/libevent.mk +++ b/package/libevent/libevent.mk @@ -5,11 +5,11 @@ ################################################################################ LIBEVENT_VERSION = 2.0.21 -LIBEVENT_SOURCE = libevent-$(LIBEVENT_VERSION)-stable.tar.gz -LIBEVENT_SITE = https://github.com/downloads/libevent/libevent +LIBEVENT_SITE = $(call github,libevent,libevent,release-$(LIBEVENT_VERSION)-stable) LIBEVENT_INSTALL_STAGING = YES LIBEVENT_LICENSE = BSD-3c, OpenBSD LIBEVENT_LICENSE_FILES = LICENSE +LIBEVENT_AUTORECONF = YES define LIBEVENT_REMOVE_PYSCRIPT rm $(TARGET_DIR)/usr/bin/event_rpcgen.py