[PATCH 0/5] Various setup patches

[PATCH 0/5] Various setup patches

[Jon TURNEY]

A smorgasbord of setup patches for your delectation

Jon TURNEY (5):
  Restore and fix up the rules for building and uploading snapshots
  Allow setup to parse more than 3 versions from the setup.ini file
  Report overall progress while md5summing packages
  Update progress display when download phase starts
  Add a self-update mechanism for setup.exe

 Makefile.am                                     |   17 ++++
 PackageTrust.h                                  |    2 +-
 download.cc                                     |    4 +
 ini.cc                                          |   78 ++++++++++++++--
 inilex.ll                                       |    1 +
 iniparse.yy                                     |    3 +-
 install.cc                                      |   27 ++++++
 libgetopt++/include/getopt++/DefaultFormatter.h |    6 +-
 main.cc                                         |  115 ++++++++++++++++++++++-
 res.rc                                          |    3 +-
 resource.h                                      |    1 +
 threebar.cc                                     |   14 +++-
 threebar.h                                      |    1 +
 13 files changed, 256 insertions(+), 16 deletions(-)

-- 
1.7.4

[PATCH 2/5] Allow setup to parse more than 3 versions from the setup.ini file

[Jon TURNEY]

This recognizes any "[version]" line as introducing the information for
another version, which doesn't have one of the trust levels [curr], [prev]
or [test], and so isn't automatically selected when setup is

Since the value of <version> carries no meaning, it might make more sense
to mandate the use of a specific string like "[also]" or "[other]", or
perhaps "[prev-1]", "[prev-2]", etc.

I have written a corresponding patch to genini

Setup already does all the neccessary sorting in version order etc. to
use these additional versions.

2011-04-24  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* PackageTrust.h (trusts): Add TRUST_OTHER.
	* inilex.ll: tokenize any other [version] as the T_OTHER token.
	* iniparse.yy: Add T_OTHER token and set package trust
	to TRUST_OTHER when it is used.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 PackageTrust.h |    2 +-
 inilex.ll      |    1 +
 iniparse.yy    |    3 ++-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/PackageTrust.h b/PackageTrust.h
index 3e6801d..4904705 100644
--- a/PackageTrust.h
+++ b/PackageTrust.h
@@ -20,10 +20,10 @@
 typedef enum
 {
   TRUST_UNKNOWN,
+  TRUST_OTHER,
   TRUST_PREV,
   TRUST_CURR,
   TRUST_TEST,
-  NTRUST
 }
 trusts;
 
diff --git a/inilex.ll b/inilex.ll
index b119049..5dfbe3b 100644
--- a/inilex.ll
+++ b/inilex.ll
@@ -116,6 +116,7 @@ STR	[!a-zA-Z0-9_./:\+~-]+
 "[test]"		return T_TEST;
 "[exp]"			return T_TEST;
 "[prev]"		return T_PREV;
+"["{STR}"]"		return T_OTHER;
 
 "("			return OPENBRACE;
 ")"			return CLOSEBRACE;
diff --git a/iniparse.yy b/iniparse.yy
index c8332ff..cd2d66e 100644
--- a/iniparse.yy
+++ b/iniparse.yy
@@ -41,7 +41,7 @@ void add_correct_version();
 %token SETUP_TIMESTAMP SETUP_VERSION PACKAGEVERSION INSTALL SOURCE SDESC LDESC
 %token CATEGORY DEPENDS REQUIRES
 %token APATH PPATH INCLUDE_SETUP EXCLUDE_PACKAGE DOWNLOAD_URL
-%token T_PREV T_CURR T_TEST
+%token T_PREV T_CURR T_TEST T_OTHER
 %token MD5 INSTALLEDSIZE MAINTAINER PRIORITY
 %token DESCTAG DESCRIPTION FILESIZE ARCHITECTURE SOURCEPACKAGE MD5LINE 
 %token RECOMMENDS PREDEPENDS
@@ -95,6 +95,7 @@ singleitem /* non-empty */
  | T_PREV NL 			{ iniBuilder->buildPackageTrust (TRUST_PREV); }
  | T_CURR NL			{ iniBuilder->buildPackageTrust (TRUST_CURR); }
  | T_TEST NL			{ iniBuilder->buildPackageTrust (TRUST_TEST); }
+ | T_OTHER NL			{ iniBuilder->buildPackageTrust (TRUST_OTHER); }
  | PRIORITY STRING NL		{ iniBuilder->buildPriority ($2); }
  | INSTALLEDSIZE STRING NL	{ iniBuilder->buildInstalledSize ($2); }
  | MAINTAINER STRING NL		{ iniBuilder->buildMaintainer ($2); }
-- 
1.7.4

Re: [PATCH 2/5] Allow setup to parse more than 3 versions from the setup.ini file

[Christopher Faylor]

On Sun, Apr 24, 2011 at 02:05:54PM +0100, Jon TURNEY wrote:
>This recognizes any "[version]" line as introducing the information for
>another version, which doesn't have one of the trust levels [curr], [prev]
>or [test], and so isn't automatically selected when setup is
>
>Since the value of <version> carries no meaning, it might make more sense
>to mandate the use of a specific string like "[also]" or "[other]", or
>perhaps "[prev-1]", "[prev-2]", etc.
>
>I have written a corresponding patch to genini
>
>Setup already does all the neccessary sorting in version order etc. to
>use these additional versions.
>
>2011-04-24  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* PackageTrust.h (trusts): Add TRUST_OTHER.
>	* inilex.ll: tokenize any other [version] as the T_OTHER token.
>	* iniparse.yy: Add T_OTHER token and set package trust
>	to TRUST_OTHER when it is used.

Sorry but what is this good for?  Who would use it?

cgf

Re: [PATCH 2/5] Allow setup to parse more than 3 versions from the setup.ini file

[Jon TURNEY]

