mc on IBM i PASE building 4.8.20 change

[jax]

To build 4.8.20 on IBM i 7.3 PASE environment required the following change:

On lines 103-104 of lib/vfs/vfs.h:

// typedef struct timespec mc_timesbuf_t[2];
typedef struct st_timespec mc_timesbuf_t[2];

I am a longtime (20+ years) user of MC on Linux, OpenBSD, Mac and Windows and am thrilled to finally have succeeded in building it on IBM i PASE environment!

[andrew_b]

Is it enough to check _AIX macro as following:

@@ -100,7 +100,11 @@ typedef void (*fill_names_f) (const char *);
 typedef void *vfsid;
 
 #ifdef HAVE_UTIMENSAT
+#ifdef _AIX
+typedef struct st_timespec mc_timesbuf_t[2];
+#else
 typedef struct timespec mc_timesbuf_t[2];
+#endif
 #else
 typedef struct utimbuf mc_timesbuf_t;
 #endif

Is AIX version check is required?

[jax]

Thank you, andrew_b.

In the time since I opened this ticket, IBM's open source development team has released their own compilation of Midnight Commander.

I am going to download their Source RPMS and do a diff upon that and submit better and more comprehensive changes, including their openpty() changes.

Shall I post back here, or open a new ticket?

[andrew_b]

Replying to jax:

Shall I post back here, or open a new ticket?

Let's here.

[jax]

Thanks, I'll work on that and get back to you.

[jax]

RPM spec file IBM i PASE environment build of MC

[jax]

RPM spec file IBM i PASE environment build of MC

[jax]

I'm attaching the patches and RPM spec that were added by IBM i Open Source team to make the system compile on PASE. Oops, I see I attached the .spec file twice :(

[jax]

Patch regarding openpty()

[jax]

Patch to configure.ac

[IBMJesseG]

@jax, thanks for submitting this up!

@andrew_b, et al, I'm with the IBM teams and can help answer questions related to the patches @jax has submitted. My apologies for the sloppiness in the common.c patch (checking #ifdef HAVE_OPENPTY but commenting #endif PASE, etc).

[IBMJesseG]

The utimensat hack in the configure.ac could be wrapped with an "os400" host check (as you can see in the spec file, we build with "powerpc64-ibm-os400") until we find a more-correct long term fix.

[andrew_b]

What is the more correct way: check $host_os like

case $host_os in
*os400)
   ;;
*)
   AC_CHECK_FUNCS([utimensat])
   ;;
esac

or use IS_AIX macro defined in configure.ac?

[andrew_b]

In the mc.spec, I can see there are some strange things.

BuildRequires?: boost

MC is writen in pure C, therefore any C++ library is not needed.

BuildRequires?: libxml2-devel
BuildRequires?: libxml2-2
BuildRequires?: libxml2-tools
BuildRequires?: libxslt-devel

MC doesn't process any XML data inside. XML library is not needed.

BuildRequires?: unixODBC-devel
BuildRequires?: curl-devel
BuildRequires?: openssl-devel < 1.1.0

Another unnecessary dependencies.

BuildRequires?: libutil-devel

Can't say what it is.

BuildRequires?: slang-devel

You want buld MC with S-Lang, but run configure with "--with-ncurses-libs=/QOpenSys/pkgs/lib" option.

[IBMJesseG]

Andrew, the "case $host_os in *os400" is the correct way, since the change is not needed for AIX.

The grade for mc.spec: D+
The .spec file is admittedly sloppy as a result of being derivative of another .spec file. I'll clean up those extraneous BuildRequires?. libutil-devel is an IBMi-only package and is needed for the IBM i implementation of openpty(). We build against S-Lang, so I'll remove the ncurses reference also.

[IBMJesseG]

Andrew, we also made a couple changes to address another subshell issue particular to IBM i. Would you like me to include updated .spec/.patch files in this issue? (code changes are small)

[andrew_b]

Replying to IBMJesseG:

Andrew, the "case $host_os in *os400" is the correct way, since the change is not needed for AIX.

Thanks!

libutil-devel is an IBMi-only package and is needed for the IBM i implementation of openpty().

We also made a couple changes to address another subshell issue particular to IBM i.

We should detect libraries for openpty more generally (see ​gnulib pty_h and pty modules for the reference). Let's move it to #3915.

Would you like me to include updated .spec/.patch files in this issue? (code changes are small)

No. We have some distro-specific stuff in the source tree (/contrib) but it is orpahed in fact and I don't know who use it.

[andrew_b]

I'm sorry I confused two IBM's operation systems: i and AIX.

[andrew_b]

Branch: 3927_ibm_i
chamgeset:9512e50bb3ece3ef00d4dff11d746c12ec03b113

If you have patches specific to IBM i only, please attach them here.

[IBMJesseG]

Updated patches and spec file, Sept 2, 2018

[IBMJesseG]

Thanks, Andrew.

To summarize the changes that @jax and I have to contribute:

subshell/common.c changes:

• openpty() support, for multiple platforms
• PTY_ZEROREAD fix for IBM i (and potentially others)

configure.ac changes:

• Change to program paths for perl/ruby/python (very specific to IBM i)
• utimensat workaround for IBM i
• define PTY_ZEROREAD for IBM i

And just to clarify the actions you'd like us to take for these:

subshell/common.c changes:

• openpty() support: bring to #3915
• PTY_ZEROREAD fix: send to branch 3927_ibm_i

configure.ac changes:

• Change to program paths: send to branch 3927_ibm_i
• utimensat workaround for IBM i: send to branch 3927_ibm_i
• define PTY_ZEROREAD for IBM i : send to branch 3927_ibm_i

When you said to attach the patches "here" I am wasn't sure if you are referring to this issue or the new branch. In any event, I've attached the latest patches and .spec file here for clarity's sake (patches 8sep2018.zip), but will gladly do what is requested.

[andrew_b]

Thanks!

Patches were applied with some my modifications to 3927_ibm_i and 3915_cleanup branches.
Please review and test.

You can checkout these branches from your local repo, or download as archives from Github:
​https://github.com/MidnightCommander/mc/archive/3927_ibm_i.zip
​https://github.com/MidnightCommander/mc/archive/3915_cleanup.zip

[andrew_b]

3915_cleanup branch doesn't contain i-specific patches. They're in the 3927_ibm_i branch only.

I'm going to merge 3915_cleanup into master during this week.

[IBMJesseG]

Thanks! I've done a review of both branches, and all looks good, except the following add to configure.ac doesn't look like it got applied anywhere.

AC_CHECK_FUNCS(openpty)

I'll let you determine whether you want that conditioned to IBM i, or left as a general check to all platforms (should be ok but I have no means of testing all the necessary platforms).

[andrew_b]

Replying to IBMJesseG:

the following add to configure.ac doesn't look like it got applied anywhere.

AC_CHECK_FUNCS(openpty)

That is in the mc-subshell.m4 file.

[IBMJesseG]

Ahh, sorry I missed that. Thanks!!

[andrew_b]

Merged to master: [3f8290e27e70a4f6212e0e0a527c1577e238371e].

git log --pretty=oneline 202b80a..3f8290e