Re: [pve-devel] [PATCH librados2-perl] Convert to dpkg-buildpackage

2018-04-19 Thread Fabian Grünbichler
On Thu, Apr 19, 2018 at 11:31:08AM +0200, Rene Jochum wrote:
> Thanks again, comments inline.
> 
> On 2018-04-19 10:16, Fabian Grünbichler wrote:
> >> @@ -52,23 +54,27 @@ install: PVE/RADOS.pm RADOS.so
> >>  .PHONY: deb ${DEB}
> >>  deb: ${DEB}
> >>  ${DEB}:
> >> -  rm -rf debian
> >> -  mkdir debian
> >> -  make DESTDIR=${CURDIR}/debian install
> >> -  install -d -m 0755 debian/DEBIAN
> >> -  sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
> >> s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g"  >> >debian/DEBIAN/control
> >> -  install -D -m 0644 copyright debian/${DOCDIR}/copyright
> >> -  install -m 0644 changelog.Debian debian/${DOCDIR}/
> >> -  gzip -9 -n debian/${DOCDIR}/changelog.Debian
> >> -  echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
> >> checkout ${GITVERSION}" > debian/${DOCDIR}/SOURCE
> >> -  fakeroot dpkg-deb --build debian
> >> -  mv debian.deb ${DEB}
> >> -  rm -rf debian
> >> +  rm -rf ${BUILDDIR}
> >> +  rsync -a * ${BUILDDIR}
> >> +
> >> +  cp -fpr debian ${BUILDDIR}
> > why rsync + cp? one should be enough ;)
> 
> Hihi, ofc.
> 
> > 
> >> +  cp changelog.Debian ${BUILDDIR}/debian/changelog
> >> +  cp copyright ${BUILDDIR}/debian/copyright
> > see above, those two could go into the checked-in debian/
> > 
> >> +  sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
> >> s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" 
> >> ${BUILDDIR}/debian/control
> > except for the PERLAPI one, all of these should be dropped / replaced:
> > VERSION and PKGRELEASE come from debian/changelog
> > ARCH comes via control and dpkg-architecture
> > 
> > whether the PERLAPI is really needed should probably be investigated.
> 
> It is needed, as "RADOS.so" only works with that perl Version - on of
> the main reasons I have to recompile on Buster.

I meant whether we really need to auto-generate it via a variable.

modifying debian/control once per major release is probably not an
issue, and we can skip generating debian/control then (it also serves as
a reminder to change for ani API-related breakage ;)).

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH librados2-perl] Convert to dpkg-buildpackage

2018-04-19 Thread Fabian Grünbichler
On Thu, Apr 19, 2018 at 11:02:44AM +0200, Rene Jochum wrote:
> On 2018-04-19 10:16, Fabian Grünbichler wrote:
> > some comments inline (your patch also only applies when ignoring
> > whitespace changes!)
> 
> 
> Hi Fabian,
> 
> thank you for looking into it! Can you give me a tip on making patches
> that apply without ignoring white space changes?

I think your editor or MUA modifies lines only containing whitespace.

e.g., if I diff the patch as you sent it, and the patch exported again
after applying it with '--ignore-whitespace', I see lots of lines where
your original patch has /^$/ (a completely empty line), and the
re-exported one has /^ $/ (a line containing a single space).

the former is not valid patch syntax, because the first character
indicates added (+), removed (-) or context ( ), the latter means an
otherwise empty context line.

> Will implement all the given recommendations now.

great :)

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] [PATCH librados2-perl] Convert to dpkg-buildpackage

2018-04-19 Thread Rene Jochum
Thanks again, comments inline.

On 2018-04-19 10:16, Fabian Grünbichler wrote:
>> @@ -52,23 +54,27 @@ install: PVE/RADOS.pm RADOS.so
>>  .PHONY: deb ${DEB}
>>  deb: ${DEB}
>>  ${DEB}:
>> -rm -rf debian
>> -mkdir debian
>> -make DESTDIR=${CURDIR}/debian install
>> -install -d -m 0755 debian/DEBIAN
>> -sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
>> s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" > >debian/DEBIAN/control
>> -install -D -m 0644 copyright debian/${DOCDIR}/copyright
>> -install -m 0644 changelog.Debian debian/${DOCDIR}/
>> -gzip -9 -n debian/${DOCDIR}/changelog.Debian
>> -echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
>> checkout ${GITVERSION}" > debian/${DOCDIR}/SOURCE
>> -fakeroot dpkg-deb --build debian
>> -mv debian.deb ${DEB}
>> -rm -rf debian
>> +rm -rf ${BUILDDIR}
>> +rsync -a * ${BUILDDIR}
>> +
>> +cp -fpr debian ${BUILDDIR}
> why rsync + cp? one should be enough ;)