On 24/04/2011 17:10, Christopher Faylor wrote:
> On Sun, Apr 24, 2011 at 02:05:54PM +0100, Jon TURNEY wrote:
>> This recognizes any "[version]" line as introducing the information for
>> another version, which doesn't have one of the trust levels [curr], [prev]
>> or [test], and so isn't automatically selected when setup is
>>
>> Since the value of <version> carries no meaning, it might make more sense
>> to mandate the use of a specific string like "[also]" or "[other]", or
>> perhaps "[prev-1]", "[prev-2]", etc.
>>
>> I have written a corresponding patch to genini
>>
>> Setup already does all the neccessary sorting in version order etc. to
>> use these additional versions.
>>
>> 2011-04-24  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>
>> 	* PackageTrust.h (trusts): Add TRUST_OTHER.
>> 	* inilex.ll: tokenize any other [version] as the T_OTHER token.
>> 	* iniparse.yy: Add T_OTHER token and set package trust
>> 	to TRUST_OTHER when it is used.
> 
> Sorry but what is this good for?  Who would use it?

If you are asking why a package maintainer might want this, then for e.g. [1]

[1] http://cygwin.com/ml/cygwin-xfree/2011-03/msg00049.html

Re: [PATCH 2/5] Allow setup to parse more than 3 versions from the setup.ini file

[Christopher Faylor]

On Fri, Apr 29, 2011 at 03:23:31PM +0100, Jon TURNEY wrote:
>On 24/04/2011 17:10, Christopher Faylor wrote:
>> On Sun, Apr 24, 2011 at 02:05:54PM +0100, Jon TURNEY wrote:
>>> This recognizes any "[version]" line as introducing the information for
>>> another version, which doesn't have one of the trust levels [curr], [prev]
>>> or [test], and so isn't automatically selected when setup is
>>>
>>> Since the value of <version> carries no meaning, it might make more sense
>>> to mandate the use of a specific string like "[also]" or "[other]", or
>>> perhaps "[prev-1]", "[prev-2]", etc.
>>>
>>> I have written a corresponding patch to genini
>>>
>>> Setup already does all the neccessary sorting in version order etc. to
>>> use these additional versions.
>>>
>>> 2011-04-24  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>>
>>> 	* PackageTrust.h (trusts): Add TRUST_OTHER.
>>> 	* inilex.ll: tokenize any other [version] as the T_OTHER token.
>>> 	* iniparse.yy: Add T_OTHER token and set package trust
>>> 	to TRUST_OTHER when it is used.
>> 
>> Sorry but what is this good for?  Who would use it?
>
>If you are asking why a package maintainer might want this, then for e.g. [1]
>
>[1] http://cygwin.com/ml/cygwin-xfree/2011-03/msg00049.html

Ok, there's one package maintainer who wants it.

Since this is, IMO, a rather large departure from past convention, it would
be nice to see if this is something that other people want.

This change will mean I'll need change to the program which generates
setup.ini so I'd like to make sure that this is going to be used.

cgf

Re: [PATCH 2/5] Allow setup to parse more than 3 versions from the setup.ini file

[Reini Urban]

2011/4/29 Christopher Faylor:
> On Fri, Apr 29, 2011 at 03:23:31PM +0100, Jon TURNEY wrote:
>>On 24/04/2011 17:10, Christopher Faylor wrote:
>>> On Sun, Apr 24, 2011 at 02:05:54PM +0100, Jon TURNEY wrote:
>>>> This recognizes any "[version]" line as introducing the information for
>>>> another version, which doesn't have one of the trust levels [curr], [prev]
>>>> or [test], and so isn't automatically selected when setup is
>>>>
>>>> Since the value of <version> carries no meaning, it might make more sense
>>>> to mandate the use of a specific string like "[also]" or "[other]", or
>>>> perhaps "[prev-1]", "[prev-2]", etc.
>>>>
>>>> I have written a corresponding patch to genini
>>>>
>>>> Setup already does all the neccessary sorting in version order etc. to
>>>> use these additional versions.
>>>>
>>>> 2011-04-24  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>>>
>>>>     * PackageTrust.h (trusts): Add TRUST_OTHER.
>>>>     * inilex.ll: tokenize any other [version] as the T_OTHER token.
>>>>     * iniparse.yy: Add T_OTHER token and set package trust
>>>>     to TRUST_OTHER when it is used.
>>>
>>> Sorry but what is this good for?  Who would use it?
>>
>>If you are asking why a package maintainer might want this, then for e.g. [1]
>>
>>[1] http://cygwin.com/ml/cygwin-xfree/2011-03/msg00049.html
>
> Ok, there's one package maintainer who wants it.
>
> Since this is, IMO, a rather large departure from past convention, it would
> be nice to see if this is something that other people want.
>
> This change will mean I'll need change to the program which generates
> setup.ini so I'd like to make sure that this is going to be used.

I am in strong favor of this new feature.

-- 
Reini Urban
http://phpwiki.org/           http://murbreak.at/

Re: [PATCH 2/5] Allow setup to parse more than 3 versions from the setup.ini file

[Christopher Faylor]

