search

iguanaworks / iguanair

Iguanaworks USB IR Project: firmware and software
http://www.iguanaworks.net
23 stars 11 forks source link

The debian packaging is not acceptable into the distro. #31

Closed leamas closed 7 years ago

leamas commented 7 years ago

The debian packaging under the packaging dirrectory is not acceptable into the distro. Some points:

Attaching a modernized debian packaging. Basically, I don't see the point updating the upstream sources with this, it would be better to mantain it as a separate package for SID/Buster with the goal to have it accepted (which should be perfectly possible with this package).

debian.zip

EDIT: This packaging is based on d1a3474, the current tip in the git repo. The patch series is a snapshot of the pull request.

EDIT2: Here also the issue that the directory containing the source tarballs at iguanaworks.net cannot browsed. This blocks distribution source scanners which looks for new releases.

IguanaBen commented 7 years ago

Thanks for this. I'm hoping we can get these patches ready to be merged into our main repo. I've been playing with the debian package you posted and I have a couple questions:

More generally, how do we handle python2 vs python3? Ideally, I'd like to require a python, but not care what version. But at the same time, when sending the source package to a ppa to make the binaries, I'd like it to have both python2 and python3 and make libraries for both so people installing binaries have the requirement of only needing one python (2 or 3). Does that make sense and can that be done? Or should we just require python2-dev and python3-dev in source and make a python2 and python3 binary where either can be installed?

It would be great to get this package in a proper debian/ubuntu format and be able to be accepted into SID. Until that happens, I'm hoping to move our debian/ubuntu repository over to ppa at https://launchpad.net/~iguanaworks/+archive/ubuntu/iguanair

leamas commented 7 years ago

Hi!

Basically, what I could and intend to do is to package this for Debian - this would make it part of sid//buster so users of this this and newer releases could install it without using "your" repo - it would become part of core Debian.

Besides obvious user advantages, being part of Debian means being part of their reviews and build machinery which normally flags at least build errors before new releases. They build on 18 platforms (!). It also connects you to the Debian/Ubuntu bug reporting flow.

As for your questions, the answer depends on what the target release is. I presume that you will take these patches and update your repo for jessie, and here is what I think about this:

IMHO, moving everything to python3 now is the correct way to avoid different codebases unless it creates problems (I cannot really test reflashing etc.)

Cheers!

--a

EDIT: Please review the copyright file! Patching upstream's copyrights is really nothing I should do, but I did it... DS

EDIT 2: I don't think there are jessie users without python3 installed out there. If there are, they will most likely need python3 real soon anyway. So, I don't see the point keeping the python2 dependencies in the packaging (that is not to say the same for the upstream sources, of course).

jdunn14 commented 7 years ago

Our code should work with Python2 or Python3 so just supporting Python3 wouldn't cause any problems for us. In fact cmake is already set up to auto-detect whichever is available and point swig at the correct one or, alternatively, a command line argument can force it to use one or the other.

I do want to make sure that customers who use our stuff aren't forced to use Python3 if their platform also supports Python2. If they're using our Python API with some other library that doesn't support Python3 then their life is harder because of that conflict. The fault there is arguably with the library that doesn't support Python3, but any dissatisfied customer or lost sale is our problem. That get's less important as more things move from 2 to 3, but we still have new customers using Ubuntu 12.04 so there's a long tail on those use cases.

All that said, go ahead and make patches assuming just Python3 going forward and we'll keep supporting Python2 in cmake. That way we can at least always point customers to building from source if they need Python2, and maybe we can tweak the debian package sources to work w either.

-Joseph

leamas commented 7 years ago

This might not be complete, but most if not all things are already patced to use python3.

Of course you should support both python2 and python3 in cmake. My point is that debian jessie users could be assumed to have python3 available - that's the packaging perspective.

jdunn14 commented 7 years ago

