Enhance shared library support for AIX #12352

KamathForAIX · 2023-10-10T13:22:49Z

This PR is for fixing issue number #12219 in meson for AIX operating system as discussed.

KamathForAIX · 2023-10-13T09:27:18Z

Respected @eli-schwartz and community members,

Kindly review this pull request. Let me know what you think.

eli-schwartz · 2023-10-13T22:31:21Z

I'm putting this on my schedule to review on Sunday. :)

KamathForAIX · 2023-10-15T13:55:28Z

Thank you.

mesonbuild/modules/python.py

mesonbuild/backend/ninjabackend.py

KamathForAIX · 2023-10-16T14:55:33Z

Respected @eli-schwartz and community members,

I addressed the comments mentioned in the review. Kindly let me know what you think now and if any further changes needed from my end.

While I tried to add conditions for AIX code so that no other targets are affected I still see one failure in checks which I could not figure out why my changes have caused the same. You all are experts and have immense knowledge of meson, Kindly let me know what can be the root cause of the same with respect to these changes.

Have a nice day ahead.

mesonbuild/backend/backends.py

mesonbuild/backend/ninjabackend.py

eli-schwartz · 2023-10-17T21:05:19Z

While I tried to add conditions for AIX code so that no other targets are affected I still see one failure in checks which I could not figure out why my changes have caused the same. You all are experts and have immense knowledge of meson, Kindly let me know what can be the root cause of the same with respect to these changes.

This failing test (msys2 clang handling of LLVM) is failing everywhere, not just on your PR. Don't worry about it.

KamathForAIX · 2023-10-18T10:22:35Z

While I tried to add conditions for AIX code so that no other targets are affected I still see one failure in checks which I could not figure out why my changes have caused the same. You all are experts and have immense knowledge of meson, Kindly let me know what can be the root cause of the same with respect to these changes.

This failing test (msys2 clang handling of LLVM) is failing everywhere, not just on your PR. Don't worry about it.

Thank you so much for this confirmation @eli-schwartz.

So I made the suggested changes. Kindly let me know if any more changes are needed.

Have a nice day ahead.

Thanks and regards,
Aditya.

KamathForAIX · 2023-10-23T09:17:43Z

Respected @eli-schwartz and community members,

Kindly let me know if I missed something in the review comments. I addressed the review changes suggested and squashed the commit into one message.

Kindly merge this PR if there are no changes needed. Thank you for your guidance.

Regards,
Aditya Kamath.

eli-schwartz · 2023-10-30T00:36:13Z

mesonbuild/backend/ninjabackend.py

-            if self.environment.machines[for_machine].is_aix():
-                rule = 'AIX_LINKER{}'.format(self.get_rule_suffix(for_machine))
-                description = 'Archiving AIX shared library'
-                cmdlist = compiler.get_command_to_archive_shlib()
-                args = []
-                options = {}
-                self.add_rule(NinjaRule(rule, cmdlist, args, description, **options, extra=None))
+        if self.environment.machines[for_machine].is_aix():
+            rule = 'AIX_LINKER{}'.format(self.get_rule_suffix(for_machine))
+            description = 'Archiving AIX shared library'
+            cmdlist = compiler.get_command_to_archive_shlib()
+            args = []
+            options = {}
+            self.add_rule(NinjaRule(rule, cmdlist, args, description, **options, extra=None))


Why is this indentation being removed here? It looks like you are dropping off the end of the for loop and only producing one rule, using whichever for_machine turns out to be last...

@eli-schwartz

You are right here. I changed it back. Also, while I changed it back, I saw an error while building Scipy, which looked like this:

File "/meson_install/opt/freeware/lib/python3.9/site-packages/mesonbuild/backend/ninjabackend.py", line 1315, in generate_rules self.generate_dynamic_link_rules() File "/meson_install/opt/freeware/lib/python3.9/site-packages/mesonbuild/backend/ninjabackend.py", line 2388, in generate_dynamic_link_rules cmdlist = compiler.get_command_to_archive_shlib() File "/meson_install/opt/freeware/lib/python3.9/site-packages/mesonbuild/compilers/compilers.py", line 963, in get_command_to_archive_shlib return self.linker.get_command_to_archive_shlib() AttributeError: 'NoneType' object has no attribute 'get_command_to_archive_shlib'

I added a guard in the compilers.py file. Kindly see the changes there and let me know. This is because linker object was none.

mesonbuild/backend/ninjabackend.py

eli-schwartz · 2023-10-30T00:38:29Z

Apologies for the lateness of the reply, there were some IRL things going on (all good news, I promise! 😄)