On Sat, Apr 30, 2011 at 12:12:19PM +0200, Reini Urban wrote:
>2011/4/29 Christopher Faylor:
>> On Fri, Apr 29, 2011 at 03:23:31PM +0100, Jon TURNEY wrote:
>>>On 24/04/2011 17:10, Christopher Faylor wrote:
>>>> On Sun, Apr 24, 2011 at 02:05:54PM +0100, Jon TURNEY wrote:
>>>>> This recognizes any "[version]" line as introducing the information for
>>>>> another version, which doesn't have one of the trust levels [curr], [prev]
>>>>> or [test], and so isn't automatically selected when setup is
>>>>>
>>>>> Since the value of <version> carries no meaning, it might make more sense
>>>>> to mandate the use of a specific string like "[also]" or "[other]", or
>>>>> perhaps "[prev-1]", "[prev-2]", etc.
>>>>>
>>>>> I have written a corresponding patch to genini
>>>>>
>>>>> Setup already does all the neccessary sorting in version order etc. to
>>>>> use these additional versions.
>>>>>
>>>>> 2011-04-24 ?Jon TURNEY ?<jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>>>>
>>>>> ? ? * PackageTrust.h (trusts): Add TRUST_OTHER.
>>>>> ? ? * inilex.ll: tokenize any other [version] as the T_OTHER token.
>>>>> ? ? * iniparse.yy: Add T_OTHER token and set package trust
>>>>> ? ? to TRUST_OTHER when it is used.
>>>>
>>>> Sorry but what is this good for? ?Who would use it?
>>>
>>>If you are asking why a package maintainer might want this, then for e.g. [1]
>>>
>>>[1] http://cygwin.com/ml/cygwin-xfree/2011-03/msg00049.html
>>
>> Ok, there's one package maintainer who wants it.
>>
>> Since this is, IMO, a rather large departure from past convention, it would
>> be nice to see if this is something that other people want.
>>
>> This change will mean I'll need change to the program which generates
>> setup.ini so I'd like to make sure that this is going to be used.
>
>I am in strong favor of this new feature.

Since I didn't see anyone complain that this is a problem, I'd say go ahead
and check it in Jon.

cgf

[PATCH 4/5] Update progress display when download phase starts

[Jon TURNEY]

If we have many packages installed, but very little to download, don't
leave the last package we checked for prerequisites displayed whilst we
locate the first package to download

2011-04-24  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* download.cc (do_download_thread): Update progress display
	when download phase starts

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 download.cc |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/download.cc b/download.cc
index 25260fd..32bb2c4 100644
--- a/download.cc
+++ b/download.cc
@@ -199,6 +199,10 @@ do_download_thread (HINSTANCE h, HWND owner)
   total_download_bytes = 0;
   total_download_bytes_sofar = 0;
 
+  Progress.SetText1 ("Checking for packages to download...");
+  Progress.SetText2 ("");
+  Progress.SetText3 ("");
+
   packagedb db;
   /* calculate the amount needed */
   for (packagedb::packagecollection::iterator i = db.packages.begin ();
-- 
1.7.4

Re: [PATCH 4/5] Update progress display when download phase starts

[Christopher Faylor]

On Sun, Apr 24, 2011 at 02:05:56PM +0100, Jon TURNEY wrote:
>If we have many packages installed, but very little to download, don't
>leave the last package we checked for prerequisites displayed whilst we
>locate the first package to download
>
>2011-04-24  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* download.cc (do_download_thread): Update progress display
>	when download phase starts

Looks good.  Please apply.  Thanks.

cgf

[PATCH 3/5] Report overall progress while md5summing packages

[Jon TURNEY]

md5sum checking all packages can take minutes with a large number of
packages to install, so report not only progress on the checking for
each individual package, but how far we are through the operation on
all packages

2011-04-24  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* install.cc (do_install_thread): Report overall progress while
	md5summing packages.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 install.cc |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/install.cc b/install.cc
index a1337da..68bf116 100644
--- a/install.cc
+++ b/install.cc
@@ -628,6 +628,29 @@ do_install_thread (HINSTANCE h, HWND owner)
   vector <packagemeta *> install_q, uninstall_q, sourceinstall_q;
 
   packagedb db;
+
+  /* Calculate the amount of data to md5sum */
+  Progress.SetText1("Calculating...");
+  int md5sum_total_bytes = 0;
+  for (packagedb::packagecollection::iterator i = db.packages.begin ();
+       i != db.packages.end (); ++i)
+  {
+    packagemeta & pkg = *(i->second);
+
+    if (pkg.desired.picked())
+    {
+      md5sum_total_bytes += pkg.desired.source()->size;
+    }
+
+    if (pkg.desired.sourcePackage ().picked())
+    {
+      md5sum_total_bytes += pkg.desired.sourcePackage ().source()->size;
+    }
+  }
+
+  /* md5sum the packages, build lists of packages to install and uninstall
+     and calculate the total amount of data to install */
+  int md5sum_total_bytes_sofar = 0;
   for (packagedb::packagecollection::iterator i = db.packages.begin ();
        i != db.packages.end (); ++i)
   {
@@ -646,6 +669,7 @@ do_install_thread (HINSTANCE h, HWND owner)
       }
       if (pkg.desired.picked())
       {
+        md5sum_total_bytes_sofar += pkg.desired.source()->size;
         total_bytes += pkg.desired.source()->size;
         install_q.push_back (&pkg);
       }
@@ -664,6 +688,7 @@ do_install_thread (HINSTANCE h, HWND owner)
       }
       if (pkg.desired.sourcePackage().picked())
       {
+        md5sum_total_bytes_sofar += pkg.desired.sourcePackage ().source()->size;
         total_bytes += pkg.desired.sourcePackage ().source()->size;
         sourceinstall_q.push_back (&pkg);
       }
@@ -674,6 +699,8 @@ do_install_thread (HINSTANCE h, HWND owner)
     {
       uninstall_q.push_back (&pkg);
     }
+
+    Progress.SetBar2 (md5sum_total_bytes_sofar, md5sum_total_bytes);
   }
 
   /* start with uninstalls - remove files that new packages may replace */
-- 
1.7.4

Re: [PATCH 3/5] Report overall progress while md5summing packages

[Christopher Faylor]

On Sun, Apr 24, 2011 at 02:05:55PM +0100, Jon TURNEY wrote:
>md5sum checking all packages can take minutes with a large number of
>packages to install, so report not only progress on the checking for
>each individual package, but how far we are through the operation on
>all packages
>
>2011-04-24  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* install.cc (do_install_thread): Report overall progress while
>	md5summing packages.

Looks good.  Please install.

cgf

[PATCH] Fix a cosmetic problem when there are no packages to install

[Jon TURNEY]

My previous patch introduces a cosmetic problem when there are no packages
to install.