I think this is just a small difference in perspective for an open source project vs a product, and actively developed stuff is not the problem. It's the semi-abandoned code that people might still try to use. For an open-source project a reasonable move is to tell people to update, or let them find others in the community to figure it out. As a (tiny) company making and supporting a product iguanaworks tries to be a little more forgiving of such things as long as it doesn't cost us too many resources.

I've had the joy of trying to use two libraries and finding out their dependencies are incompatible so I'll do what I can to not be the guy cursed out for causing that =).

I didn't think you were suggesting we drop Python2 from the cmake layer. We'll absolutely support it there for the foreseeable future and if you provide pull requests for Python3 based packages we'll merge those. If I find a way to support Python2 at a tar/desc or srpm layer I'll let you know, but it's not a high priority.

leamas commented 7 years ago

As of now, this is python3-compatible as AFAICT. The large chunk is swig, which handles python3 as required. I have made on or two patches to make the scripts usable in python3 - it does not break the -2 compatiblilty.

IguanaBen commented 7 years ago

I agree. Let's move forward with debian packaging in a python3 world. As long as the source still works with python2, that should be fine. If we hear lots of people wanting python2 and a .deb package, we can re-evaluate then.

@leamas would you be willing to put these changes into a pull request? Thanks!

IguanaBen commented 7 years ago

Can you help me build the debian package with this patch? I updated git to the latest. I applied your debian.zip and removed all the patches as I believe they have already been applied to our git repo. I changed the changelog to 1.1.9-2 for testing. Then in the usb_ir/packaging directory, I ran

make tarball

and renamed that file to what debian expects and but it in the parent directory. I then ran

make debian

and I get these errors

make[2]: Entering directory '/data/Iguana/gitpublic/software/usb_ir'
dh_auto_clean
rm -rf software/usb_ir/build
rm -f software/usb_ir/config.h
test -d software/lirc-drv-iguanair && \
    make -C software/lirc-drv-iguanair clean || :
make[2]: Leaving directory '/data/Iguana/gitpublic/software/usb_ir'
   dh_clean
make[1]: Leaving directory '/data/Iguana/gitpublic/software/usb_ir'
 dpkg-source -I.svn -Ipackaging/iguanaIR-1.1.9-2* -Iwin32 -I*.deb -I*.rpm -b usb_ir
