Re: [PATCH] fdo#39643: Remove --disable-strip-solver configure option

Hello Petr,

thank you for your review and your comments.
Here is a new version with these issues fixed:

1. no strip in solenv/bin/deliver.pl any more
2. solenv/bin/modules/installer preserves striping code
3. there is a new target install-strip that do strip during installation 
process.

Question: i can not see where $strip var in ooinstall is parsed (in 
make_installer.pl call)?

if ($ENV{BUILD_TYPE} =~ m/ODK/) {
    print "Running SDK installer\n";
    system ("cd $ENV{SRC_ROOT}/instsetoo_native/util ; " .
    "perl -w $ENV{SOLARENV}/bin/make_installer.pl " .
    "-f openoffice.lst -l en-US -p LibreOffice_SDK " .
    "-u $tmp_dir " .
    "-buildid $BUILD $destdir $strip $msi " .
    "-simple $path") && die "Failed to install: $!";
}

For the new param in Makefile ooinstall -strip to be passed to
make_installer.pl I have to parse it myself in ooinstall, right?

Ciao
David

On 25.04.2012 16:56, Petr Mladek wrote:
> d.ostrovsky@idaia.de  píše v Út 24. 04. 2012 v 11:33 +0200:
> > Do we agree about to drop the striping code from ooinstaller?
> What do you mean by ooinstaller? Just the single ooinstall script or
> also also the solenv/bin/modules/installer/* stuff?
>
> It is hard to say until we know what is the benefit of stripping. If it
> makes sense, you need to keep the support for "make install-strip". It
> will call ooinstall as well.
>
> > As far as I can see the use cases we have are:
> >
> > Use Case (UC) 1: distro maintainer would build without -g, nothing to
> > be stripped.
> This is not true. Many distro maintainers actually build with -g. They
> later use some magic to put the debug information into separate
> packages.
>
>
> > UC 2: developer with change, compile, test cycle would build with -g
> > and install LO with make install oder make dev-install. He wants
> > definitely preserve the symbols.
> yup
>
> > UC 3: developer compiles it with symbols and want provide the build
> > artifacts to somebody to test.
> > It could be the only justification to provide a new make target
> > "strip-and-install". But the tests with stripped
> > artifacts are useless, no deeper analysis is possible.
> Developer might have the symbols to track the code in debugger to
> understand how it works. A backtrace from user is useful only for
> crashes or some other particular problems. It is not useful to check
> that a functionality works as expected. If you have full debug build,
> the stripped binaries can safe a lot for download.
>
> When I think about it, we want to keep stripping capability in the
> installer stuff for this purpose. Though, we want to disable it in the
> default "make install".
>
> >   Another
> > argument for not to strip even in this case is a new configure option
> > that was introduced in the last hackfest to selectively specify the
> > list of libraries to build with symbols. With this option not the
> > whole LO but only a couple of libs would be built with symbols.
> Yes, it might be better but I still think that strip might safe a lot
> here on download.
>
> Please, wait a bit. I am trying to build without -g and without
> stripping to get some data. I am going crazy today because the binaries
> are stripped in solver even when I use --disable-strip-solver. Sigh.
>
>
> Best Regards,
> Petr
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice


From 860462c53c8e33e77b57db0e693e508b0b39f40d Mon Sep 17 00:00:00 2001
From: David Ostrovsky <David.Ostrovsky@gmx.de>
Date: Wed, 25 Apr 2012 21:29:28 +0200
Subject: [PATCH] fdo#39643: Remove --disable-strip-solver configure option

---
 Makefile                                  |    9 ++++-
 config_host.mk.in                         |    1 -
 configure.in                              |   30 ---------------
 instsetoo_native/util/makefile.mk         |    8 ++--
 solenv/bin/deliver.pl                     |   59 +----------------------------
 solenv/bin/modules/installer/globals.pm   |    2 +-
 solenv/bin/modules/installer/parameter.pm |    4 +-
 solenv/bin/ooinstall                      |    6 ---
 solenv/gbuild/gbuild.mk                   |    6 ---
 solenv/gbuild/platform/solaris.mk         |    6 ---
 solenv/gbuild/platform/unxgcc.mk          |    6 ---
 11 files changed, 16 insertions(+), 121 deletions(-)

diff --git a/Makefile b/Makefile
index 9c18cd9..8993256 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 # -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
 
-.PHONY : all bootstrap build check clean clean-build clean-host dev-install distclean 
distro-pack-install docs fetch findunusedcode id install subsequentcheck tags
+.PHONY : all bootstrap build check clean clean-build clean-host dev-install distclean 
distro-pack-install docs fetch findunusedcode id install install-strip subsequentcheck tags
 
 ifeq ($(MAKECMDGOALS),)
 MAKECMDGOALS:=all
@@ -439,6 +439,13 @@ install:
        echo "Installation finished, you can now execute:" && \
        echo "$(INSTALLDIR)/program/soffice"
 
+install-strip:
+       echo "Installing in $(INSTALLDIR)..." && \
+       ooinstall -strip "$(INSTALLDIR)" && \
+       echo "" && \
+       echo "Installation finished, you can now execute:" && \
+       echo "$(INSTALLDIR)/program/soffice"
+
 dev-install: build
        @rm -rf $(DEVINSTALLDIR)
        @mkdir $(DEVINSTALLDIR)
diff --git a/config_host.mk.in b/config_host.mk.in
index 9a6b09c..5e02107 100644
--- a/config_host.mk.in
+++ b/config_host.mk.in
@@ -89,7 +89,6 @@ export DISABLE_SCRIPTING=@DISABLE_SCRIPTING@
 export DISABLE_LINKOO=@DISABLE_LINKOO@
 export DISABLE_NEON=@DISABLE_NEON@
 export DISABLE_PYTHON=@DISABLE_PYTHON@
-export DISABLE_STRIP=@DISABLE_STRIP@
 export DLLTOOL=@DLLTOOL@
 export DMAKEROOT=@SRC_ROOT@/solenv/inc/startup
 export DOCDIR=@DOCDIR@
diff --git a/configure.in b/configure.in
index 0b0ceda..f4e33be 100644
--- a/configure.in
+++ b/configure.in
@@ -616,19 +616,6 @@ AC_ARG_ENABLE(symbols,
          the specified gbuild-build targets (all means everything, - prepended
          means not to enable, / appended means everything in the directory,
          there is no ordering and removal disabling takes precedence).])
-    [
-                          Enabling symbols disables the stripping of the solver
-                          (--disable-strip-solver).
-    ],
-,)
-
-AC_ARG_ENABLE(strip-solver,
-    AS_HELP_STRING([--disable-strip-solver],
-        [Disable the stripping of the solver. By default the solver is stripped
-         unless a build with debugging symbols (--enable-symbols) is requested.])
-    [
-                          This switch allows to override this setting.
-    ],
 ,)
 
 AC_ARG_ENABLE(werror,
@@ -3558,23 +3545,6 @@ fi
 AC_SUBST(ENABLE_SYMBOLS)
 AC_SUBST(ENABLE_SYMBOLS_FOR)
 
-dnl Determine if the solver is to be stripped or not.
-dnl ===================================================================
-AC_MSG_CHECKING([whether to strip the solver or not.])
-if test "$enable_strip_solver" = "no"; then
-    DISABLE_STRIP="TRUE"
-    AC_MSG_RESULT([no])
-else
-    if test -n "$ENABLE_SYMBOLS"; then
-        DISABLE_STRIP="TRUE"
-        AC_MSG_RESULT([no])
-    else
-        DISABLE_STRIP=
-        AC_MSG_RESULT([yes])
-    fi
-fi
-AC_SUBST(DISABLE_STRIP)
-
 if test "$enable_headless" = "yes"; then
     # be sure to do not mess with uneeded stuff
     test_randr=no
diff --git a/instsetoo_native/util/makefile.mk b/instsetoo_native/util/makefile.mk
index 71e9184..70e9273 100644
--- a/instsetoo_native/util/makefile.mk
+++ b/instsetoo_native/util/makefile.mk
@@ -271,19 +271,19 @@ ooodevhelppack_%{$(PKGFORMAT:^".")} :
 
 $(foreach,i,$(alllangiso) sdkoo_$i) : $$@{$(PKGFORMAT:^".")}
 sdkoo_%{$(PKGFORMAT:^".")} :
-    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_SDK -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSISDKOOTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -dontstrip -format $(@:e:s/.//) 
$(VERBOSESWITCH)
+    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_SDK -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSISDKOOTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -format $(@:e:s/.//) 
$(VERBOSESWITCH)
 
 $(foreach,i,$(alllangiso) sdkoodev_$i) : $$@{$(PKGFORMAT:^".")}
 sdkoodev_%{$(PKGFORMAT:^".")} :
-    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_Dev_SDK -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSISDKOOTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -dontstrip -format $(@:e:s/.//) 
$(VERBOSESWITCH)
+    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_Dev_SDK -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSISDKOOTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -format $(@:e:s/.//) 
$(VERBOSESWITCH)
 
 $(foreach,i,$(alllangiso) lotest_$i) : $$@{$(PKGFORMAT:^".")}
 lotest_%{$(PKGFORMAT:^".")} :
-    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_Test -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSITESTTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -dontstrip -format $(@:e:s/.//) 
$(VERBOSESWITCH)
+    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_Test -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSITESTTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -format $(@:e:s/.//) $(VERBOSESWITCH)
 
 $(foreach,i,$(alllangiso) lodevtest_$i) : $$@{$(PKGFORMAT:^".")}
 lodevtest_%{$(PKGFORMAT:^".")} :
-    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_Dev_Test -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSITESTTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -dontstrip -format $(@:e:s/.//) 
$(VERBOSESWITCH)
+    $(PERL) -w $(SOLARENV)$/bin$/make_installer.pl -f $(PRJ)$/util$/openoffice.lst -l 
$(subst,$(@:s/_/ /:1)_, $(@:b)) -p LibreOffice_Dev_Test -u $(OUT) -buildid $(BUILD) -msitemplate 
$(MSITESTTEMPLATEDIR) -msilanguage $(COMMONMISC)$/win_ulffiles -format $(@:e:s/.//) $(VERBOSESWITCH)
 
 
 $(foreach,i,$(alllangiso) ure_$i) : $$@{$(PKGFORMAT:^".")}
diff --git a/solenv/bin/deliver.pl b/solenv/bin/deliver.pl
index c524ce5..4502615 100755
--- a/solenv/bin/deliver.pl
+++ b/solenv/bin/deliver.pl
@@ -63,7 +63,6 @@ use File::Spec;
 @copy_filter_patterns = (
                         );
 
-$strip              = '';
 $is_debug           = 0;
 
 $error              = 0;
@@ -135,16 +134,6 @@ use sigtrap 'handler' => \&cleanup_and_die, 'normal-signals';
 
 parse_options();
 
-if ( ! $opt_delete ) {
-    if ( $ENV{GUI} eq 'WNT' ) {
-        if ($ENV{COM} eq 'GCC') {
-            initialize_strip() ;
-        };
-    } else {
-        initialize_strip();
-    }
-}
-
 init_globals();
 push_default_actions();
 parse_dlst();
@@ -649,39 +638,6 @@ sub glob_and_copy
     }
 }
 
-sub is_unstripped {
-    my $file_name = shift;
-    my $nm_output;
-
-    if (-f $file_name.$maybedot) {
-        my $file_type = `file $file_name`;
-        # OS X file command doesn't know if a file is stripped or not
-        if (($file_type =~ /not stripped/o) || ($file_type =~ /Mach-O/o) ||
-            (($file_type =~ /PE/o) && ($ENV{GUI} eq 'WNT') &&
-             ($nm_output = `nm $file_name 2>&1`) && $nm_output &&
-             !($nm_output =~ /no symbols/i) && !($nm_output =~ /not recognized/i))) {
-            return '1' if ($file_name =~ /\.bin$/o);
-            return '1' if ($file_name =~ /\.so\.*/o);
-            return '1' if ($file_name =~ /\.dylib\.*/o);
-            return '1' if ($file_name =~ /\.com\.*/o);
-            return '1' if ($file_name =~ /\.dll\.*/o);
-            return '1' if ($file_name =~ /\.exe\.*/o);
-            return '1' if (basename($file_name) !~ /\./o);
-        }
-    };
-    return '';
-}
-
-sub initialize_strip {
-    if (((!defined $ENV{CROSS_COMPILING}) || ($ENV{CROSS_COMPILING} ne 'YES')) &&
-        ((!defined $ENV{DISABLE_STRIP}) || ($ENV{DISABLE_STRIP} eq ""))) {
-        $strip .= 'guw ' if ($^O eq 'cygwin');
-        $strip .= 'strip';
-        $strip .= " -x" if ($ENV{OS} eq 'MACOSX');
-        $strip .= " -R '.comment' -s" if ($ENV{OS} eq 'LINUX');
-    };
-};
-
 sub is_jar {
     my $file_name = shift;
 
@@ -699,15 +655,6 @@ sub execute_system {
     };
 };
 