Calling ThreeBarProgressPage::SetBar2(0, 0) indicates we are -2147483648%
done, while we iterate over all packages looking for packages to md5sum.

So avoid calling SetBar2() in that case.

2011-07-21  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* install.cc (do_install_thread): Fix cosmetic problem when
	there are no packages to install.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 install.cc |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/install.cc b/install.cc
index 9f13d7b..882c0e5 100644
--- a/install.cc
+++ b/install.cc
@@ -700,7 +700,8 @@ do_install_thread (HINSTANCE h, HWND owner)
       uninstall_q.push_back (&pkg);
     }
 
-    Progress.SetBar2 (md5sum_total_bytes_sofar, md5sum_total_bytes);
+    if (md5sum_total_bytes > 0)
+      Progress.SetBar2 (md5sum_total_bytes_sofar, md5sum_total_bytes);
   }
 
   /* start with uninstalls - remove files that new packages may replace */
-- 
1.7.5.1

Re: [PATCH] Fix a cosmetic problem when there are no packages to install

[Corinna Vinschen]

On Jul 22 14:02, Jon TURNEY wrote:
> My previous patch introduces a cosmetic problem when there are no packages
> to install.
> 
> Calling ThreeBarProgressPage::SetBar2(0, 0) indicates we are -2147483648%
> done, while we iterate over all packages looking for packages to md5sum.
> 
> So avoid calling SetBar2() in that case.
> 
> 2011-07-21  Jon TURNEY  <jon.turney@dronecode.org.uk>
> 
> 	* install.cc (do_install_thread): Fix cosmetic problem when
> 	there are no packages to install.

Go ahead.  Just change your ChangeLog entry to something more
descriptive, like, say, "Only set progress bar output if at least
one package gets installed."


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix a cosmetic problem when there are no packages to install

[Jon TURNEY]

On 22/07/2011 15:12, Corinna Vinschen wrote:
> On Jul 22 14:02, Jon TURNEY wrote:
>> My previous patch introduces a cosmetic problem when there are no packages
>> to install.
>>
>> Calling ThreeBarProgressPage::SetBar2(0, 0) indicates we are -2147483648%
>> done, while we iterate over all packages looking for packages to md5sum.
>>
>> So avoid calling SetBar2() in that case.
>>
>> 2011-07-21  Jon TURNEY<jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>
>> 	* install.cc (do_install_thread): Fix cosmetic problem when
>> 	there are no packages to install.
>
> Go ahead.  Just change your ChangeLog entry to something more
> descriptive, like, say, "Only set progress bar output if at least
> one package gets installed."

Done, with that correction.

[PATCH 1/5] Restore and fix up the rules for building and uploading snapshots

[Jon TURNEY]

Restore and fix up the rules for building and uploading snapshots
removed in version 2.81 of Makefile.am

2011-03-30  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* Makefile.am (release, snapshot, clean-local): Restore and fix up the
	rules for building and uploading snapshots

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 Makefile.am |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 5cebab9..bd51002 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -293,6 +293,21 @@ setup_version.c : $(srcdir)/ChangeLog Makefile
 .rc.o:
 	$(WINDRES) --include-dir $(srcdir) -o $@ $<
 
+# this target creates:
+#   setup-x.yyy.exe             (UPXed stripped exe)
+#   setup-debug-x.yyy.exe.gz    (gzipped unstripped exe)
+#   setup-x.yyy.tar.bz2         (source)
+release: setup.exe $(srcdir)/ChangeLog Makefile setup-src
+	cp -pf setup.exe setup-debug-${VER}.exe
+	gzip -f9 setup-debug-${VER}.exe
+	cp -pf setup.exe setup-${VER}.exe
+	$(STRIP) setup-${VER}.exe
+	upx --best setup-${VER}.exe
+
+# Create a snapshot and upload it (requires write access)
+snapshot: release
+	scp -C setup-${VER}.exe setup-debug-${VER}.exe.gz setup-${VER}-src.tar.bz2 $${cygwinsite:-cygwin.com:/var/www/sourceware/htdocs/cygwin/setup/snapshots/}
+
 # static const char version_store[] = VERSION_PREFIX " 2.686";
 setup-src:
 	@ver=setup-$$(sed -n 's/^static const char version_store.* VERSION_PREFIX " \([^"]*\)".*$$/\1/p' setup_version.c);\
@@ -303,3 +318,5 @@ setup-src:
 	sort | tar -T - -cjf ${CURDIR}/$$ver-src.tar.bz2;\
 	echo $$ver-src.tar.bz2; exec rm -f $$ver
 
+clean-local:
+	rm -f setup-*.exe setup-debug-*.exe.gz setup-*-src.tar.bz2
-- 
1.7.4

Re: [PATCH 1/5] Restore and fix up the rules for building and uploading snapshots

[Christopher Faylor]

On Sun, Apr 24, 2011 at 02:05:53PM +0100, Jon TURNEY wrote:
>Restore and fix up the rules for building and uploading snapshots
>removed in version 2.81 of Makefile.am
>
>2011-03-30  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* Makefile.am (release, snapshot, clean-local): Restore and fix up the
>	rules for building and uploading snapshots

See Change #2.81 (which you apparently already know about).

I don't think setup.exe snapshots are very useful.  I never see much
indication that anyone uses them which is why I removed the code to
begin with.  I'd rather not raise setup.exe to the level of a "product"
which we have to beta test.

cgf

Re: [PATCH 1/5] Restore and fix up the rules for building and uploading snapshots

[Jon TURNEY]

On 24/04/2011 17:13, Christopher Faylor wrote:
> On Sun, Apr 24, 2011 at 02:05:53PM +0100, Jon TURNEY wrote:
>> Restore and fix up the rules for building and uploading snapshots
>> removed in version 2.81 of Makefile.am
>>
>> 2011-03-30  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>
>> 	* Makefile.am (release, snapshot, clean-local): Restore and fix up the
>> 	rules for building and uploading snapshots
> 
> See Change #2.81 (which you apparently already know about).