Hihi, ofc.

> 
>> +cp changelog.Debian ${BUILDDIR}/debian/changelog
>> +cp copyright ${BUILDDIR}/debian/copyright
> see above, those two could go into the checked-in debian/
> 
>> +sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
>> s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" 
>> ${BUILDDIR}/debian/control
> except for the PERLAPI one, all of these should be dropped / replaced:
> VERSION and PKGRELEASE come from debian/changelog
> ARCH comes via control and dpkg-architecture
> 
> whether the PERLAPI is really needed should probably be investigated.

It is needed, as "RADOS.so" only works with that perl Version - on of
the main reasons I have to recompile on Buster.

> 
>> +make DESTDIR=${BUILDDIR} install
>> +install -D -m 0644 copyright ${BUILDDIR}/${DOCDIR}/copyright
>> +install -m 0644 changelog.Debian ${BUILDDIR}/${DOCDIR}/
>> +gzip -9 -n ${BUILDDIR}/${DOCDIR}/changelog.Debian
> these four should not be needed (dpkg-buildpackage will call the
> Makefile to compile/build anyway, and sets DESTDIR correctly)
> 
> copyright and changelog are handled by the appropriate debhelpers
> 
>> +echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
>> checkout ${GITVERSION}" > ${BUILDDIR}/${DOCDIR}/SOURCE
> this does not work as intended (debdiff says the SOURCE file is not
> contained after applying the patch)

Fixed, in V2




-- 
René Jochum
Mail: r...@jochums.at
Tel: +43 664 750 77 653
Web: https://rene.jochums.at
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH librados2-perl] Convert to dpkg-buildpackage

2018-04-19 Thread Rene Jochum
On 2018-04-19 10:16, Fabian Grünbichler wrote:
> some comments inline (your patch also only applies when ignoring
> whitespace changes!)


Hi Fabian,

thank you for looking into it! Can you give me a tip on making patches
that apply without ignoring white space changes?

Will implement all the given recommendations now.

Thanks,
René


___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH librados2-perl] Convert to dpkg-buildpackage

2018-04-19 Thread Fabian Grünbichler
thanks for this! one of the leftover packages which we haven't converted
so far (for lack of recent changes). now that Alwin did some changes and
is planning to do some more, it's probably a good idea to convert it.

some comments inline (your patch also only applies when ignoring
whitespace changes!)

I already tested some of my recommendations to make sure they work, so
maybe you want to use the following on top of your patch as the base of
a v2:

---8<---
From afc0bb4ebf13d90d8bee5a0da4633c5df59dfc3c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= 
Date: Thu, 19 Apr 2018 10:04:48 +0200
Subject: [PATCH] WIP
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fabian Grünbichler 
---
 Makefile | 17 +
 debian/control.in|  2 +-
 changelog.Debian => debian/changelog |  0
 copyright => debian/copyright|  0
 debian/librados2-perl.docs   |  1 +
 debian/rules |  3 ++-
 6 files changed, 9 insertions(+), 14 deletions(-)
 rename changelog.Debian => debian/changelog (100%)
 rename copyright => debian/copyright (100%)
 create mode 100644 debian/librados2-perl.docs

diff --git a/Makefile b/Makefile
index a99627d..13add44 100644
--- a/Makefile
+++ b/Makefile
@@ -55,18 +55,11 @@ install: PVE/RADOS.pm RADOS.so
 deb: ${DEB}
 ${DEB}:
rm -rf ${BUILDDIR}
-   rsync -a * ${BUILDDIR}
-
-   cp -fpr debian ${BUILDDIR}
-   cp changelog.Debian ${BUILDDIR}/debian/changelog
-   cp copyright ${BUILDDIR}/debian/copyright
-   sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" 
${BUILDDIR}/debian/control
-
-   make DESTDIR=${BUILDDIR} install
-   install -D -m 0644 copyright ${BUILDDIR}/${DOCDIR}/copyright
-   install -m 0644 changelog.Debian ${BUILDDIR}/${DOCDIR}/
-   gzip -9 -n ${BUILDDIR}/${DOCDIR}/changelog.Debian
-   echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
checkout ${GITVERSION}" > ${BUILDDIR}/${DOCDIR}/SOURCE
+   rsync -ra * ${BUILDDIR}
+
+   sed -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" ${BUILDDIR}/debian/control
+
+   echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
checkout ${GITVERSION}" > ${BUILDDIR}/SOURCE
 