-sub strip_target {
-    my $file = shift;
-    my $temp_file = shift;
-    $temp_file =~ s/\/{2,}/\//g;
-    my $rc = copy($file, $temp_file);
-    execute_system("$strip $temp_file");
-    return $rc;
-};
-
 sub copy_if_newer
 {
     # return 0 if file is unchanged ( for whatever reason )
@@ -752,11 +699,7 @@ sub copy_if_newer
     # to minimize the possibility for race conditions
     local $temp_file = sprintf('%s.%d-%d', $to, $$, time());
     $rc = '';
-    if (($strip ne '') && (defined $ENV{PROEXT}) && (is_unstripped($from))) {
-        $rc = strip_target($from, $temp_file);
-    } else {
-        $rc = copy($from, $temp_file);
-    };
+    $rc = copy($from, $temp_file);
     if ( $rc) {
         if ( is_newer($temp_file, $from, 0) ) {
             $rc = utime($$from_stat_ref[9], $$from_stat_ref[9], $temp_file);
diff --git a/solenv/bin/modules/installer/globals.pm b/solenv/bin/modules/installer/globals.pm
index fc65622..fed1453 100644
--- a/solenv/bin/modules/installer/globals.pm
+++ b/solenv/bin/modules/installer/globals.pm
@@ -235,7 +235,7 @@ BEGIN
     $mergefiles_added_into_collector = 0;
     $creating_windows_installer_patch = 0;
 
-    $strip = 1;
+    $strip = 0;
 
     $logfilename = "logfile.log";   # the default logfile name for global errors
     @logfileinfo = ();
diff --git a/solenv/bin/modules/installer/parameter.pm b/solenv/bin/modules/installer/parameter.pm
index e84dbdc..21b2776 100644
--- a/solenv/bin/modules/installer/parameter.pm
+++ b/solenv/bin/modules/installer/parameter.pm
@@ -70,7 +70,7 @@ The following parameter are needed:
 -helppack : do create a helppack, no product pack (optional)
 -patch : do create a patch (optional)
 -patchinc: Source for the patch include files (Solaris only)
--dontstrip: No file stripping (Unix only)
+-strip: Stripping files (Unix only)
 -log : Logging all available information (optional)
 
 Examples for Windows:
@@ -148,7 +148,7 @@ sub getparameter
         elsif ($param eq "-helppack") { $installer::globals::helppack = 1;}
         elsif ($param eq "-patch") { $installer::globals::patch = 1; }
         elsif ($param eq "-debian") { $installer::globals::debian = 1; }
-        elsif ($param eq "-dontstrip") { $installer::globals::strip = 0; }
+        elsif ($param eq "-strip") { $installer::globals::strip = 1; }
         elsif ($param eq "-destdir")    # new parameter for simple installer
         {
             $installer::globals::rootpath ne "" && die "must set destdir before -i or -simple";
diff --git a/solenv/bin/ooinstall b/solenv/bin/ooinstall
index f1866c6..6f64f7b 100755
--- a/solenv/bin/ooinstall
+++ b/solenv/bin/ooinstall
@@ -101,12 +101,6 @@ if ( defined $ENV{DESTDIR} &&
     $destdir = "-destdir \"$ENV{DESTDIR}\"";
 }
 
-$strip='';
-if ( defined $ENV{DISABLE_STRIP} &&
-     $ENV{DISABLE_STRIP} eq "TRUE" ) {
-    $strip = "-dontstrip";
-}
-
 $msi='';
 if ($is_windows) {
     $msi = "-msitemplate 
$ENV{SRC_ROOT}/instsetoo_native/$ENV{INPATH}/misc/openoffice/msi_templates " .
diff --git a/solenv/gbuild/gbuild.mk b/solenv/gbuild/gbuild.mk
index 7d1d66e..f8413f6 100644
--- a/solenv/gbuild/gbuild.mk
+++ b/solenv/gbuild/gbuild.mk
@@ -85,12 +85,6 @@ else
 gb_SYMBOL := $(false)
 endif
 
-ifeq ($(strip $(DISABLE_STRIP)$(disable_strip)),)
-gb_STRIP := $(true)
-else
-gb_STRIP := $(false)
-endif
-
 gb_TIMELOG := 0
 ifneq ($(strip $(TIMELOG)$(timelog)),)
 gb_TIMELOG := 1
diff --git a/solenv/gbuild/platform/solaris.mk b/solenv/gbuild/platform/solaris.mk
index 779ccf5..04ec93e 100644
--- a/solenv/gbuild/platform/solaris.mk
+++ b/solenv/gbuild/platform/solaris.mk
@@ -167,12 +167,6 @@ gb_LinkTarget_LDFLAGS += \
 
 endif
 
-ifneq ($(gb_SYMBOL),$(true))
-ifeq ($(gb_STRIP),$(true))
-gb_LinkTarget_LDFLAGS += -Wl,--strip-all
-endif
-endif
-
 ifneq ($(gb_DEBUGLEVEL),0)
 gb_COMPILEROPTFLAGS := -O0
 gb_LINKEROPTFLAGS :=
diff --git a/solenv/gbuild/platform/unxgcc.mk b/solenv/gbuild/platform/unxgcc.mk
index 387b583..d288d63 100644
--- a/solenv/gbuild/platform/unxgcc.mk
+++ b/solenv/gbuild/platform/unxgcc.mk
@@ -167,12 +167,6 @@ gb_LinkTarget_LDFLAGS += \
 
 endif
 
-ifneq ($(gb_SYMBOL),$(true))
-ifeq ($(gb_STRIP),$(true))
-gb_LinkTarget_LDFLAGS += -Wl,--strip-all
-endif
-endif
-
 ifneq ($(gb_DEBUGLEVEL),0)
 gb_COMPILEROPTFLAGS := -O0
 gb_LINKEROPTFLAGS :=
-- 
1.7.5.4