I guess I misunderstood the check-in comment to mean "Remove these rules
because they have been broken by other changes" rather than "Remove these
rules because testing is unnecessary" :-)

> I don't think setup.exe snapshots are very useful.  I never see much
> indication that anyone uses them which is why I removed the code to
> begin with.  I'd rather not raise setup.exe to the level of a "product"
> which we have to beta test.

I wrote a lengthy response here, but it's your decision, so, patch withdrawn.

Re: [PATCH 1/5] Restore and fix up the rules for building and uploading snapshots

[Christopher Faylor]

On Fri, Apr 29, 2011 at 03:23:35PM +0100, Jon TURNEY wrote:
>On 24/04/2011 17:13, Christopher Faylor wrote:
>> On Sun, Apr 24, 2011 at 02:05:53PM +0100, Jon TURNEY wrote:
>>> Restore and fix up the rules for building and uploading snapshots
>>> removed in version 2.81 of Makefile.am
>>>
>>> 2011-03-30  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>>
>>> 	* Makefile.am (release, snapshot, clean-local): Restore and fix up the
>>> 	rules for building and uploading snapshots
>> 
>>See Change #2.81 (which you apparently already know about).
>
>I guess I misunderstood the check-in comment to mean "Remove these
>rules because they have been broken by other changes" rather than
>"Remove these rules because testing is unnecessary" :-)
>
>>I don't think setup.exe snapshots are very useful.  I never see much
>>indication that anyone uses them which is why I removed the code to
>>begin with.  I'd rather not raise setup.exe to the level of a "product"
>>which we have to beta test.
>
>I wrote a lengthy response here, but it's your decision, so, patch
>withdrawn.

FYI, I had a lengthy response to the tone of this message but I decided not
to send it.

I guess there's a lot of that going around.

cgf

[PATCH 5/5] Add a self-update mechanism for setup.exe

[Jon TURNEY]

Updating setup.exe has 3 stages:
1) Download updated setup.exe to a temporary location
2) Execute that temporary copy of setup.exe with --copy-to instructing
it to copy itself over the setup.exe to be updated
3) Execute the updated setup.exe with --remove-from instructing it
to delete the temporary copy

A named mutex is used to ensure setup exits from each stage before
the next stage can start.

Unfortunately, at the moment, we don't usefully check the setup version number
until after we have downloaded and parsed setup.ini, which is perhaps a bit
late to offer to update setup.exe

v2: Address comments from Dave Korn
Properly quote arguments to ensure spaces in paths are handled safely
Place the setup URL in a string resouce

2011-03-29  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* res.rc (IDS_OLD_SETUP_VERSION): Change text to offer to download
	new version of setup.
	(IDS_SETUP_URL): Added string resource.
	* resource.h (IDS_SETUP_URL): Added resource identifier.
	* main.cc (wait_for_exit, self_update_remove_from,
	self_update_copy_to, WinMain): Add -copy-to and --remove-from options
	for self-update process.
	* ini.cc (self_update_download): New function to download updated
	setup.
	(do_ini_thread): Prompt to download updated setup if a newer
	version exists.  Return a result indicating what should happen
	next.
	* threebar.h (PropertyPage::OnFinish): Add an implementation of
	OnFinish virtual function for this class.
	* threebar.cc (OnFinish): Ditto.
	 (OnMessageApp): Setup should finish on WM_APP_SETUP_INI_DOWNLOAD_COMPLETE
	if an updated setup was downloaded.

2011-03-29  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* include/getopt++/DefaultFormatter.h (DefaultFormatter): Fix option string
	formatting when it has no short option.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 ini.cc                                          |   78 ++++++++++++++--
 libgetopt++/include/getopt++/DefaultFormatter.h |    6 +-
 main.cc                                         |  115 ++++++++++++++++++++++-
 res.rc                                          |    3 +-
 resource.h                                      |    1 +
 threebar.cc                                     |   14 +++-
 threebar.h                                      |    1 +
 7 files changed, 204 insertions(+), 14 deletions(-)

diff --git a/ini.cc b/ini.cc
index d99bc60..8b33e68 100644
--- a/ini.cc
+++ b/ini.cc
@@ -285,7 +285,68 @@ do_remote_ini (HWND owner)
   return ini_count;
 }
 