KamathForAIX · 2023-10-31T01:34:19Z

Apologies for the lateness of the reply, there were some IRL things going on (all good news, I promise! 😄)

Thank you for your reply. No problem. Please let me know now. I have made those changes.

Previously, AIX support was updated to archive shared libraries per AIX platform conventions, which expect .a files that contain .so files. This is usually correct, but an edge case occurs for loadable plugins, e.g. what meson creates for `shared_module()`. A notable example is python extensions (SciPy, for example). These should *not* be archived, because the .so file itself needs to be loaded as a plugin. For example, SciPy fails to import in the python interpreter. Handle this by differentiating between plugins and regular libraries, and only archiving when safe to do so. Fixes mesonbuild#12219

A compiler may not have a linker: ``` 'NoneType' object has no attribute 'get_command_to_archive_shlib' ```

eli-schwartz · 2023-10-31T23:28:32Z

I have tweaked the commit messages and moved some code in the second commit back into the first commit where it seemed to belong. Here is a range-diff of the changes:

$ git --no-pager range-diff origin/master 1563bac5e cfec25502
1:  2bac26601 ! 1:  f4d19db25 Use target_aix_so_archive to decide to archive shared library in AIX
    @@ Metadata
     Author: Aditya Vidyadhar Kamath <[email protected]>
     
      ## Commit message ##
    -    Use target_aix_so_archive to decide to archive shared library in AIX
    +    Use target.aix_so_archive to decide to archive shared library in AIX
     
    -    Enhance shared library support for AIX
    +    Previously, AIX support was updated to archive shared libraries per AIX
    +    platform conventions, which expect .a files that contain .so files. This
    +    is usually correct, but an edge case occurs for loadable plugins, e.g.
    +    what meson creates for `shared_module()`. A notable example is python
    +    extensions (SciPy, for example).
     
    -    Change == to is and simplify patch to fix build break
    +    These should *not* be archived, because the .so file itself needs to be
    +    loaded as a plugin. For example, SciPy fails to import in the python
    +    interpreter.
     
    -    Simplify the logic by overriding aix_so_archive in SharedModule
    +    Handle this by differentiating between plugins and regular libraries,
    +    and only archiving when safe to do so.
     
    -    Check if AIX is the machine always
    -
    -    Fix to cross compile build break
    -
    -    Avoid using aix_so_archive in other targets
    -
    -    Remove unnecessary conditions
    -
    -    Add comment to condition change in install section for AIX
    -
    -    Simply if condition to a simple and logic
    -
    -    Use only target.aix_so_archive and change one cond to original
    +    Fixes #12219
     
      ## mesonbuild/backend/backends.py ##
     @@ mesonbuild/backend/backends.py: class Backend:
    @@ mesonbuild/backend/ninjabackend.py: class NinjaBackend(backends.Backend):
      
          def should_use_dyndeps_for_target(self, target: 'build.BuildTarget') -> bool:
              if mesonlib.version_compare(self.ninja_version, '<1.10.0'):
    -@@ mesonbuild/backend/ninjabackend.py: class NinjaBackend(backends.Backend):
    - 
    -                 options = self._rsp_options(compiler)
    -                 self.add_rule(NinjaRule(rule, command, args, description, **options, extra=pool))
    --            if self.environment.machines[for_machine].is_aix():
    --                rule = 'AIX_LINKER{}'.format(self.get_rule_suffix(for_machine))
    --                description = 'Archiving AIX shared library'
    --                cmdlist = compiler.get_command_to_archive_shlib()
    --                args = []
    --                options = {}
    --                self.add_rule(NinjaRule(rule, cmdlist, args, description, **options, extra=None))
    -+        if self.environment.machines[for_machine].is_aix():
    -+            rule = 'AIX_LINKER{}'.format(self.get_rule_suffix(for_machine))
    -+            description = 'Archiving AIX shared library'
    -+            cmdlist = compiler.get_command_to_archive_shlib()
    -+            args = []
    -+            options = {}
    -+            self.add_rule(NinjaRule(rule, cmdlist, args, description, **options, extra=None))
    - 
    -         args = self.environment.get_build_command() + \
    -             ['--internal',
     @@ mesonbuild/backend/ninjabackend.py: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47485'''))
              else:
                  dependencies = target.get_dependencies()
    @@ mesonbuild/backend/ninjabackend.py: https://gcc.gnu.org/bugzilla/show_bug.cgi?id
                      # they are all built
                      #Add archive file if shared library in AIX for build all.
     -                if isinstance(t, build.SharedLibrary):
    --                    if self.environment.machines[t.for_machine].is_aix():
    -+                if self.environment.machines[t.for_machine].is_aix():
    -+                    if isinstance(t, build.SharedLibrary) and t.aix_so_archive:
    ++                if isinstance(t, build.SharedLibrary) and t.aix_so_archive:
    +                     if self.environment.machines[t.for_machine].is_aix():
                              linker, stdlib_args = self.determine_linker_and_stdlib_args(t)
                              t.get_outputs()[0] = linker.get_archive_name(t.get_outputs()[0])
    -                 targetlist.append(os.path.join(self.get_target_dir(t), t.get_outputs()[0]))
     
      ## mesonbuild/build.py ##
     @@ mesonbuild/build.py: class SharedLibrary(BuildTarget):
2:  1563bac5e < -:  --------- Fix to 'NoneType' object has no attribute 'get_command_to_archive_shlib'
-:  --------- > 2:  cfec25502 Fix traceback on AIX in shlib archiving code

KamathForAIX · 2023-11-01T05:17:41Z

Thank you @eli-schwartz for the merge and your guidance. I learned a lot from you. Thank you, community members.

KamathForAIX requested a review from jpakkane as a code owner October 10, 2023 13:22

eli-schwartz reviewed Oct 15, 2023

View reviewed changes

mesonbuild/modules/python.py Outdated Show resolved Hide resolved

mesonbuild/backend/ninjabackend.py Outdated Show resolved Hide resolved

eli-schwartz reviewed Oct 17, 2023

View reviewed changes

mesonbuild/backend/backends.py Outdated Show resolved Hide resolved

mesonbuild/backend/ninjabackend.py Outdated Show resolved Hide resolved

KamathForAIX force-pushed the git-issue-12219 branch from 88cf00b to 2bac266 Compare October 18, 2023 13:48

eli-schwartz reviewed Oct 30, 2023

View reviewed changes

KamathForAIX requested a review from dcbaker as a code owner October 30, 2023 04:07

KamathForAIX added 2 commits October 31, 2023 19:18

Fix traceback on AIX in shlib archiving code

cfec255

A compiler may not have a linker: ``` 'NoneType' object has no attribute 'get_command_to_archive_shlib' ```

eli-schwartz force-pushed the git-issue-12219 branch from 1563bac to cfec255 Compare October 31, 2023 23:26

eli-schwartz merged commit cfec255 into mesonbuild:master Nov 1, 2023
39 checks passed

rgommers mentioned this pull request Dec 7, 2023

BUG: Cannot build master branch of numpy in AIX. numpy/numpy#25244

Closed

KamathForAIX mentioned this pull request Dec 6, 2024

BUG: Building pandas via pip fails in AIX due to use of lower meson version pandas-dev/pandas#60495

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance shared library support for AIX #12352

Enhance shared library support for AIX #12352

KamathForAIX commented Oct 10, 2023

KamathForAIX commented Oct 13, 2023

eli-schwartz commented Oct 13, 2023

KamathForAIX commented Oct 15, 2023

KamathForAIX commented Oct 16, 2023 •

edited

Loading

eli-schwartz commented Oct 17, 2023

KamathForAIX commented Oct 18, 2023

KamathForAIX commented Oct 23, 2023

eli-schwartz Oct 30, 2023

KamathForAIX Oct 30, 2023

eli-schwartz commented Oct 30, 2023

KamathForAIX commented Oct 31, 2023

eli-schwartz commented Oct 31, 2023

KamathForAIX commented Nov 1, 2023 •

edited

Loading

Enhance shared library support for AIX #12352

Enhance shared library support for AIX #12352

Conversation

KamathForAIX commented Oct 10, 2023

KamathForAIX commented Oct 13, 2023

eli-schwartz commented Oct 13, 2023

KamathForAIX commented Oct 15, 2023

KamathForAIX commented Oct 16, 2023 • edited Loading

eli-schwartz commented Oct 17, 2023

KamathForAIX commented Oct 18, 2023

KamathForAIX commented Oct 23, 2023

eli-schwartz Oct 30, 2023

Choose a reason for hiding this comment

KamathForAIX Oct 30, 2023

Choose a reason for hiding this comment

eli-schwartz commented Oct 30, 2023

KamathForAIX commented Oct 31, 2023

eli-schwartz commented Oct 31, 2023

KamathForAIX commented Nov 1, 2023 • edited Loading

KamathForAIX commented Oct 16, 2023 •

edited

Loading

KamathForAIX commented Nov 1, 2023 •

edited

Loading