cd ${BUILDDIR}; dpkg-buildpackage -b -uc -us
rm -rf build
diff --git a/debian/control.in b/debian/control.in
index cb29f38..f5fdc8e 100644
--- a/debian/control.in
+++ b/debian/control.in
@@ -5,7 +5,7 @@ Priority: optional
 Standards-Version: @@VERSION@@-@@PKGRELEASE@@
 
 Package: librados2-perl
-Architecture: @@ARCH@@
+Architecture: any
 Depends: libc6 (>= 2.2.5), perl (>= 5.20.1-5), @PERLAPI@, librados2 (>= 
0.67.5), libpve-access-control
 Description: Perl bindings for librados
  This package contains librados perl binding used by Proxmox VE.
diff --git a/changelog.Debian b/debian/changelog
similarity index 100%
rename from changelog.Debian
rename to debian/changelog
diff --git a/copyright b/debian/copyright
similarity index 100%
rename from copyright
rename to debian/copyright
diff --git a/debian/librados2-perl.docs b/debian/librados2-perl.docs
new file mode 100644
index 000..11b531f
--- /dev/null
+++ b/debian/librados2-perl.docs
@@ -0,0 +1 @@
+SOURCE
diff --git a/debian/rules b/debian/rules
index 0983a2d..a4652c2 100755
--- a/debian/rules
+++ b/debian/rules
@@ -7,4 +7,5 @@ DH_VERBOSE = 1
 %:
dh $@
 
-override_dh_auto_build:
\ No newline at end of file
+override_dh_auto_build:
+   make RADOS.so
-- 
2.14.2

--->8---

On Wed, Apr 18, 2018 at 10:27:00PM +0200, René Jochum wrote:
> Also fixes a minor spelling error in changelog.Debian and extends the
> .gitignore to ignore build files.
> 
> I've made this so i get a .changes file which i need for my dput usage.
> ---
>  .gitignore  |  7 +++
>  Makefile| 36 +++--
>  changelog.Debian|  2 +-

I'd move this into debian as well ;)

>  debian/compat   |  1 +
>  control.in => debian/control.in |  8 +---
>  debian/rules| 10 +
>  6 files changed, 45 insertions(+), 19 deletions(-)
>  create mode 100644 .gitignore
>  create mode 100644 debian/compat
>  rename control.in => debian/control.in (83%)
>  create mode 100755 debian/rules
> 
> diff --git a/.gitignore b/.gitignore
> new file mode 100644
> index 000..70af628
> --- /dev/null
> +++ b/.gitignore
> @@ -0,0 +1,7 @@
> +RADOS.c
> +RADOS.so

these should only be generated in build/ , and thus not need to be
ignored separately? doesn't really hurt though.

> +build/
> +
> +*.buildinfo
> +*.changes
> +*.deb
> diff --git a/Makefile b/Makefile
> index 2c1451f..a99627d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -14,6 +14,8 @@ 

[pve-devel] [PATCH librados2-perl] Convert to dpkg-buildpackage

2018-04-18 Thread René Jochum
Also fixes a minor spelling error in changelog.Debian and extends the
.gitignore to ignore build files.

I've made this so i get a .changes file which i need for my dput usage.
---
 .gitignore  |  7 +++
 Makefile| 36 +++--
 changelog.Debian|  2 +-
 debian/compat   |  1 +
 control.in => debian/control.in |  8 +---
 debian/rules| 10 +
 6 files changed, 45 insertions(+), 19 deletions(-)
 create mode 100644 .gitignore
 create mode 100644 debian/compat
 rename control.in => debian/control.in (83%)
 create mode 100755 debian/rules

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..70af628
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,7 @@
+RADOS.c
+RADOS.so
+build/
+
+*.buildinfo
+*.changes
+*.deb
diff --git a/Makefile b/Makefile
index 2c1451f..a99627d 100644
--- a/Makefile
+++ b/Makefile
@@ -14,6 +14,8 @@ DOCDIR=${PREFIX}/share/doc/${PACKAGE}
 MAN1DIR=${MANDIR}/man1/
 PERLDIR=${PREFIX}/share/perl5

+BUILDDIR=build
+
 PERL_ARCHLIB := `perl -MConfig -e 'print $$Config{archlib};'`
 PERL_INSTALLVENDORARCH := `perl -MConfig -e 'print 
$$Config{installvendorarch};'`
 PERL_APIVER := `perl -MConfig -e 'print 
$$Config{debian_abi}//$$Config{version};'`
@@ -26,7 +28,7 @@ CFLAGS= -shared -fPIC -O2 -Werror -Wtype-limits -Wall 
-Wl,-z,relro \
 PERLSODIR=$(PERL_INSTALLVENDORARCH)/auto

 ARCH:=$(shell dpkg-architecture -qDEB_BUILD_ARCH)