-static bool
+static int
+self_update_download(HWND owner)
+{
+  /* Get the download URL string resource */
+  char setup_url[1000];
+  if (LoadString (GetModuleHandle(NULL), IDS_SETUP_URL, setup_url, sizeof(setup_url)) <= 0)
+    {
+      MessageBox (owner, "Unable to determine download URL.", "Cygwin Setup", MB_OK);
+      return IDD_CHOOSE;
+    }
+
+  /* Download new setup to a temporary location */
+  std::string downloadedSetup = local_dir + "\\setup.tmp.exe";
+  if (get_url_to_file (setup_url, downloadedSetup, 0, owner))
+    {
+      MessageBox (owner, "Downloading updated setup.exe failed.", "Cygwin Setup", MB_OK);
+      return IDD_CHOOSE;
+    }
+
+  /* Create and claim a mutex so the new version invoked with --copy-to doesn't
+     try to overwrite us before we've exited */
+  HANDLE mutex = CreateMutex(NULL, TRUE, "Global\\Cygwin.Setup");
+  if (!mutex)
+    {
+      log (LOG_PLAIN) << "CreateMutex failed :" << GetLastError () << endLog;
+    }
+
+  /* Invoke setup with same options + --copy-to current location */
+  std::string newArgs = "\"" + downloadedSetup + "\"";
+
+  for (int i = 1; _argv[i]; i++)
+    {
+      newArgs += " \"";
+      newArgs += _argv[i];
+      newArgs += "\"";
+    }
+
+  TCHAR filename[MAX_PATH+1];
+  GetModuleFileName(NULL, filename, MAX_PATH);
+  newArgs += " \"--copy-to=";
+  newArgs += filename;
+  newArgs += "\"";
+
+#ifdef DEBUG
+  log (LOG_BABBLE) << "invoking '" << newArgs << "'" << endLog;
+#endif
+
+  PROCESS_INFORMATION processInfo;
+  STARTUPINFO startupInfo;
+  GetStartupInfo(&startupInfo);
+  if (!CreateProcess(downloadedSetup.c_str(), (char *)(newArgs.c_str()),
+                    NULL, NULL, FALSE, 0, NULL, NULL, &startupInfo, &processInfo) > 0)
+    {
+      MessageBox (owner, "Executing updated setup.exe failed.", "Cygwin Setup", MB_OK);
+      log (LOG_PLAIN) << "CreateProcess failed :" << GetLastError () << endLog;
+      return IDD_CHOOSE;
+    }
+
+  return -1;
+}
+
+static int
 do_ini_thread (HINSTANCE h, HWND owner)
 {
   size_t ini_count = 0;
@@ -295,7 +356,7 @@ do_ini_thread (HINSTANCE h, HWND owner)
     ini_count = do_remote_ini (owner);
 
   if (ini_count == 0)
-    return false;
+    return 0;
 
   if (get_root_dir ().c_str())
     {
@@ -339,11 +400,14 @@ do_ini_thread (HINSTANCE h, HWND owner)
   if (ini_setup_version.size())
     {
       if (version_compare(setup_version, ini_setup_version) < 0)
-	note (owner, IDS_OLD_SETUP_VERSION, setup_version,
-	      ini_setup_version.c_str());
+        if (yesno (owner, IDS_OLD_SETUP_VERSION, setup_version,
+                   ini_setup_version.c_str()) == IDYES)
+          {
+            return self_update_download(owner);
+          }
     }
 
-  return true;
+  return IDD_CHOOSE;
 }
 
 static DWORD WINAPI
@@ -354,10 +418,10 @@ do_ini_thread_reflector(void* p)
 
   try
   {
-    bool succeeded = do_ini_thread((HINSTANCE)context[0], (HWND)context[1]);
+    int nextpage = do_ini_thread((HINSTANCE)context[0], (HWND)context[1]);
 
     // Tell the progress page that we're done downloading
-    Progress.PostMessageNow(WM_APP_SETUP_INI_DOWNLOAD_COMPLETE, 0, succeeded);
+    Progress.PostMessageNow(WM_APP_SETUP_INI_DOWNLOAD_COMPLETE, 0, nextpage);
   }
   TOPLEVEL_CATCH("ini");
 
diff --git a/libgetopt++/include/getopt++/DefaultFormatter.h b/libgetopt++/include/getopt++/DefaultFormatter.h
index ced6956..cbf700b 100644
--- a/libgetopt++/include/getopt++/DefaultFormatter.h
+++ b/libgetopt++/include/getopt++/DefaultFormatter.h
@@ -27,7 +27,11 @@ class DefaultFormatter {
   public:
     DefaultFormatter (std::ostream &aStream) : theStream(aStream) {}
     void operator () (Option *anOption) {
-      std::string output = std::string() + " -" + anOption->shortOption ()[0];
+      std::string output;
+      if (anOption->shortOption()[0] != '\0')
+          output +=  std::string() + " -" + anOption->shortOption ()[0];
+      else
+        output += "   ";
       output += " --" ;
       output += anOption->longOption ();
       output += std::string (40 - output.size(), ' ');
diff --git a/main.cc b/main.cc
index bddff6f..47619ba 100644
--- a/main.cc
+++ b/main.cc
@@ -68,6 +68,7 @@ static const char *cvsid =
 
 #include "getopt++/GetOption.h"
 #include "getopt++/BoolOption.h"
+#include "getopt++/StringOption.h"
 
 #include "Exception.h"
 #include <stdexcept>
@@ -88,6 +89,8 @@ bool is_legacy;
 static BoolOption UnattendedOption (false, 'q', "quiet-mode", "Unattended setup mode");
 static BoolOption PackageManagerOption (false, 'M', "package-manager", "Semi-attended chooser-only mode");
 static BoolOption HelpOption (false, 'h', "help", "print help");
+static StringOption SelfUpdateCopyTo ("", '\0', "copy-to", "Used by self-update.  Copy self to location and invoke.", true);
+static StringOption SelfUpdateRemoveFrom ("", '\0', "remove-from", "Used by self-update.  Remove file after starting.", true);
 static BOOL WINAPI (*dyn_AttachConsole) (DWORD);
 static BOOL WINAPI (*dyn_GetLongPathName) (LPCTSTR, LPTSTR, DWORD);
 
@@ -239,6 +242,91 @@ set_legacy (const char *command)
   is_legacy = strstr (buf, "setup-legacy");
 }
 
+static HANDLE
+wait_for_exit()
+{
+  /* Wait on mutex held by previous instance until it exits, so
+     we don't try to remove it before it's exited */
+  HANDLE mutex = CreateMutex(NULL, FALSE, "Global\\Cygwin.Setup");
+  if (!mutex)
+    {
+      log (LOG_PLAIN) << "CreateMutex failed :" << GetLastError () << endLog;
+    }
+  else
+    {
+      log (LOG_PLAIN) << "Waiting on Mutex" << endLog;
+      WaitForSingleObject(mutex, INFINITE);
+    }
+
+  return mutex;
+}
+
+static void
+self_update_remove_from(const std::string &SelfUpdateRemoveFromString)
+{
+  HANDLE mutex = wait_for_exit();
+
+  log (LOG_PLAIN) << "Removing temporary copy of self '" << SelfUpdateRemoveFromString << "'" << endLog;
+  if (DeleteFile(SelfUpdateRemoveFromString.c_str()) == 0)
+    {
+      log (LOG_PLAIN) << "DeleteFile failed :" << GetLastError () << endLog;
+    }
+
+  ReleaseMutex(mutex);
+}
+
+static void
+self_update_copy_to(const std::string &SelfUpdateCopyToString, int argc)
+{
+  wait_for_exit();
+  /*
+     We hold on to this mutex until we exit, which implicitly releases it, as it's
+     the next instance of ourself should not try to remove us until we have exited
+  */
+
+  log (LOG_PLAIN) << "Copying self to '" << SelfUpdateCopyToString << "'" << endLog;
+
+  /* Copy self to new location */
+  TCHAR filename[MAX_PATH+1];
+  GetModuleFileName(NULL, filename, MAX_PATH);
+  if (CopyFile(filename, SelfUpdateCopyToString.c_str(), FALSE) == 0)
+    {
+      log (LOG_PLAIN) << "CopyFile failed :" << GetLastError () << endLog;
+    }
+
+  /*
+     Now invoke ourself in new location with all the same arguments, except
+     --remove-from this temporary copy, rather than --copy-to the new location
+  */
+  std::string newArgs = "\"" + SelfUpdateCopyToString + "\"";
+
+  for (int i = 1; i < argc; i++)
+    {
+      if (strncmp(_argv[i],"--copy-to",strlen("--copy-to")) != 0)
+        {
+          newArgs += " \"";
+          newArgs += _argv[i];
+          newArgs += "\"";
+        }
+      else
+        {
+          newArgs += " \"--remove-from=";
+          newArgs += filename;
+          newArgs += "\"";
+        }
+    }
+
+#ifdef DEBUG
+  log (LOG_BABBLE) << "invoking '" << newArgs << "'" << endLog;
+#endif
+
+  PROCESS_INFORMATION processInfo;
+  STARTUPINFO startupInfo;
+  GetStartupInfo(&startupInfo);
+  CreateProcess(SelfUpdateCopyToString.c_str(), (char *)newArgs.c_str(),
+                NULL, NULL, FALSE, 0, NULL, NULL, &startupInfo, &processInfo);
+}
+
 #ifndef __CYGWIN__
 int WINAPI
 WinMain (HINSTANCE h,
@@ -318,11 +406,32 @@ main (int argc, char **argv)
     log (LOG_PLAIN) << "Starting cygwin install, version " 
                     << setup_version << endLog;
 
-    UserSettings Settings (local_dir);
+#ifdef DEBUG
+    for (int i = 0; i < argc; i++)
+      {
+        log (LOG_BABBLE) << "argv[" << i << "] = " << _argv[i] << endLog;
+      }
+#endif
+
+    std::string SelfUpdateRemoveFromString = SelfUpdateRemoveFrom;
+    if (SelfUpdateRemoveFromString.size())
+      {
+        self_update_remove_from(SelfUpdateRemoveFromString);
+      }
+
+    std::string SelfUpdateCopyToString = SelfUpdateCopyTo;
+    if (SelfUpdateCopyToString.size())
+      {
+        self_update_copy_to(SelfUpdateCopyToString, argc);
+      }
+    else
+      {
+        UserSettings Settings (local_dir);
 
-    main_display ();
+        main_display ();
 
-    Settings.save ();	// Clean exit.. save user options.
+        Settings.save ();	// Clean exit.. save user options.
+      }
 
     if (rebootneeded)
       {
diff --git a/res.rc b/res.rc
index 92454eb..83e8bad 100644
--- a/res.rc
+++ b/res.rc
@@ -501,7 +501,7 @@ BEGIN
     IDS_UNINSTALL_COMPLETE  "Uninstalls complete."
     IDS_WININET             "Unable to find or load the Internet Explorer 5 DLLs"
     IDS_ERR_CHDIR           "Could not change dir to %s: %s [%.8x]"
-    IDS_OLD_SETUP_VERSION   "This setup is version %s, but setup.ini claims version %s is available.\nYou might want to upgrade to get the latest features and bug fixes."
+    IDS_OLD_SETUP_VERSION   "This setup is version %s, but setup.ini claims version %s is available.\nDo you want to upgrade to get the latest features and bug fixes?"
     IDS_DOWNLOAD_INCOMPLETE "Download Incomplete.  Try again?"
     IDS_INSTALL_ERROR	    "Installation error (%s), Continue with other packages?"
     IDS_INSTALL_INCOMPLETE  "Installation incomplete.  Check %s for details"
@@ -552,4 +552,5 @@ BEGIN
     IDS_CANT_MKDIR     "Couldn't create directory %s, sorry.  (Is drive full or read-only?)"
     IDS_NO_CWD	       "Local package directory %s not found.\nYou can still use setup.exe to remove installed\npackages, but there "
       "will be nothing to install.\n\nPress OK if that's what you wanted\nor Cancel to choose a different directory."
+    IDS_SETUP_URL      "http://cygwin.com/setup.exe"
 END
diff --git a/resource.h b/resource.h
index 8845497..e79d840 100644
--- a/resource.h
+++ b/resource.h
@@ -39,6 +39,7 @@
 #define IDS_MAYBE_MKDIR                   136
 #define IDS_CANT_MKDIR                    137
 #define IDS_NO_CWD			  138
+#define IDS_SETUP_URL                     139
 
 // Dialogs
 
diff --git a/threebar.cc b/threebar.cc
index b88dd38..cc4e083 100644
--- a/threebar.cc
+++ b/threebar.cc
@@ -240,9 +240,9 @@ ThreeBarProgressPage::OnMessageApp (UINT uMsg, WPARAM wParam, LPARAM lParam)
       }
     case WM_APP_SETUP_INI_DOWNLOAD_COMPLETE:
       {
-	if (lParam)
+	if (lParam > 0)
 	  GetOwner ()->SetActivePageByID (IDD_CHOOSE);
-	else
+	else if (lParam == 0) /* download failed */
 	  {
 	    if (source == IDC_SOURCE_CWD)
 	      {
@@ -276,6 +276,10 @@ ThreeBarProgressPage::OnMessageApp (UINT uMsg, WPARAM wParam, LPARAM lParam)
 		GetOwner ()->SetActivePageByID (IDD_SITE);
 	      }
 	  }
+        else /* a setup.exe update was downloaded */
+          {
+            PropSheet_PressButton(GetOwner()->GetHWND(), PSBTN_FINISH);
+          }
 	break;
       }
     default:
@@ -288,3 +292,9 @@ ThreeBarProgressPage::OnMessageApp (UINT uMsg, WPARAM wParam, LPARAM lParam)
   return true;
 
 }
+
+bool
+ThreeBarProgressPage::OnFinish ()
+{
+  return true;
+}
diff --git a/threebar.h b/threebar.h
index 225703a..328ccbf 100644
--- a/threebar.h
+++ b/threebar.h
@@ -63,6 +63,7 @@ public:
 
   virtual void OnInit ();
   virtual void OnActivate ();
+  virtual bool OnFinish ();
   virtual bool OnMessageApp (UINT uMsg, WPARAM wParam, LPARAM lParam);
   virtual long OnUnattended ()
   {
-- 
1.7.4

Re: [PATCH 5/5] Add a self-update mechanism for setup.exe

[Christopher Faylor]

On Sun, Apr 24, 2011 at 02:05:57PM +0100, Jon TURNEY wrote:
>Updating setup.exe has 3 stages:
>1) Download updated setup.exe to a temporary location
>2) Execute that temporary copy of setup.exe with --copy-to instructing
>it to copy itself over the setup.exe to be updated
>3) Execute the updated setup.exe with --remove-from instructing it
>to delete the temporary copy
>
>A named mutex is used to ensure setup exits from each stage before
>the next stage can start.
>
>Unfortunately, at the moment, we don't usefully check the setup version number
>until after we have downloaded and parsed setup.ini, which is perhaps a bit
>late to offer to update setup.exe
>
>v2: Address comments from Dave Korn
>Properly quote arguments to ensure spaces in paths are handled safely
>Place the setup URL in a string resouce
>
>2011-03-29  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* res.rc (IDS_OLD_SETUP_VERSION): Change text to offer to download
>	new version of setup.
>	(IDS_SETUP_URL): Added string resource.
>	* resource.h (IDS_SETUP_URL): Added resource identifier.
>	* main.cc (wait_for_exit, self_update_remove_from,
>	self_update_copy_to, WinMain): Add -copy-to and --remove-from options
>	for self-update process.
>	* ini.cc (self_update_download): New function to download updated
>	setup.
>	(do_ini_thread): Prompt to download updated setup if a newer
>	version exists.  Return a result indicating what should happen
>	next.
>	* threebar.h (PropertyPage::OnFinish): Add an implementation of
>	OnFinish virtual function for this class.
>	* threebar.cc (OnFinish): Ditto.
>	 (OnMessageApp): Setup should finish on WM_APP_SETUP_INI_DOWNLOAD_COMPLETE
>	if an updated setup was downloaded.
>
>2011-03-29  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* include/getopt++/DefaultFormatter.h (DefaultFormatter): Fix option string
>	formatting when it has no short option.

