Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MAINT: update meson.build to make it work on IBM i system #17193

Merged
merged 3 commits into from Oct 12, 2022
Merged

MAINT: update meson.build to make it work on IBM i system #17193

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2bd66eb
update meson.build to make it works on i
GavinGZhang Oct 11, 2022
39e5559
remove the pthread per the fix #17200 from Ralf Gommers
GavinGZhang Oct 12, 2022
974296b
DOC: add comment to refer to more details for IBM i build issue
rgommers Oct 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions meson.build
View file
Open in desktop
Expand Up @@ -50,6 +50,13 @@ m_dep = cc.find_library('m', required : false)
if m_dep.found()
add_project_link_arguments('-lm', language : 'c')
endif
if host_machine.system() == 'os400'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll note that this string isn't documented in https://mesonbuild.com/Reference-tables.html#operating-system-names, so not guaranteed to remain stable (although in practice, it shouldn't change).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. We didn't focus on mason , as we can smoothly(almost, except for the unkown CPU type. 😄 ) pip install it on our system. I would guess it's trying to using some python interface to get it. I am make sure this interface would not change to other value on my system, as "os400" exactly match our system.

add_global_arguments('-pthread', language : 'cpp')
rgommers marked this conversation as resolved.
Show resolved Hide resolved
add_global_arguments('-D__STDC_FORMAT_MACROS', language : 'cpp')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines should use add_project_argument rather than add_global_argument.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding __STDC_FORMAT_MACROS is fine - I guess the OS/400 C++ compiler is a bit outdated (modern C++ compilers should unconditionally define what's behind this macro - see https://stackoverflow.com/questions/8132399/how-to-printf-uint64-t-fails-with-spurious-trailing-in-format/8132440#8132440).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines should use add_project_argument rather than add_global_argument.

Yeah. I would try add_project_argument. BTW, in fact, by checking the build.ninja , we can find that "-pthread" could be added for some c++ source code, while, it only failed on some files , such as control.cc. To be honest , I didn't figure out why mason treat control.cc specially.

Adding __STDC_FORMAT_MACROS is fine - I guess the OS/400 C++ compiler is a bit outdated (modern C++ compilers should unconditionally define what's behind this macro - see https://stackoverflow.com/questions/8132399/how-to-printf-uint64-t-fails-with-spurious-trailing-in-format/8132440#8132440).

Yes, this is exactly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines should use add_project_argument rather than add_global_argument.

Last change: can you push this update?

add_project_link_arguments('-Wl,-bnotextro', language : 'c')
add_project_link_arguments('-Wl,-bnotextro', language : 'cpp')
add_project_link_arguments('-Wl,-bnotextro', language : 'fortran')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a little tricky to find what this flag means. From the AIX 7.2 docs for ld:

notextro or nro: Does not check to ensure that there are no load time relocation entries for the text section of the output object file. This is the default.

So I guess on OS/400 it is not the default, unlike on AIX, and somehow this is a problem? For completeness it'd be nice to see the build error that shows up in the absence of this flag @zheddie.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. here's the error message if without those options:

[19/1566] Compiling C object scipy/special/libcephes.a.p/cephes_const.c.o
      [20/1566] Linking target scipy/_lib/_test_ccallback.cpython-39.so
      FAILED: scipy/_lib/_test_ccallback.cpython-39.so
      cc  -o scipy/_lib/_test_ccallback.cpython-39.so scipy/_lib/_test_ccallback.cpython-39.so.p/src__test_ccallback.c.o -Wl,-berok -Wl,-bnoipath -Wl,-bbigtoc -shared -fPIC -lm -Wl,-blibpath:/qopensys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10:/qopensys/pkgs/lib/gcc:/qopensys/pkgs/lib:/lib:/usr/lib
      ld: 0711-302 ERROR: Object scipy/_lib/_test_ccallback.cpython-39.so.p/src__test_ccallback.c.o, csect <.text>
          The csect is part of the .text section, and relocation entries
          from the csect have been written to the .loader section.
      ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
      collect2: error: ld returned 8 exit status

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Interesting that it's AIX 6.1 with GCC, which seems to be working on other IBM hardware (I think, or we would have had more complaints by now) but not on IBM i system.

Anyway, this change looks good then. It's failing on the very first shared library being built, so it's going to fail on all of them. Hence adding a project-wide link argument seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. Sorry, just noticed that I am still using the add_global_arguments(). It's a mis-match from my local system and build system. My test were donewith add_project_arguments() . So, reset the code change with correct one. Sorry about it.

As for the IBM i , yes, we are not AIX, and this is our first type trying using the pip3 directly on our system. Before that we are trying using the RPM to ship Scipy, which is not easy for us to keep pace with open source world.

endif

# Adding at project level causes many spurious -lgfortran flags.
add_languages('fortran', native: false)
Expand Down