Control: tags -1 - moreinfo On Saturday, July 02 2016, Gianfranco Costamagna wrote:
>>I'd like to get a few changes I've made to newlisp uploaded. They > >>basically fix two bugs: 828805 and 828806. >> >>The changes are: >> >>- Support GNU/kFreeBSD builds (by creating the necessary makefiles and >> adjusting source files accordingly), and >> >>- Do not use -m32/-m64 when building. >> >>I have also updated the Vcs-* links in order to reflect the use of >>collab-maint instead of my personal git server. >> >>I'm Cc'ing Andrey Rahmatullin on this message because he is the DD who >>sponsored the package first, so I believe I should give him "precedence" >>(also because I'd like to get DM rights on newlisp, so it's easier if I >>work with just one person). > > > sure, I won't upload it, unless Andrey asks me. > > I have just a few notes, from a quick review: > > 1) + libncurses5-dev > > > why? Hm, this is actually only needed for GNU/kFreeBSD, which uses -lncurses on the linking phase. I updated Build-Depends to reflect that. > please explain the additional build dependency in changelog! Done. > 2) > did you remove the -m32 and -m64 with some special sed command? Yeah, it was just a simple sed command to remove things from all Makefiles. But it seems I did not specify the right set of files to apply the substitutions. > I ask, because you also patched some binaries in the source tree, and > I'm mostly sure this isn't what you have to do: > +diff --git a/qa-specific-tests/ffitest.dylib > b/qa-specific-tests/ffitest.dylib > +index 3017a91..4e0eb2e 100755 > +--- a/qa-specific-tests/ffitest.dylib > ++++ b/qa-specific-tests/ffitest.dylib > > > this, IIRC is some OSX special file, so I guess you better remove that file > from the patch, since it is introducing really useless stuff. Totally right, and this is actually a Windows binary used for testing. > also, you have an ~1k LOC patch, where probably you would just need to patch > two or three places > (but if you got this patch upstream accepted I would leave it as-is) > > Otherwise I would avoid patching places such as > +- (compile-recover "gcc -m32 ../util/ffitest.c -shared -o > ffitest.dylib") > +- (compile-recover "gcc -m64 ../util/ffitest.c -shared -o > ffitest.dylib")) > > > makefile_sunos* > makefile_opensolaris* > makefile_netbsd* > > and so on > > As a personal opinion, I would patch all of them only after getting them > accepted > upstream, and in case they don't care about this, just patch the minimum set > of files/makefiles > used in Debian/Linux/kFreeBSD builds. Right, I only patched the GNU/{Linux,kFreeBSD} files now. Thanks for the heads up. > Otherwise a 1k lines patch will be probably a nightmare to maintain/rebase on > new releases. On the one hand, I see your point in maintaining a large patch on Debian and rebasing it on every new release. On the other hand, this patch only touches the build system, which is unlikely to change much in the near future. Also, I am in touch with upstream and will propose all of my local patches to them, so hopefully I won't need to carry anything else for the next releases. I've uploaded a new version of the package to mentors.d.n. If you could take a look, I'd appreciate! Cheers, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/
signature.asc
Description: PGP signature