Sounds wonderful but couldn't you just rename the original setup.exe out
of the way, copy a new version over it in place, and then re-execute?

cgf

Re: [PATCH 5/5] Add a self-update mechanism for setup.exe

[Jon TURNEY]

On 24/04/2011 17:18, Christopher Faylor wrote:
> On Sun, Apr 24, 2011 at 02:05:57PM +0100, Jon TURNEY wrote:
>> Updating setup.exe has 3 stages:
>> 1) Download updated setup.exe to a temporary location
>> 2) Execute that temporary copy of setup.exe with --copy-to instructing
>> it to copy itself over the setup.exe to be updated
>> 3) Execute the updated setup.exe with --remove-from instructing it
>> to delete the temporary copy
>>
>> A named mutex is used to ensure setup exits from each stage before
>> the next stage can start.
>>
>> Unfortunately, at the moment, we don't usefully check the setup version number
>> until after we have downloaded and parsed setup.ini, which is perhaps a bit
>> late to offer to update setup.exe
>>
>> v2: Address comments from Dave Korn
>> Properly quote arguments to ensure spaces in paths are handled safely
>> Place the setup URL in a string resouce
>>
>> 2011-03-29  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>
>> 	* res.rc (IDS_OLD_SETUP_VERSION): Change text to offer to download
>> 	new version of setup.
>> 	(IDS_SETUP_URL): Added string resource.
>> 	* resource.h (IDS_SETUP_URL): Added resource identifier.
>> 	* main.cc (wait_for_exit, self_update_remove_from,
>> 	self_update_copy_to, WinMain): Add -copy-to and --remove-from options
>> 	for self-update process.
>> 	* ini.cc (self_update_download): New function to download updated
>> 	setup.
>> 	(do_ini_thread): Prompt to download updated setup if a newer
>> 	version exists.  Return a result indicating what should happen
>> 	next.
>> 	* threebar.h (PropertyPage::OnFinish): Add an implementation of
>> 	OnFinish virtual function for this class.
>> 	* threebar.cc (OnFinish): Ditto.
>> 	 (OnMessageApp): Setup should finish on WM_APP_SETUP_INI_DOWNLOAD_COMPLETE
>> 	if an updated setup was downloaded.
>>
>> 2011-03-29  Jon TURNEY  <jon.turney-GrJqePx9RPPAJUdA+FbntA@public.gmane.org>
>>
>> 	* include/getopt++/DefaultFormatter.h (DefaultFormatter): Fix option string
>> 	formatting when it has no short option.
> 
> Sounds wonderful but couldn't you just rename the original setup.exe out
> of the way, copy a new version over it in place, and then re-execute?

I'm just following a sequence of actions which a bit of google research found
other people had successfully used.

what you suggest seems like it should work, although I'd perhaps be a bit wary
of assuming that MoveFile() on the running executable works always.