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/
Attachment:
signature.asc
Description: PGP signature