Re: [PATCH] Uninstall configuration files when RM has spaces
Amos Jeffries has voted approve. Status is now: Approved For details, see: http://bundlebuggy.aaronbentley.com/project/squid/request/%3C4A327FC0.7090709%40measurement-factory.com%3E Project: Squid
Re: [PATCH] Uninstall configuration files when RM has spaces
On 06/12/2009 04:20 PM, Amos Jeffries wrote: > Alex Rousskov wrote: >> Hello, >> >> Make distuninstallcheck may fail because when we pass $(RM) to >> scripts/remove-cfg.sh, the variable value is passed as two parameters: >> rm and -f. The script expects it to be a single parameter. >> >> The earlier patch discussed below worked at the time of commit but >> something has changed in my build environment or Squid: either $(RM) did >> not have spaces before or those spaces were escaped automatically. My >> understanding is that the attached patch would work in either case. > > Correct. We use identical quoting for other scripts. > > Please commit. Committed to trunk. Please commit to v3.1 or let me know if I should (I do not know if you have already closed v3.1 for direct commits like v3.0 has been closed). Thank you, Alex. >> -- change log - >> Make distuninstallcheck work when $(RM) contains spaces (e.g., rm -f). >> >> We must quote $(RM) value when passing it to scripts/remove-cfg.sh >> The bug was detected by running test-builds.sh. >> --- >> >> Thank you, >> >> Alex. >> bb:approve >> >> >> On 02/27/2009 04:43 AM, Amos Jeffries wrote: >>> Alex Rousskov wrote: Hello, Please review the attached patch. The patched Makefiles delete installed configuration files, restoring functionality removed from Squid some time ago, but hopefully in a safer manner. This is the cleanest way I could find to make "make distcheck" work again (with the other pending SourceLayout changes). change log -- Made "make distuninstallcheck" work: Fixed "make distuninstallcheck" by removing installed configuration files iff they are identical to the installed default configuration files. Added scripts/remove-cfg.sh to do the safe removal because we need that functionality in many Makefiles. Made installed mime.conf removal safe. We were removing it without checking for modifications. Added commands to remove the following installed default configuration files: cachemgr.conf.default and msntauth.conf.default. Thank you, Alex. >>> Works for me too. I still have trouble with the translations, but am >>> working on that now. >>> >>> Amos >> > > Amos
Re: [PATCH] Uninstall configuration files when RM has spaces
Alex Rousskov wrote: Hello, Make distuninstallcheck may fail because when we pass $(RM) to scripts/remove-cfg.sh, the variable value is passed as two parameters: rm and -f. The script expects it to be a single parameter. The earlier patch discussed below worked at the time of commit but something has changed in my build environment or Squid: either $(RM) did not have spaces before or those spaces were escaped automatically. My understanding is that the attached patch would work in either case. Correct. We use identical quoting for other scripts. Please commit. -- change log - Make distuninstallcheck work when $(RM) contains spaces (e.g., rm -f). We must quote $(RM) value when passing it to scripts/remove-cfg.sh The bug was detected by running test-builds.sh. --- Thank you, Alex. bb:approve On 02/27/2009 04:43 AM, Amos Jeffries wrote: Alex Rousskov wrote: Hello, Please review the attached patch. The patched Makefiles delete installed configuration files, restoring functionality removed from Squid some time ago, but hopefully in a safer manner. This is the cleanest way I could find to make "make distcheck" work again (with the other pending SourceLayout changes). change log -- Made "make distuninstallcheck" work: Fixed "make distuninstallcheck" by removing installed configuration files iff they are identical to the installed default configuration files. Added scripts/remove-cfg.sh to do the safe removal because we need that functionality in many Makefiles. Made installed mime.conf removal safe. We were removing it without checking for modifications. Added commands to remove the following installed default configuration files: cachemgr.conf.default and msntauth.conf.default. Thank you, Alex. Works for me too. I still have trouble with the translations, but am working on that now. Amos Amos -- Please be using Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1
Re: [PATCH] Uninstall configuration files when RM has spaces
Bundle Buggy has detected this merge request. For details, see: http://bundlebuggy.aaronbentley.com/project/squid/request/%3C4A327FC0.7090709%40measurement-factory.com%3E Project: Squid
[PATCH] Uninstall configuration files when RM has spaces
Hello, Make distuninstallcheck may fail because when we pass $(RM) to scripts/remove-cfg.sh, the variable value is passed as two parameters: rm and -f. The script expects it to be a single parameter. The earlier patch discussed below worked at the time of commit but something has changed in my build environment or Squid: either $(RM) did not have spaces before or those spaces were escaped automatically. My understanding is that the attached patch would work in either case. -- change log - Make distuninstallcheck work when $(RM) contains spaces (e.g., rm -f). We must quote $(RM) value when passing it to scripts/remove-cfg.sh The bug was detected by running test-builds.sh. --- Thank you, Alex. bb:approve On 02/27/2009 04:43 AM, Amos Jeffries wrote: > Alex Rousskov wrote: >> Hello, >> >> Please review the attached patch. The patched Makefiles delete >> installed configuration files, restoring functionality removed from >> Squid some time ago, but hopefully in a safer manner. This is the >> cleanest way I could find to make "make distcheck" work again (with the >> other pending SourceLayout changes). >> >> change log -- >> Made "make distuninstallcheck" work: >> >> Fixed "make distuninstallcheck" by removing installed configuration >> files iff they are identical to the installed default configuration >> files. >> >> Added scripts/remove-cfg.sh to do the safe removal because we need that >> functionality in many Makefiles. >> >> Made installed mime.conf removal safe. We were removing it without >> checking for modifications. >> >> Added commands to remove the following installed default configuration >> files: cachemgr.conf.default and msntauth.conf.default. >> >> >> Thank you, >> >> Alex. >> > Works for me too. I still have trouble with the translations, but am > working on that now. > > Amos Make distuninstallcheck work when $(RM) contains spaces (e.g., rm -f). We must quote $(RM) value when passing it to scripts/remove-cfg.sh The bug was detected by running test-builds.sh. === modified file 'errors/Makefile.am' --- errors/Makefile.am 2009-06-12 05:07:44 + +++ errors/Makefile.am 2009-06-12 16:24:44 + @@ -98,7 +98,7 @@ done; \ fi \ done; - @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh $(RM) $(DESTDIR)$(DEFAULT_STYLESHEET) + @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh "$(RM)" $(DESTDIR)$(DEFAULT_STYLESHEET) rm -f $(DESTDIR)$(DEFAULT_STYLESHEET).default # undocumented hack. You can use this target to create multi-lingual === modified file 'helpers/basic_auth/MSNT/Makefile.am' --- helpers/basic_auth/MSNT/Makefile.am 2009-04-23 09:52:10 + +++ helpers/basic_auth/MSNT/Makefile.am 2009-06-12 16:24:44 + @@ -47,6 +47,6 @@ fi uninstall-local: - @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh $(RM) $(DESTDIR)$(MSNTAUTH_CONF) + @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh "$(RM)" $(DESTDIR)$(MSNTAUTH_CONF) $(RM) -f $(DESTDIR)$(MSNTAUTH_CONF).default === modified file 'src/Makefile.am' --- src/Makefile.am 2009-06-01 12:31:43 + +++ src/Makefile.am 2009-06-12 16:24:44 + @@ -790,8 +790,8 @@ $(mkinstalldirs) $(DESTDIR)$(DEFAULT_LOG_PREFIX) uninstall-local: - @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh $(RM) $(DESTDIR)$(DEFAULT_MIME_TABLE) - @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh $(RM) $(DESTDIR)$(DEFAULT_CONFIG_FILE) + @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh "$(RM)" $(DESTDIR)$(DEFAULT_MIME_TABLE) + @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh "$(RM)" $(DESTDIR)$(DEFAULT_CONFIG_FILE) CLEANFILES += cf_gen_defines.h cf.data cf_parser.h squid.conf.default squid.conf.documented \ globals.cc string_arrays.c repl_modules.cc DiskIO/DiskIOModules_gen.cc \ === modified file 'tools/Makefile.am' --- tools/Makefile.am 2009-04-23 09:52:10 + +++ tools/Makefile.am 2009-06-12 16:24:44 + @@ -47,7 +47,7 @@ fi uninstall-local: - @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh $(RM) $(DESTDIR)$(DEFAULT_CACHEMGR_CONFIG) + @$(SHELL) $(top_srcdir)/scripts/remove-cfg.sh "$(RM)" $(DESTDIR)$(DEFAULT_CACHEMGR_CONFIG) $(RM) -f $(DESTDIR)$(DEFAULT_CACHEMGR_CONFIG).default DISTCLEANFILES =