dpkg-source: info: using source format `3.0 (quilt)'
dpkg-source: info: building iguanair using existing ./iguanair_1.1.9.orig.tar.bz2
dpkg-source: error: cannot represent change to ChangeLog:
dpkg-source: error:   new version is symlink to packaging/debian/changelog
dpkg-source: error:   old version is plain file
dpkg-source: error: cannot represent change to runCmake:
dpkg-source: error:   new version is symlink to bootstrap/runCmake
dpkg-source: error:   old version is plain file
dpkg-source: error: cannot represent change to files/python/usr/bin/iguanaIR-reflasher:
dpkg-source: error:   new version is symlink to ../share/iguanaIR-reflasher/iguanaIR-reflasher
dpkg-source: error:   old version is plain file
dpkg-source: error: cannot represent change to files/python/usr/share/iguanaIR-reflasher/hex/loader-0.hex:
dpkg-source: error:   new version is symlink to ../../../../../../../../firmware/usb_ir_loader/usb_ir_loader.hex
dpkg-source: error:   old version is plain file
dpkg-source: error: cannot represent change to files/python/usr/share/iguanaIR-reflasher/hex/body-0.hex:
dpkg-source: error:   new version is symlink to ../../../../../../../../firmware/usb_ir_body/usb_ir_body.hex
dpkg-source: error:   old version is plain file
dpkg-source: error: cannot represent change to packaging/iguanaIR-1.1.9-2.tar.bz2: binary file contents changed
dpkg-source: error: add packaging/iguanaIR-1.1.9-2.tar.bz2 in debian/source/include-binaries if you want to store the modified binary in the debian tarball
dpkg-source: error: unrepresentable changes to source
dpkg-buildpackage: error: dpkg-source -I.svn -Ipackaging/iguanaIR-1.1.9-2* -Iwin32 -I*.deb -I*.rpm -b usb_ir gave error exit status 2
Makefile:38: recipe for target 'ubuntu' failed
make: *** [ubuntu] Error 2

Seems it doesn't like symlinks for some of the files.

leamas commented 7 years ago

Hi!

Basically, you make the packagin hard for two reasons:

I attach a revised debian directory which builds on stretch using a workflow like:

$ cd iguanair
$ mkdir ..//debian-build
$ TARNAME=iguanair_0.1.49d7d77.orig ./make-dist.sh
$ cp dist/iguanair_0.1.49d7d77.orig.tar.gz ../debian-build
$ cd ../debian-build
$ unzip path-to-debian-zip-archive
$ cp -ar debian  iguanair_0.1.49d7d77.orig
$ cd iguanair_0.1.49d7d77.orig
$ debuild  -us -uc

I note that you have dropped the tmpfiles.d/iguanair.conf file from the sources. Is this on purpose?

debian.zip

IguanaBen commented 7 years ago

I'm fine with separating the packaging from the main source, I just need to learn how to build the packages in the new system. (And manually setting the git commint in the version). I tried following your workflow from above. I think I did it right. Just to confirm, the iguanair_0.1.49d7d77.orig directory has only the debian directory in it and nothing else, right? IE, the tar.gz file hasn't be untarred before running debuild. If that is right, when I run debuild, I get:

 dpkg-buildpackage -rfakeroot -D -us -uc
dpkg-buildpackage: source package iguanair
dpkg-buildpackage: source version 0.1.49d7d77-1
dpkg-buildpackage: source distribution unstable
dpkg-buildpackage: source changed by Alec Leamas <leamas.alec@gmail.com>
 dpkg-source --before-build iguanair_0.1.49d7d77.orig
dpkg-buildpackage: host architecture amd64
dpkg-source: info: applying 0001-Add-tmpfiles.d-support.patch
 fakeroot debian/rules clean
make: Warning: File 'debian/rules' has modification time 18319 s in the future
dh clean --with systemd 
   dh_testdir
   debian/rules override_dh_auto_clean
make[1]: Entering directory '/data/Iguana/debian-build/iguanair_0.1.49d7d77.orig'
make[1]: Warning: File 'debian/rules' has modification time 18316 s in the future
dh_auto_clean
rm -rf software/usb_ir/build software/usb_ir/config.h
cd software/lirc-drv-iguanair; make clean
/bin/sh: 1: cd: can't cd to software/lirc-drv-iguanair
make[2]: Entering directory '/data/Iguana/debian-build/iguanair_0.1.49d7d77.orig'
make[2]: *** No rule to make target 'clean'.  Stop.
make[2]: Leaving directory '/data/Iguana/debian-build/iguanair_0.1.49d7d77.orig'
debian/rules:18: recipe for target 'override_dh_auto_clean' failed
make[1]: *** [override_dh_auto_clean] Error 2
make[1]: Leaving directory '/data/Iguana/debian-build/iguanair_0.1.49d7d77.orig'
debian/rules:15: recipe for target 'clean' failed
make: *** [clean] Error 2
dpkg-buildpackage: error: fakeroot debian/rules clean gave error exit status 2
debuild: fatal error at line 1376:
dpkg-buildpackage -rfakeroot -D -us -uc failed

It looks like the debuild script made the software directory, but only the usb_ir under software, not the lirc-drv-iguanair.

Thanks,

jdunn14 commented 7 years ago

I was able to build using the directions and zip from leamas. Ben and I will look through this and see how we want to merge it once the next release is close to complete. Thanks.

leamas commented 7 years ago

Hm... I was a bit tired yesterday. Missed that the zip file contains lot's of junk such as the directories .debhelper, iguanair, libiguanair0. libiguanair-dev, lirc-drv-iguanair and tmp. Please kill this garbage.

And I also missed the 'tar -xf iguanair_0.1.49d7d77.orig', but you already figured this out.

Also, perhaps the tone in what I wrote was less friendly than I intended.. TBH, I'm not quite sure - after all, I'm just a dumb Swede trying to write English.

leamas commented 7 years ago

Attaching a cleaned up packaging.

One open issue here is how to handle the sysV init files. Debian basically wants them available at least in Jessie, but I leave it uninstalled as-is. You might want to install them either as a regular sysV setup in /etc/init or just make them available in e. g. /usr/share/iguanair

If possible, I would like this patch to be applied: 

--- ./files/python/usr/share/iguanaIR-reflasher/iguanaIR-reflasher  (original)
+++ ./files/python/usr/share/iguanaIR-reflasher/iguanaIR-reflasher  (refactored)
@@ -518,7 +518,7 @@
     pages = readPages(open(filename, 'r'))

     if blank_pages:
-        blanks = range(128)
+        blanks = list(range(128))
         for page in pages:
             blanks.remove(page['start'] // 64)

This makes the script python3-acceptable; should still wokr on python2.

Also, if possible, please change the shebang from /usr/bin/python2.7 to /usr/bin/env python -tt - it's probably a good idea anyway.

Stull curious about the dropped /usr/lib/tmpfiles.d/iguanair.conf - was this on purpose?

--alec

debian.zip

jdunn14 commented 7 years ago

No problem with the tone. I could only fault you if I spoke any Swedish... or honestly was fluent in any language other than English.

Thanks for the cleaned up zip, and the fact that range() isn't really a list in Python3 hadn't crossed my mind. Happy to apply the patch. By the way, "/usr/bin/env python -tt" doesn't work. Fedora does not split the arguments to env so it complains that it can't find 'python -tt' in the path. I'm going to drop the pretty much useless '-tt' as it's the default behavior in Python3 anyway.

I'll worry about the sysV init files once I close out a stack of tickets and test building on a handful of platforms, so hopefully next week-ish.

As for /usr/lib/tmpfiles.d/iguanair.conf, I certainly didn't drop it on purpose, and in fact a git log on usb_ir/files/systemd/lib/tmpfiles.d/iguanair.conf says it was added by one of your patches and is still there. Maybe we're just not installing it or it's in the wrong place in the source tree?

leamas commented 7 years ago

OK, thanks

Maybe we're just not installing it or it's in the wrong place in the source tree?

I leave this in your hands :)

jdunn14 commented 7 years ago

I'm confused and I'm sorry if I'm being dense here, but what did you mean by dropped? It is in the build I made last night:

jdunn@debian:~/debian-build$ dpkg-deb -c iguanair_0.1.49d7d77-1_amd64.deb | grep conf -rw-r--r-- root/root 49 2017-02-04 19:58 ./usr/lib/tmpfiles.d/iguanair.conf

It wasn't dropped, it's still there. Is that the wrong place? It's where your pull request installed it. What needs to be changed?

leamas commented 7 years ago

Seems to be some packaging error on my side... stay tuned.

leamas commented 7 years ago

Too long delay, I apologize... anyway, whatever error here was, it was on my side. That is, the file is present where it should be, and my patch bogus.

Sorry for the noise.

IguanaBen commented 7 years ago

Please take a look at 5145e5 in branch debian_packaging. I believe that incorporates most (all?) of your patches and also separates out the python packages.

IguanaBen commented 7 years ago

Actually, just look at latest in master as we merged the changes.

leamas commented 7 years ago

Sorry for new delay, nordic summer holidays this time.

At a glance, all looks OK in the sense that it should be possible to create a debian package which goes into the distribution. That said. having the packaging in the master might cause problems, but it does not affect those efforts.

leamas commented 7 years ago

After all, all the issues I raised seems to be fixed in current master. Closing. Thanks !