-GITVERSION:=$(shell cat .git/refs/heads/master)
+GITVERSION:=$(shell git rev-parse HEAD)

 DEB=${PACKAGE}_${VERSION}-${PKGREL}_${ARCH}.deb

@@ -52,23 +54,27 @@ install: PVE/RADOS.pm RADOS.so
 .PHONY: deb ${DEB}
 deb: ${DEB}
 ${DEB}:
-   rm -rf debian
-   mkdir debian
-   make DESTDIR=${CURDIR}/debian install
-   install -d -m 0755 debian/DEBIAN
-   sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" debian/DEBIAN/control
-   install -D -m 0644 copyright debian/${DOCDIR}/copyright
-   install -m 0644 changelog.Debian debian/${DOCDIR}/
-   gzip -9 -n debian/${DOCDIR}/changelog.Debian
-   echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
checkout ${GITVERSION}" > debian/${DOCDIR}/SOURCE
-   fakeroot dpkg-deb --build debian
-   mv debian.deb ${DEB}
-   rm -rf debian
+   rm -rf ${BUILDDIR}
+   rsync -a * ${BUILDDIR}
+
+   cp -fpr debian ${BUILDDIR}
+   cp changelog.Debian ${BUILDDIR}/debian/changelog
+   cp copyright ${BUILDDIR}/debian/copyright
+   sed -e s/@@VERSION@@/${VERSION}/ -e s/@@PKGRELEASE@@/${PKGREL}/ -e 
s/@@ARCH@@/${ARCH}/ -e "s|@PERLAPI@|perlapi-$(PERL_APIVER)|g" 
${BUILDDIR}/debian/control
+
+   make DESTDIR=${BUILDDIR} install
+   install -D -m 0644 copyright ${BUILDDIR}/${DOCDIR}/copyright
+   install -m 0644 changelog.Debian ${BUILDDIR}/${DOCDIR}/
+   gzip -9 -n ${BUILDDIR}/${DOCDIR}/changelog.Debian
+   echo "git clone git://git.proxmox.com/git/librados2-perl.git\\ngit 
checkout ${GITVERSION}" > ${BUILDDIR}/${DOCDIR}/SOURCE
+
+   cd ${BUILDDIR}; dpkg-buildpackage -b -uc -us
+   rm -rf build
lintian ${DEB}

 .PHONY: clean
-clean:
-   rm -rf debian *.deb ${PACKAGE}-*.tar.gz dist *.1.pod *.1.gz RADOS.so 
RADOS.c
+clean:
+   rm -rf ${BUILDDIR} *.deb ${PACKAGE}-*.tar.gz dist *.1.pod *.1.gz 
RADOS.so RADOS.c
find . -name '*~' -exec rm {} ';'

 .PHONY: distclean
diff --git a/changelog.Debian b/changelog.Debian
index 7f4b898..38084e2 100644
--- a/changelog.Debian
+++ b/changelog.Debian
@@ -1,6 +1,6 @@
 librados2-perl (1.0-5) unstable; urgency=medium

-  * allow to specify the userid with rados_create
+  * allow one to specify the userid with rados_create

   * split method pve_rados_connect

diff --git a/debian/compat b/debian/compat
new file mode 100644
index 000..9a03714
--- /dev/null
+++ b/debian/compat
@@ -0,0 +1 @@
+10
\ No newline at end of file
diff --git a/control.in b/debian/control.in
similarity index 83%
rename from control.in
rename to debian/control.in
index c2dc77e..cb29f38 100644
--- a/control.in
+++ b/debian/control.in
@@ -1,9 +1,11 @@
-Package: librados2-perl
-Version: @@VERSION@@-@@PKGRELEASE@@
+Source: librados2-perl
+Maintainer: Proxmox Support Team 
 Section: perl
 Priority: optional
+Standards-Version: @@VERSION@@-@@PKGRELEASE@@
+
+Package: librados2-perl
 Architecture: @@ARCH@@
 Depends: libc6 (>= 2.2.5), perl (>= 5.20.1-5), @PERLAPI@, librados2 (>= 
0.67.5), libpve-access-control
-Maintainer: Proxmox Support Team 
 Description: Perl bindings for librados
  This package contains librados perl binding used by Proxmox VE.
diff --git a/debian/rules b/debian/rules
new file mode 100755
index 000..0983a2d
--- /dev/null
+++ b/debian/rules
@@ -0,0 +1,10 @@
+#!/usr/bin/make -f
+# See debhelper(7) (uncomment to enable)
+# output every command that