Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8217269

jpackage Makefile improvments

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: internal
    • Fix Version/s: internal
    • Component/s: tools
    • Labels:

      Description

      Feedback from build team on EA2:
      Just a few minor nits:

      * In Launcher-jdk.jpackage.gmk, please indent the else clause ("$(eval $(call SetupBuildLauncher, jpackage ....") two spaces.

      * In Lib-jdk.jpackage.gmk, I think there's still room to prune some more unnecessary compiler flags and parameters to SetupJdkExecutable:

        65 CFLAGS_linux := -fPIC, \
        66 CFLAGS_solaris := -KPIC, \
        67 CFLAGS_macosx := -fPIC, \

       I wonder if these really are needed. Normally, only shared libraries neeed PIC code. (And for those we set it automatically.)

        69 LDFLAGS_windows := -nologo, \

      This should not be needed, it's incorporated in CXXFLAGS_JDKEXE. (Also in the second block, for jpackageapplauncherw).

        72 LIBS_solaris := -lc, \

      Same here; this should not be needed. It's in GLOBAL_LIBS on Solaris, and is always included.

        75 VERSIONINFO_RESOURCE := $(GLOBAL_VERSION_INFO_RESOURCE), \

      This is setup by default by SetupJdkExecutable, so you can remove it. (Also in the second block, for jpackageapplauncherw).

      Finally, I do believe that the setups of jpackageapplauncher and jpackageapplauncherw should be done in Launcher-jdk.jpackage.gmk, not Lib-jdk.jpackage.gmk. Since they are to be shipped as resources, they are not "really" launchers in our normal sense, but they are at least more launchers than they are libraries.

      As we've talked about privately, in the future I'd like to see Windows too use the SetupBuildLauncher method for creating the launcher, but this is clearly good enough for inclusion in the mainline.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                herrick Andy Herrick
                Reporter:
                herrick Andy Herrick
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: