[Rpm-maint] [rpm-software-management/rpm] RFC Merge RPM 4.13 OS/2 code changes (#260)

[Panu Matilainen]

An OS that is niche to the point that I haven't even heard of it is hardly more justifiable than a dead one. And it's not just a niche OS or a dead OS but it's also a non-POSIX, decidedly non-unix OS, so it's quite a leap we're talking about here.

And we've just been considering bumping the system requirement to POSIX.1-2008 (from 2001) to make certain things simpler/possible.

That said, there are a number of things in this patch that could be merged if done right. Some examples:
* The IMA plugin should not be built at all if <sys/xattr.h> and lxsetattr() are not present on the system. That check belongs to configure.ac + Makefiles and has nothing at all to do with OS/2, it's plain correctness.
* Similarly for ctermid(), test for the existence of the function, not OS.
* build/pack.c already includes <sys/wait.h>. If the order is wrong then change that instead of yet more ifdefs
* __python_path hardcoding /usr/lib is wrong anyway, but so is %{prefix} because rpm's prefix and python's prefix don't necessarily have anything to do with each other, it should discover the python path by some other means
* The added error check in doScript() for child == -1 would be correct for fork() too. I also wonder why is spawnvp() used there if everything else seems to be happy with fork()?
* Line- and path-separator issues could be handled centrally (configure.ac / system.h macro or such) without making it any worse for anybody

Quite possibly something else too, I didn't look *that* carefully. In general, the way to add portability is not to sprinkle ifdef's but add abstraction.

tools/os2deps.c is missing in that patch so no idea what it does, but the invocation through a script helper looks a bit strange, and this magic pattern looks overly broad, I'd expect there to be some further identification in that string:
+%__os2_magic           ^(32|64)-bit.*$

In any case it looks like that's dealing with os2 *executables* so a better name would be something less broad than "os2".

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/260#issuecomment-339914006
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20171027/83a8f61a/attachment.html>