Uploaded image for project: 'Accumulo'
  1. Accumulo
  2. ACCUMULO-2812

Support more architectures when compiling native lib

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 1.6.0
    • 1.6.1, 1.7.0
    • native

    Description

      Since we're no longer pre-building the native maps for distribution, and instead packaging a tiny C project to make it easier for users to create their own, it does not make sense for the Makefile to specify a specific architecture.

      Rather, it makes more sense that the Makefile that is packaged not specify the target architecture and instead rely on the current system architecture instead.

      I think all this would require is to drop -m64 from the CXXFLAGS, which seems fine for Linux. I'm not sure if this has any unintended side-effects for other systems, such as Mac.

      Attachments

        1. ACCUMULO-2812.v1.patch
          2 kB
          Christopher Tubbs
        2. ACCUMULO-2812.v2.patch
          2 kB
          Christopher Tubbs

        Issue Links

          Activity

            The assumption here is that most users will be building on the target system in which they want to run, and that their default compiler options support the current arch.

            We could implement this with an additional environment variable to pass extra flags, and for convenience, those flags can be set from the build_native_library.sh command-line. I think that solution better supports different circumstances than simply forcing -m64 as is currently done.

            ctubbsii Christopher Tubbs added a comment - The assumption here is that most users will be building on the target system in which they want to run, and that their default compiler options support the current arch. We could implement this with an additional environment variable to pass extra flags, and for convenience, those flags can be set from the build_native_library.sh command-line. I think that solution better supports different circumstances than simply forcing -m64 as is currently done.

            Patch ACCUMULO-2812.v1.patch uses default arch options, but allows passing compiler options to make via USERFLAGS environment variable with build_native_library.sh's command line.

            ctubbsii Christopher Tubbs added a comment - Patch ACCUMULO-2812.v1.patch uses default arch options, but allows passing compiler options to make via USERFLAGS environment variable with build_native_library.sh's command line.

            Patch depends on ACCUMULO-2762 patch being applied first.

            ctubbsii Christopher Tubbs added a comment - Patch depends on ACCUMULO-2762 patch being applied first.
            mackrorysd Sean Mackrory added a comment -

            +1 (non-binding) overall - I think your assumption that users are building on the target system is safe default behavior. I would suggest changing the shell command so that only variables named exactly USERFLAGS get matched and not variables that might contain USERFLAGS as a substring (as rare as that might be):

            env | grep "^USERFLAGS=" | cut -d= -f2
            mackrorysd Sean Mackrory added a comment - +1 (non-binding) overall - I think your assumption that users are building on the target system is safe default behavior. I would suggest changing the shell command so that only variables named exactly USERFLAGS get matched and not variables that might contain USERFLAGS as a substring (as rare as that might be): env | grep "^USERFLAGS=" | cut -d= -f2
            elserj Josh Elser added a comment -

            Would be nice to see a comment in the script about what USERFLAGS does, not to mention some sort of higher-level documentation. Maybe add in getopt and add a help message to the build_native_library.sh script? A mention in the user manual would also be good.

            Otherwise, LGTM.

            elserj Josh Elser added a comment - Would be nice to see a comment in the script about what USERFLAGS does, not to mention some sort of higher-level documentation. Maybe add in getopt and add a help message to the build_native_library.sh script? A mention in the user manual would also be good. Otherwise, LGTM.

            Patch ACCUMULO-2812.v2.patch updated to reflect mackrorysd's suggestion.

            ctubbsii Christopher Tubbs added a comment - Patch ACCUMULO-2812.v2.patch updated to reflect mackrorysd 's suggestion.

            Commit 7ccf2026913993d5ce076c05f78e3921c532b2ac in accumulo's branch refs/heads/1.6.1-SNAPSHOT from ctubbsii
            [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=7ccf202 ]

            ACCUMULO-2812 Pass options to compiler for native lib

            Use system default architecture options when compiling, instead of forcing
            -m64. Allow passing additional arguments (USERFLAGS) to specify additional
            compiler options. Set USERFLAGS with the build_native_library.sh's
            command-line arguments.

            jira-bot ASF subversion and git services added a comment - Commit 7ccf2026913993d5ce076c05f78e3921c532b2ac in accumulo's branch refs/heads/1.6.1-SNAPSHOT from ctubbsii [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=7ccf202 ] ACCUMULO-2812 Pass options to compiler for native lib Use system default architecture options when compiling, instead of forcing -m64. Allow passing additional arguments (USERFLAGS) to specify additional compiler options. Set USERFLAGS with the build_native_library.sh's command-line arguments.

            Commit 7ccf2026913993d5ce076c05f78e3921c532b2ac in accumulo's branch refs/heads/master from ctubbsii
            [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=7ccf202 ]

            ACCUMULO-2812 Pass options to compiler for native lib

            Use system default architecture options when compiling, instead of forcing
            -m64. Allow passing additional arguments (USERFLAGS) to specify additional
            compiler options. Set USERFLAGS with the build_native_library.sh's
            command-line arguments.

            jira-bot ASF subversion and git services added a comment - Commit 7ccf2026913993d5ce076c05f78e3921c532b2ac in accumulo's branch refs/heads/master from ctubbsii [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=7ccf202 ] ACCUMULO-2812 Pass options to compiler for native lib Use system default architecture options when compiling, instead of forcing -m64. Allow passing additional arguments (USERFLAGS) to specify additional compiler options. Set USERFLAGS with the build_native_library.sh's command-line arguments.

            People

              ctubbsii Christopher Tubbs
              ctubbsii Christopher Tubbs
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: