Ticket #3927 (closed enhancement: fixed)
mc on IBM i PASE building 4.8.20 change
Reported by: | jax | Owned by: | andrew_b |
---|---|---|---|
Priority: | minor | Milestone: | 4.8.22 |
Component: | mc-vfs | Version: | 4.8.20 |
Keywords: | PASE, AIX, iSeries, AS/400 | Cc: | |
Blocked By: | Blocking: | ||
Branch state: | merged | Votes for changeset: | committed-master |
Description
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!
Attachments
Change History
comment:2 follow-up: ↓ 3 Changed 6 years ago by 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?
Changed 6 years ago by jax
RPM spec file IBM i PASE environment build of MC
Changed 6 years ago by jax
RPM spec file IBM i PASE environment build of MC
comment:5 Changed 6 years ago by 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 :(
comment:6 Changed 6 years ago by 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).
comment:7 Changed 6 years ago by 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.
comment:8 Changed 6 years ago by 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?
comment:9 Changed 6 years ago by 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.
comment:10 Changed 6 years ago by 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.
comment:11 follow-up: ↓ 12 Changed 6 years ago by 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)
comment:12 in reply to: ↑ 11 Changed 6 years ago by 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.
comment:13 Changed 6 years ago by andrew_b
I'm sorry I confused two IBM's operation systems: i and AIX.
comment:14 Changed 6 years ago by andrew_b
- Owner set to andrew_b
- Status changed from new to accepted
- Branch state changed from no branch to on review
- Milestone changed from Future Releases to 4.8.22
Branch: 3927_ibm_i
changeset:9512e50bb3ece3ef00d4dff11d746c12ec03b113
If you have patches specific to IBM i only, please attach them here.
Changed 6 years ago by IBMJesseG
- Attachment patches, 3sep2018.zip added
Updated patches and spec file, Sept 2, 2018
comment:15 Changed 6 years ago by 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.
comment:16 Changed 6 years ago by 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
comment:17 Changed 6 years ago by 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.
comment:18 follow-up: ↓ 19 Changed 6 years ago by 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).
comment:19 in reply to: ↑ 18 Changed 6 years ago by 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.
comment:20 Changed 6 years ago by IBMJesseG
Ahh, sorry I missed that. Thanks!!
comment:21 Changed 6 years ago by andrew_b
- Votes for changeset set to IBMJesseG andrew_b
- Branch state changed from on review to approved
comment:22 Changed 6 years ago by andrew_b
- Status changed from accepted to testing
- Votes for changeset changed from IBMJesseG andrew_b to committed-master
- Resolution set to fixed
- Branch state changed from approved to merged
Merged to master: [3f8290e27e70a4f6212e0e0a527c1577e238371e].
git log --pretty=oneline 202b80a..3f8290e
Is it enough to check _AIX macro as following:
Is AIX version check is required?