Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Corinna Vinschen]

On Aug 11 19:24, Achim Gratz wrote:
> Corinna Vinschen writes:
> > Patch applied.
> 
> As a follow-up on the discussion I've checked why this is a conversion
> operator in the first place and the answer is "no particularly good
> reason".  I propose to keep the implementation unchanged, but as a plain
> member function str() instead, which should be easier to grok some time
> later.  This is the only path the conversion operator got invoked
> through anyway.
> 

Applied.


Thanks,
Corinna

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


pgpJacBM2DzBD.pgp
Description: PGP signature

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Achim Gratz]

Corinna Vinschen writes:
> Patch applied.

As a follow-up on the discussion I've checked why this is a conversion
operator in the first place and the answer is "no particularly good
reason".  I propose to keep the implementation unchanged, but as a plain
member function str() instead, which should be easier to grok some time
later.  This is the only path the conversion operator got invoked
through anyway.

>From 296273b76f6913c49bab363b08fb18865eec4351 Mon Sep 17 00:00:00 2001
From: Achim Gratz 
Date: Sat, 10 Aug 2013 15:24:41 +0200
Subject: [PATCH] remove conversion operator std::string(), instead use its
 implementation for str() member function

   * csu_util/MD5Sum.h (MD5Sum): Remove declaration for conversion
   operator std::string().  Remove implementation of member
   function str() using the conversion operator.
   * csu_util/MD5Sum.cc (MD5Sum::str): Reuse implementation of
   conversion operator std::String to implement member function
   str() with.
---
 csu_util/MD5Sum.cc | 3 ++-
 csu_util/MD5Sum.h  | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/csu_util/MD5Sum.cc b/csu_util/MD5Sum.cc
index eb679b5..1d9c362 100644
--- a/csu_util/MD5Sum.cc
+++ b/csu_util/MD5Sum.cc
@@ -80,7 +80,8 @@ MD5Sum::finish()
   delete internalData; internalData = 0;
 }
 
-MD5Sum::operator std::string() const
+std::string
+MD5Sum::str() const
 {
   std::ostringstream hexdigest;
 
diff --git a/csu_util/MD5Sum.h b/csu_util/MD5Sum.h
index ff9021e..5c51bbc 100644
--- a/csu_util/MD5Sum.h
+++ b/csu_util/MD5Sum.h
@@ -44,8 +44,7 @@ class MD5Sum
 void finish();
 
 bool isSet() const { return (state == Set); };
-operator std::string() const;
-std::string str() const { return (std::string)(*this); };
+std::string str() const;
 bool operator == (const MD5Sum& other) const;
 bool operator != (const MD5Sum& other) const { return !(*this == other); };
 
-- 
1.8.3.1



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Achim Gratz]

Corinna Vinschen writes:
> Not quite.  ostringstream::str returns string, the string constructor
> implicitely returns string&.

I could be reading it wrong, but I don't think that's what the C++11
standard says should happen.  The implicit copy constructor transfering
the return value out of the function will treat the argument of the
return as an rvalue reference (but it'd do that just the same for
ostringstream::str, since that's simply how it is declared.

> It's sometimes tricky to wrap the brain
> around the differences as far as the scope is concerned.

It is true that the temporary object created by the explicit constructor
in the return statement is destroyed at the end of full-expression of
the return (12.2, 12.4).  But, the return statement itself has
initialization semantics, which means it operates as if a copy
constructor was invoked between the argument of the return and the
actual return value of the function (which in this case is a conversion
function, but I didn't find any hints in the standard that this would
somehow change semantics).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Samples for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Corinna Vinschen]

On Aug  9 13:54, Christopher Faylor wrote:
> On Fri, Aug 09, 2013 at 07:47:23PM +0200, Corinna Vinschen wrote:
> >On Aug  9 12:55, Christopher Faylor wrote:
> >> On Fri, Aug 09, 2013 at 11:01:32AM -0500, Thrall, Bryan wrote:
> >> >Christopher Faylor wrote on 2013-08-09:
> >> >> On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote:
> >> >>> On Aug  8 20:34, Achim Gratz wrote:
> >>  
> >>  I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
> >>  package was checked that used to clear up after a reboot.  Today,
> >> >with a
> >>  freshly built setup.exe this failure was now entirely reproduceable.
> >>  I've fixed it by reimplementing the string formatting for the MD5
> >> >digest
> >>  using C++ stream functions.
> >>  
> >> >>> 
> >> > From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17
> >> >> 00:00:00 2001
> >>  From: Achim Gratz 
> >>  Date: Thu, 8 Aug 2013 20:23:31 +0200
> >>  Subject: [PATCH] fix SEGV on WinXP/Pro
> >>  
> >>  * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
> >>  Reimplement using stringstream to avoid a SEGV on WinXP/Pro.
> >> >>> 
> >> >>> Patch applied.
> >> >>> 
> >>  -  return std::string(hexdigest);
> >> >>> ^^
> >> >>> I'm wondering if that was the problem.  This expression constructs a
> >> >>> std:string and then immediately destructs it since the scope is
> >> >limited
> >> >>> to the end of the function (which the return statement is all about).
> >> >>> Reading the value of this object in the parent function is basically
> >> >>> luck, isn't it?
> >> >> 
> >> >> Sheesh.  Yes, that looks like the problem.  But doesn't the new code
> >> >do
> >> >> pretty much the same thing?
> >> >> 
> >> >> +  std::ostringstream hexdigest;
> >> >> +  return hexdigest.str();
> >> >
> >> >According to this:
> >> >
> >> >http://stackoverflow.com/questions/275214/scope-and-return-values-in-c
> >> >
> >> >Returning the object should be ok because it is copied before leaving
> >> >the function scope; returning a reference or pointer to the object is
> >> >where you get into problems.
> >> 
> >> Thanks for clarifying.  Isn't that what the original code did too then?
> >
> >Not quite.  ostringstream::str returns string, the string constructor
> >implicitely returns string&.  It's sometimes tricky to wrap the brain
> >around the differences as far as the scope is concerned.
> 
> Perhaps a comment here would help future perusers of this function.

In theory it's basic C++ behaviour, but comments don't hurt, so, sure.


Corinna

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


pgpUGI_O02W8g.pgp
Description: PGP signature

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Achim Gratz]

Corinna Vinschen writes:
>> -  return std::string(hexdigest);
>  ^^
>
> I'm wondering if that was the problem.

I was wondering about this, too — but then the SEGV should have happened
in the calling function.  I believe that the vetting of hexdigest[32]
with a zero was the actual point of failure.  That really should not
happen, I think, but then I haven't chased it down to the actual
instructions.

>  This expression constructs a
> std:string and then immediately destructs it since the scope is limited
> to the end of the function (which the return statement is all about).

This being C++, the compiler is expected to copy the temporary object to
the return object or (as would be possible here) to construct the
temporary object in the place of the return object, thereby saving the
copy operation.

> Reading the value of this object in the parent function is basically
> luck, isn't it?

If it was just giving out a char* that would be a common mistake to
make, but not much of C++ would work if that was the case when an actual
object is involved.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Christopher Faylor]

On Fri, Aug 09, 2013 at 07:47:23PM +0200, Corinna Vinschen wrote:
>On Aug  9 12:55, Christopher Faylor wrote:
>> On Fri, Aug 09, 2013 at 11:01:32AM -0500, Thrall, Bryan wrote:
>> >Christopher Faylor wrote on 2013-08-09:
>> >> On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote:
>> >>> On Aug  8 20:34, Achim Gratz wrote:
>>  
>>  I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
>>  package was checked that used to clear up after a reboot.  Today,
>> >with a
>>  freshly built setup.exe this failure was now entirely reproduceable.
>>  I've fixed it by reimplementing the string formatting for the MD5
>> >digest
>>  using C++ stream functions.
>>  
>> >>> 
>> > From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17
>> >> 00:00:00 2001
>>  From: Achim Gratz 
>>  Date: Thu, 8 Aug 2013 20:23:31 +0200
>>  Subject: [PATCH] fix SEGV on WinXP/Pro
>>  
>>  * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
>>  Reimplement using stringstream to avoid a SEGV on WinXP/Pro.
>> >>> 
>> >>> Patch applied.
>> >>> 
>>  -  return std::string(hexdigest);
>> >>> ^^
>> >>> I'm wondering if that was the problem.  This expression constructs a
>> >>> std:string and then immediately destructs it since the scope is
>> >limited
>> >>> to the end of the function (which the return statement is all about).
>> >>> Reading the value of this object in the parent function is basically
>> >>> luck, isn't it?
>> >> 
>> >> Sheesh.  Yes, that looks like the problem.  But doesn't the new code
>> >do
>> >> pretty much the same thing?
>> >> 
>> >> +  std::ostringstream hexdigest;
>> >> +  return hexdigest.str();
>> >
>> >According to this:
>> >
>> >http://stackoverflow.com/questions/275214/scope-and-return-values-in-c
>> >
>> >Returning the object should be ok because it is copied before leaving
>> >the function scope; returning a reference or pointer to the object is
>> >where you get into problems.
>> 
>> Thanks for clarifying.  Isn't that what the original code did too then?
>
>Not quite.  ostringstream::str returns string, the string constructor
>implicitely returns string&.  It's sometimes tricky to wrap the brain
>around the differences as far as the scope is concerned.

Perhaps a comment here would help future perusers of this function.

cgf

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Corinna Vinschen]

On Aug  9 12:55, Christopher Faylor wrote:
> On Fri, Aug 09, 2013 at 11:01:32AM -0500, Thrall, Bryan wrote:
> >Christopher Faylor wrote on 2013-08-09:
> >> On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote:
> >>> On Aug  8 20:34, Achim Gratz wrote:
>  
>  I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
>  package was checked that used to clear up after a reboot.  Today,
> >with a
>  freshly built setup.exe this failure was now entirely reproduceable.
>  I've fixed it by reimplementing the string formatting for the MD5
> >digest
>  using C++ stream functions.
>  
> >>> 
> > From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17
> >> 00:00:00 2001
>  From: Achim Gratz 
>  Date: Thu, 8 Aug 2013 20:23:31 +0200
>  Subject: [PATCH] fix SEGV on WinXP/Pro
>  
>  * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
>  Reimplement using stringstream to avoid a SEGV on WinXP/Pro.
> >>> 
> >>> Patch applied.
> >>> 
>  -  return std::string(hexdigest);
> >>> ^^
> >>> I'm wondering if that was the problem.  This expression constructs a
> >>> std:string and then immediately destructs it since the scope is
> >limited
> >>> to the end of the function (which the return statement is all about).
> >>> Reading the value of this object in the parent function is basically
> >>> luck, isn't it?
> >> 
> >> Sheesh.  Yes, that looks like the problem.  But doesn't the new code
> >do
> >> pretty much the same thing?
> >> 
> >> +  std::ostringstream hexdigest;
> >> +  return hexdigest.str();
> >
> >According to this:
> >
> >http://stackoverflow.com/questions/275214/scope-and-return-values-in-c
> >
> >Returning the object should be ok because it is copied before leaving
> >the function scope; returning a reference or pointer to the object is
> >where you get into problems.
> 
> Thanks for clarifying.  Isn't that what the original code did too then?

Not quite.  ostringstream::str returns string, the string constructor
implicitely returns string&.  It's sometimes tricky to wrap the brain
around the differences as far as the scope is concerned.


Corinna

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


pgpCuWNqsnEoF.pgp
Description: PGP signature

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Christopher Faylor]

On Fri, Aug 09, 2013 at 11:01:32AM -0500, Thrall, Bryan wrote:
>Christopher Faylor wrote on 2013-08-09:
>> On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote:
>>> On Aug  8 20:34, Achim Gratz wrote:
 
 I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
 package was checked that used to clear up after a reboot.  Today,
>with a
 freshly built setup.exe this failure was now entirely reproduceable.
 I've fixed it by reimplementing the string formatting for the MD5
>digest
 using C++ stream functions.
 
>>> 
> From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17
>> 00:00:00 2001
 From: Achim Gratz 
 Date: Thu, 8 Aug 2013 20:23:31 +0200
 Subject: [PATCH] fix SEGV on WinXP/Pro
 
 * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
 Reimplement using stringstream to avoid a SEGV on WinXP/Pro.
>>> 
>>> Patch applied.
>>> 
 -  return std::string(hexdigest);
>>> ^^
>>> I'm wondering if that was the problem.  This expression constructs a
>>> std:string and then immediately destructs it since the scope is
>limited
>>> to the end of the function (which the return statement is all about).
>>> Reading the value of this object in the parent function is basically
>>> luck, isn't it?
>> 
>> Sheesh.  Yes, that looks like the problem.  But doesn't the new code
>do
>> pretty much the same thing?
>> 
>> +  std::ostringstream hexdigest;
>> +  return hexdigest.str();
>
>According to this:
>
>http://stackoverflow.com/questions/275214/scope-and-return-values-in-c
>
>Returning the object should be ok because it is copied before leaving
>the function scope; returning a reference or pointer to the object is
>where you get into problems.

Thanks for clarifying.  Isn't that what the original code did too then?

cgf

RE: [PATCH] setup.exe SEGV on WinXP/Pro

[Thrall, Bryan]

Christopher Faylor wrote on 2013-08-09:
> On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote:
>> On Aug  8 20:34, Achim Gratz wrote:
>>> 
>>> I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
>>> package was checked that used to clear up after a reboot.  Today,
with a
>>> freshly built setup.exe this failure was now entirely reproduceable.
>>> I've fixed it by reimplementing the string formatting for the MD5
digest
>>> using C++ stream functions.
>>> 
>> 
 From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17
> 00:00:00 2001
>>> From: Achim Gratz 
>>> Date: Thu, 8 Aug 2013 20:23:31 +0200
>>> Subject: [PATCH] fix SEGV on WinXP/Pro
>>> 
>>> * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
>>> Reimplement using stringstream to avoid a SEGV on WinXP/Pro.
>> 
>> Patch applied.
>> 
>>> -  return std::string(hexdigest);
>> ^^
>> I'm wondering if that was the problem.  This expression constructs a
>> std:string and then immediately destructs it since the scope is
limited
>> to the end of the function (which the return statement is all about).
>> Reading the value of this object in the parent function is basically
>> luck, isn't it?
> 
> Sheesh.  Yes, that looks like the problem.  But doesn't the new code
do
> pretty much the same thing?
> 
> +  std::ostringstream hexdigest;
> +  return hexdigest.str();

According to this:

http://stackoverflow.com/questions/275214/scope-and-return-values-in-c

Returning the object should be ok because it is copied before leaving
the function scope; returning a reference or pointer to the object is
where you get into problems.

--
Bryan Thrall
Principal Software Engineer
FlightSafety International * Visual Simulation Systems * 5695 Campus
Parkway  * Hazelwood, MO 63042
Tel: (314) 551-8413 * Fax: (314) 551-8444
bryan.thr...@flightsafety.com * www.flightsafety.com * A Berkshire
Hathaway company

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Christopher Faylor]

On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote:
>On Aug  8 20:34, Achim Gratz wrote:
>> 
>> I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
>> package was checked that used to clear up after a reboot.  Today, with a
>> freshly built setup.exe this failure was now entirely reproduceable.
>> I've fixed it by reimplementing the string formatting for the MD5 digest
>> using C++ stream functions.
>> 
>
>> >From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001
>> From: Achim Gratz 
>> Date: Thu, 8 Aug 2013 20:23:31 +0200
>> Subject: [PATCH] fix SEGV on WinXP/Pro
>> 
>> * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
>> Reimplement using stringstream to avoid a SEGV on WinXP/Pro.
>
>Patch applied.
>
>> -  return std::string(hexdigest);
> ^^
>
>I'm wondering if that was the problem.  This expression constructs a
>std:string and then immediately destructs it since the scope is limited
>to the end of the function (which the return statement is all about).
>Reading the value of this object in the parent function is basically
>luck, isn't it?

Sheesh.  Yes, that looks like the problem.  But doesn't the new code do
pretty much the same thing?

+  std::ostringstream hexdigest;
+  return hexdigest.str();


cgf

Re: [PATCH] setup.exe SEGV on WinXP/Pro

[Corinna Vinschen]

On Aug  8 20:34, Achim Gratz wrote:
> 
> I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a
> package was checked that used to clear up after a reboot.  Today, with a
> freshly built setup.exe this failure was now entirely reproduceable.
> I've fixed it by reimplementing the string formatting for the MD5 digest
> using C++ stream functions.
> 

> >From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001
> From: Achim Gratz 
> Date: Thu, 8 Aug 2013 20:23:31 +0200
> Subject: [PATCH] fix SEGV on WinXP/Pro
> 
> * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const):
> Reimplement using stringstream to avoid a SEGV on WinXP/Pro.

Patch applied.

> -  return std::string(hexdigest);
 ^^

I'm wondering if that was the problem.  This expression constructs a
std:string and then immediately destructs it since the scope is limited
to the end of the function (which the return statement is all about).
Reading the value of this object in the parent function is basically
luck, isn't it?


Corinna

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

Re: [PATCH] setup.exe

[Christopher Faylor]

On Tue, Jan 22, 2013 at 04:32:22AM +0900, green fox wrote:
>Chris, language please.

Sorry for being unclear.  I was writing in English.

>The problem in lack of decent package manager is something most of us here
>know for a long time. Hope everyone agrees on this point.

Reiteration of this point in all of your messages does not serve any
useful purpose.

A quick scan of your reply seemed to show that you are asking for
features and outlining well-known shortcomings.  Unless you are
volunteering to do that work, this is likely YA wishlist which will go
nowhere.

For your amusement:

http://www.cygwin.com/ml/cygwin-apps/2004-12/msg00019.html

>I hope this is clear enough example for _one_ of the many reasons why
>we need and write workarounds.

I was asking why the *specific* change needed to be implemented.  You
did not answer my question.  But that's ok.  This particular case is
closed anyway.

>As a side note
>libzypp =>Depends on mingw boost C++  gtk e2fsporg and so forth...
>I really like the algorithm they have, but we need it running quick,
>cheap, and simple.
>have not looked into it, but if it matches our requirements, prahaps a
>option. not sure yet.

No idea what libzypp is.

**cgf googles

http://doc.opensuse.org/projects/libzypp/HEAD/

If someone has ported rpm to mingw that would certainly be interesting.
Otherwise, this is a cart-before-the-horse situation.

>One of the reasons we sill have perl/awk/shell scripts to roll things
>out, is when things break, its not that hard to fix/patch a workaround.

I love perl/awk/shell scripts.  I use them quite frequently.  I was
actually suggesting that they would be an alternative to the suggested
patch.  I think that, in general, wrapping your installation needs in a
wrapper is a fine idea.

cgf

Re: [PATCH] setup.exe

[green fox]

Chris, language please.

The problem in lack of decent package manager is something most of us here
know for a long time. Hope everyone agrees on this point.

Here we're just focusing to concentrate the work,
refactor what key items we need, or can leave out with, re-use and such.

At the same time, things must be kept running.
So a workaround (for the mean time) is needed.
We know that setup.exe is not enough.
And as clealy noted, this setup-.ini thing is a _workaround_
We need a package manager to allow routine package management tasks to
be accomplished in a single command.

We need
1- a consistent user interface
2- automated package installation
3- upgrading and searching
4- package removal
5- package builder
6- lookup of package dependency
7- bootstrap cygwin to a clean env
8- generate/modify setup.ini and customize

 *And everyting must be script-ready*

let me know if I left something out

The existing and runing system does not allow one to specify a version
for a given package/
example)
what if package foo 3.1.6 had changed ABI, and 3.1.x (other thatn 6)
restored the old ABI.
if we have a dependency from bar, either
A) if bar is compiled with 3.1.6 , then make sure foo is against 3.1.6 as well
B) or check that foo != 3.1.6 , and double check that bar is compiled
for version != 3.1.6

Version control using  prev|curr|test is not the right way to do things, IMHO

I hope this is clear enough example for _one_ of the many reasons why
we need and write workarounds.

As a side note
libzypp =>Depends on mingw boost C++  gtk e2fsporg and so forth...
I really like the algorithm they have, but we need it running quick,
cheap, and simple.
have not looked into it, but if it matches our requirements, prahaps a
option. not sure yet.

One of the reasons we sill have perl/awk/shell scripts to roll things out, is
when things break, its not that hard to fix/patch a workaround.

fox

Re: [PATCH] setup.exe

[Achim Gratz]

Achim Gratz writes:
> I have one last idea of how to live without that particular change.  If
> it works, I'll drop the patch.

I've done some tests and I can implement my minimum requirement by
changing the directory structure to something like:

repo/release/... (packages directory)
repo/setup_current/setup.ini
repo/setup_2013-01-21/setup.ini

This works for me since I'm rewriting setup.ini anyway and each install
entry now simply gets a "../" prepended.  This isn't quite as neat as
simply telling setup.exe where to find an alternative setup.ini, but as
long as you have write access to the repo it gets the job done with
minimum fuzz.  I guess if you could put absolute paths into your
setup.ini otherwise, but then you'll have to re-create it again with
relative paths on release.  I'm keeping the patch locally since it has
proven useful to me, but for the "official" setup.exe I can use the
above workaround.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH] setup.exe

[Christopher Faylor]

On Mon, Jan 21, 2013 at 06:46:00PM +0900, green fox wrote:
>On 1/21/13, Christopher Faylor wrote:
>> On Mon, Jan 21, 2013 at 04:03:04PM +0900, green fox wrote:
>>>On 1/20/13, Christopher Faylor wrote:
What *specifically* do you really like?
>>>
>>>+1 for being able to specify custom setup.ini Not a happy moment when
>>>you realize some package is missing, having spent 2 housr to distribute
>>>the blob on the network.
>>
>> If you are missing a package then sending out a new setup.ini which
>> includes the missing package should fix the problem.  I don't see how
>> being able to specify something other than setup.ini helps.
>>
>> Btw, I'm not looking for votes.  I'm looking for explanations for why
>> this option is useful.  It seems like the described functionality could
>> be handled with just a shell script wrapper.
>
>Because we do not have a decent package manager

And, that's because we don't have any "decent" developers willing to
write one.  Why haven't you stepped up?

But, anyway, you are apparently missing the point.  I'm not looking for
complaints.  I'm looking for actual reasoned arguments for why a patch
should be applied.  If you are just knee-jerk responding to someone
saying "unattended setup" then you're going to have to actually read and
understand what the patch does and indicate why it would be useful.

cgf

Re: [PATCH] setup.exe

[green fox]

On 1/21/13, Christopher Faylor wrote:
> On Mon, Jan 21, 2013 at 04:03:04PM +0900, green fox wrote:
>>On 1/20/13, Christopher Faylor wrote:
>>>What *specifically* do you really like?
>>
>>+1 for being able to specify custom setup.ini Not a happy moment when
>>you realize some package is missing, having spent 2 housr to distribute
>>the blob on the network.
>
> If you are missing a package then sending out a new setup.ini which
> includes the missing package should fix the problem.  I don't see how
> being able to specify something other than setup.ini helps.
>
> Btw, I'm not looking for votes.  I'm looking for explanations for why
> this option is useful.  It seems like the described functionality could
> be handled with just a shell script wrapper.
>
> cgf
>

Because we do not have a decent package manager

fox

Re: [PATCH] setup.exe

[green fox]

On 1/20/13, Achim Gratz wrote:
> I plan to publish my infrastructure, but haven't yet since it a) isn't
> feature complete and b) I need to clean up a few things.  I don't want
> to fork setup.exe if I can help it.

Agrred. No one likes to fork, and that includes myself.
And believe me, clean up of code will never happen. We've got a
rollout script that noone dare touches it to clean it up. And it
grows, too.

The lack for any good (standard) bootstrap method for mass deployment
is a problem.
And everybody result in writing one'sown in-house version.

> Let me briefly describe how it works: I currently use lftp to mirror
> Cygwin and Cygport locally (in a pinch you could use setup.exe itself to
> do this).  I have locally built packages and (sometimes) the Cygwin
> snapshots re-packaged into Cygwin packages as additional package
> sources.  I then use a Perl script to combine all package sources, pull
> in dependencies of the selected packages and write this out to a new
> setup.ini and install hierarchy (optionally with removing unreferenced
> files from earlier versions).  I'm injecting additional groups into the
> new setup.ini so that I can do different installs from the same
> setup.ini easily (I currently have CLI, GUI, Devel, CygDev - but you can
> define whatever you want).  This is what then gets distributed to the

Diffrent vehicle(rsync), but same here.
Diffrence is, we gave up on the setup.ini route in the past, but if
the setup.ini thing works nicely, this is cleaner approach than what
we've got.
Having to build and deploy custom packages, wrote a quick and dirty
script that makes a self extracting archive blob. Then it pushes
out/exec on remote. Very dirty, but we had a existing base for pushing
out binary blobs/remove executon anyway.

> This is working and it doesn't require any changes to setup.exe.
Very cool :-)

> As I said, I have additional requirements to provide different releases,
> which for the sake of discussion can be simplified to being able to take
> a setup.ini and "freeze" it along with all the package files it
> references and then be able to install it again in exactly that state at
> a later time.

Actually this is the most important thing lacking from the curernt situation.

We have a mirror that holds packages of (current and past )specific
versions, and
the freeze/save state is performed by the (avobe mentioned) blob maker
(that can re-create the same blob , as long as supplied with same list
).
But this is not the best way to do things. It's jsut a workaroud,
that's been used for years.

> I guess anyone trying to get Cygwin into a corporate network is in the
> same boat.
>

really need a solid package manager for cygwin.

> start setup.exe two times in a row) and the ability to delete packages
> from the command line.  The functionality to do this already is in
> setup.exe, there are just no controls for this on the command line.

+1 on providing a solid option
As a minimum having install|remove|update|show|find|depends would be needed.

And if possible
-mirror  : set mirror to download package( can specify multiple)
-cache   : set cache directory to use
 (for packages that may aready been downloaded/partial download in progress)
Something like combination of the exsiting [--local-install]
[--local-package-dir]
-file, -f   : read package list to install/remove from file
([-P] from a file)
-force: Force install/remove

> Maybe an option to suppress the GUI completely would be nice, but I
> personally like to have the progress status on screen.
>
I believe there is [--quiet-mode]

>> My 2cents worth of thought tells me, providing setup.exe as front
>> end/stub/gui to call a solid back end package manager is a reasonable
>> way to go.
>
> That would be a different setup.exe than we have today.  If Cygwin wants
> to go that route, a more capable package backend would be nice, libzypp
> anyone?
>

It was just a dream.
But really, having only curr|prev|test for version control is not funny.

Re: [PATCH] setup.exe

[Christopher Faylor]

On Mon, Jan 21, 2013 at 04:03:04PM +0900, green fox wrote:
>On 1/20/13, Christopher Faylor wrote:
>>What *specifically* do you really like?
>
>+1 for being able to specify custom setup.ini Not a happy moment when
>you realize some package is missing, having spent 2 housr to distribute
>the blob on the network.

If you are missing a package then sending out a new setup.ini which
includes the missing package should fix the problem.  I don't see how
being able to specify something other than setup.ini helps.

Btw, I'm not looking for votes.  I'm looking for explanations for why
this option is useful.  It seems like the described functionality could
be handled with just a shell script wrapper.

cgf

Re: [PATCH] setup.exe

[green fox]

On 1/20/13, Christopher Faylor wrote:
>
> What *specifically* do you really like?
>
> cgf
>

+1 for being able to specify custom setup.ini
Not a happy moment when you realize some package is missing, having
spent 2 housr to distribute the blob on the network.

Only a _workaround_, but If I can just push out the new
setup--.ini
or something, and have a bat script automate the setup depend on the
specific machine group,
it is much better than the current situation.
Haven't tried it out, but it seems promising.

Not the best way, having a solid package manager is best...

fox

Re: [PATCH] setup.exe

[Achim Gratz]

Christopher Faylor writes:
> I was referring to your change which handled versioned setup.ini's.
> That is not a requirement for an unattended Cygwin install.

I do have that requirement, not that I love it very much.

> If there was a general hue and cry for the feature that you want to add,
> I'd consider adding it.  AFAIK, there isn't.

The mere existence of multiple "alternative" installers should speak to
you.

> If we did add it we would be obligated to support it so, if there is a
> need for this type of feature, I'd like to see a lot more discussion
> about why it's necessary and the best way to implement it.

I have one last idea of how to live without that particular change.  If
it works, I'll drop the patch.  I will propose at least two other
changes that I don't think have workarounds that wouldn't require to
duplicate large swathes of functionality that already is in setup.exe,
but simply not accessible from the command line.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Samples for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra

Re: [PATCH] setup.exe

[Achim Gratz]

green fox writes:
> Achim, reguardless of if this code getting into cygwin (or not), could
> you be able to provide a copy of this on public git/whatever server?

I plan to publish my infrastructure, but haven't yet since it a) isn't
feature complete and b) I need to clean up a few things.  I don't want
to fork setup.exe if I can help it.

> I really like this.

Thanks.

> Just for note, in the past, unattended instalation/update using
> automated package management, we resorted into using apt-cyg written
> by Stephen Jungels.

I've looked at apt-cyg and all the other alternative installers I could
find, but they could either not bootstrap Cygwin or would require too
much infrastructure for installation.  That's why I decided to stick
with setup.exe (also because it is the supported Cygwin installer).

Let me briefly describe how it works: I currently use lftp to mirror
Cygwin and Cygport locally (in a pinch you could use setup.exe itself to
do this).  I have locally built packages and (sometimes) the Cygwin
snapshots re-packaged into Cygwin packages as additional package
sources.  I then use a Perl script to combine all package sources, pull
in dependencies of the selected packages and write this out to a new
setup.ini and install hierarchy (optionally with removing unreferenced
files from earlier versions).  I'm injecting additional groups into the
new setup.ini so that I can do different installs from the same
setup.ini easily (I currently have CLI, GUI, Devel, CygDev - but you can
define whatever you want).  This is what then gets distributed to the
software servers and installed via batch files, or you could even put it
on a flash drive or burn a DVD.

This is working and it doesn't require any changes to setup.exe.  The
users can use setup.exe to alter their installations if they think they
know what they're doing and I can keep providing updated versions
without messing up all those installations each time.

As I said, I have additional requirements to provide different releases,
which for the sake of discussion can be simplified to being able to take
a setup.ini and "freeze" it along with all the package files it
references and then be able to install it again in exactly that state at
a later time.

> Its just that we have duplication on effort here, not just me, Achim,
> the cyg-apt(diffrent guy), more then few "Alternative" installers on
> the net, each admin having their own way of ghosting/cloneing/setting
> up cygwin...

I guess anyone trying to get Cygwin into a corporate network is in the
same boat.

> It may not be the intended use-case of the cygwin dev, however with
> all respect, there is a need for a solid cui with /short and sweet
> options/automated install/update/package management/use of
> proxy(s)/cache/use of mirror/alternative dir as source/dl from
> multiple servers/offline/version check/hash check, as a bare minimum
> requirement when managing multilpe clients.

So far, the only things I would want to change in setup.exe is to name
alternative setup.ini files (for which I sent the patch), but given the
resistance I've met I'll check again if I can work around this without
duplicating whole directory trees or using links/junctions.  If it's
possible to do the moral equivalent by just changing the directory
structure, then I'll happily drop the patch.

The remaining wishes are to add an option to again provide the
"install+update" functionality that was briefly present but then got
changed into either "install" or "update" (which currently requires to
start setup.exe two times in a row) and the ability to delete packages
from the command line.  The functionality to do this already is in
setup.exe, there are just no controls for this on the command line.
Maybe an option to suppress the GUI completely would be nice, but I
personally like to have the progress status on screen.

> And setup.exe has very good GUI, with new and improved commandlines,
> its just that it is not suitable for day to day administration
> purpose. However we all have diffrent ways of providing the package
> data. Nfs/mirror/rsync/ftp/p2p/samba/dvd/ghost/proxy, etc,etc...
> Trying to provide "Everything" in setup.exe is cauing the code to get
> into spagetti.

As I said, this is all dealt with outside setup.exe already and I agree
that this should _not_ be bolted onto setup.exe at all.

> My 2cents worth of thought tells me, providing setup.exe as front
> end/stub/gui to call a solid back end package manager is a reasonable
> way to go.

That would be a different setup.exe than we have today.  If Cygwin wants
to go that route, a more capable package backend would be nice, libzypp
anyone?

> For admin purpose using the backend directly. With well
> difined porotocol on package version control, we could start writeing
> / or even re-use existing package managers. Just a thought.

While I'd love to see something like this, see above why the current
state isn't that far from what is needed, at least in the short 

Re: [PATCH] setup.exe

[Christopher Faylor]

On Sun, Jan 20, 2013 at 12:35:32PM +0900, green fox wrote:
>Achim, reguardless of if this code getting into cygwin (or not), could
>you be able to provide a copy of this on public git/whatever server?
>I really like this.

What *specifically* do you really like?

cgf

Re: [PATCH] setup.exe

[green fox]

Achim, reguardless of if this code getting into cygwin (or not), could
you be able to provide a copy of this on public git/whatever server?
I really like this.

Just for note, in the past, unattended instalation/update using
automated package management, we resorted into using apt-cyg written
by Stephen Jungels.
(http://code.google.com/apt-cyg)

However, I had requirements for closed network instalation, and (back
then nobody supported that) added options for --localdiskcache,
--force, --ForceUpdate and md5sum file check... etc. The modified
script is floating in superuser.com
(superuser.com/questions/99198/cygwin-offline-installer) if anyone
wants it.

Its just that we have duplication on effort here, not just me, Achim,
the cyg-apt(diffrent guy), more then few "Alternative" installers on
the net, each admin having their own way of ghosting/cloneing/setting
up cygwin...

It may not be the intended use-case of the cygwin dev, however with
all respect, there is a need for a solid cui with /short and sweet
options/automated install/update/package management/use of
proxy(s)/cache/use of mirror/alternative dir as source/dl from
multiple servers/offline/version check/hash check, as a bare minimum
requirement when managing multilpe clients.

And setup.exe has very good GUI, with new and improved commandlines,
its just that it is not suitable for day to day administration
purpose. However we all have diffrent ways of providing the package
data. Nfs/mirror/rsync/ftp/p2p/samba/dvd/ghost/proxy, etc,etc...
Trying to provide "Everything" in setup.exe is cauing the code to get
into spagetti.

Could we somehow share and concatinate the effort?

My 2cents worth of thought tells me, providing setup.exe as front
end/stub/gui to call a solid back end package manager is a reasonable
way to go. For admin purpose using the backend directly. With well
difined porotocol on package version control, we could start writeing
/ or even re-use existing package managers. Just a thought.

GreenFox


On 1/20/13, Christopher Faylor wrote:
> On Sat, Jan 19, 2013 at 09:47:31PM +0100, Achim Gratz wrote:
>>Christopher Faylor writes:
>>>I'm really not too keen on adding hacks to setup so that people can use
>>>it for other than its intended purpose.
>>
>>If there is another method to do an unattended Cygwin install, I'm all
>>ears.  I have briefly pondered to script a standalone installer, but it
>>requires too much infrastructure and I don't see much value in trying
>>to re-do what setup.exe already does.  The question of doing unattended
>>installs comes up often enough that official support would be nice,
>>IMHO.
>
> I was referring to your change which handled versioned setup.ini's.
> That is not a requirement for an unattended Cygwin install.
>
>>>The code is already a complicated obtuse mishmash and adding more stuff
>>>which would be used by only one person doesn't seem like a good idea.
>>
>>A self-fulfilling prophecy: nobody is doing it because nobody is doing
>>it.  I'll have to add on whatever I need, if it's useless to you then
>>just don't use it.
>
> If there was a general hue and cry for the feature that you want to add,
> I'd consider adding it.  AFAIK, there isn't.  If we did add it we would
> be obligated to support it so, if there is a need for this type of
> feature, I'd like to see a lot more discussion about why it's necessary
> and the best way to implement it.
>
> cgf
>

Re: [PATCH] setup.exe

[Christopher Faylor]

On Sat, Jan 19, 2013 at 09:47:31PM +0100, Achim Gratz wrote:
>Christopher Faylor writes:
>>I'm really not too keen on adding hacks to setup so that people can use
>>it for other than its intended purpose.
>
>If there is another method to do an unattended Cygwin install, I'm all
>ears.  I have briefly pondered to script a standalone installer, but it
>requires too much infrastructure and I don't see much value in trying
>to re-do what setup.exe already does.  The question of doing unattended
>installs comes up often enough that official support would be nice,
>IMHO.

I was referring to your change which handled versioned setup.ini's.
That is not a requirement for an unattended Cygwin install.

>>The code is already a complicated obtuse mishmash and adding more stuff
>>which would be used by only one person doesn't seem like a good idea.
>
>A self-fulfilling prophecy: nobody is doing it because nobody is doing
>it.  I'll have to add on whatever I need, if it's useless to you then
>just don't use it.

If there was a general hue and cry for the feature that you want to add,
I'd consider adding it.  AFAIK, there isn't.  If we did add it we would
be obligated to support it so, if there is a need for this type of
feature, I'd like to see a lot more discussion about why it's necessary
and the best way to implement it.

cgf

Re: [PATCH] setup.exe

[Achim Gratz]

Christopher Faylor writes:
> I'm really not too keen on adding hacks to setup so that people can
> use it for other than its intended purpose.

If there is another method to do an unattended Cygwin install, I'm all
ears.  I have briefly pondered to script a standalone installer, but it
requires too much infrastructure and I don't see much value in trying to
re-do what setup.exe already does.  The question of doing unattended
installs comes up often enough that official support would be nice,
IMHO.

> The code is already a complicated obtuse mishmash and adding more
> stuff which would be used by only one person doesn't seem like a good
> idea.

A self-fulfilling prophecy: nobody is doing it because nobody is doing
it.  I'll have to add on whatever I need, if it's useless to you then
just don't use it.



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Re: [PATCH] setup.exe

[Christopher Faylor]

On Sat, Jan 19, 2013 at 08:41:05AM +0100, Achim Gratz wrote:
>Christopher Faylor writes:
>> You'd really be much better served to submit one patch at a time.
>
>Noted.
>
>> If you look at bootstrap.sh, you will see why I'm puzzled why this should be
>> needed.  The script is intended to be run from the build directory which
>> should be separate from the source directory.
>
>In all the combinations I tried, this didn't result in a working build
>environment, but let me try again.  It is possible I got fooled by
>CVSgrab and there's still something missing in my source tree.
>
>> "strip" as a target is really not a good idea.
>
>I'm not wedded to the name and I really only intend to use "upx" myself
>(whose name is also up for grabs if you don't like it).
>
>> Who is the target audience for this?
>
>I'm in the process of updating our age-old "IT approved" Cygwin
>installation to 1.7.x.  One of the things I need to provide is the
>ability to "freeze" and "roll back" installations to some known state
>and do it from a script.

I'm really not too keen on adding hacks to setup so that people can
use it for other than its intended purpose.  The code is already
a complicated obtuse mishmash and adding more stuff which would
be used by only one person doesn't seem like a good idea.

cgf

Re: [PATCH] setup.exe

[Achim Gratz]

Christopher Faylor writes:
> You'd really be much better served to submit one patch at a time.

Noted.

> If you look at bootstrap.sh, you will see why I'm puzzled why this should be
> needed.  The script is intended to be run from the build directory which
> should be separate from the source directory.

In all the combinations I tried, this didn't result in a working build
environment, but let me try again.  It is possible I got fooled by
CVSgrab and there's still something missing in my source tree.

> "strip" as a target is really not a good idea.

I'm not wedded to the name and I really only intend to use "upx" myself
(whose name is also up for grabs if you don't like it).

> Who is the target audience for this?

I'm in the process of updating our age-old "IT approved" Cygwin
installation to 1.7.x.  One of the things I need to provide is the
ability to "freeze" and "roll back" installations to some known state
and do it from a script.  I have a (not quite finished yet) script
infrastructure that collects the necessary files into an install
hierarchy and re-writes setup.ini to match that.  The current plan is to
rename setup.ini to some unique name upon freeze/release and make sure
that the files referenced by all these ini files are kept along with any
updates that come with a new setup.ini.  That way I hope to fulfill that
requirement without unduly needing to duplicate gigabytes of data for
each release.  All this of course only works if "setup.ini" isn't
hard-coded into setup.

> It doesn't seem like it is worth complicating setup for this type of
> feature.

If you are talking about the interactive installation method I tend to
agree.

This change however is intended to add some support for non-interactive
installations (and I will likely request some more support for that
later on, like deleting packages from the command line).  The
alternative would be to put each setup.ini in its own directory and link
the install hierachy (that works, I've tested it), but the master copy
I'm producing will be replicated to some 45 distribution servers around
the world in a two- or sometimes three-stage process and I'm trying to
avoid testing the chances of the links surviving the replication (over
which I have no control other than saying "do it now" and I can't check
most of those servers easily).

But you are free of course to not wanting to support that and drop the
patch, in which case I'll carry it forward locally.  No big deal.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

Re: [PATCH] setup.exe

[Christopher Faylor]

On Fri, Jan 18, 2013 at 06:23:49PM +0100, Achim Gratz wrote:
>As requested by Corinna on the Cygwin list, here's a patch to document
>some recent changes in the build environment.

You'd really be much better served to submit one patch at a time.

>From 3dd23c6063a3edb8bfd1874f5b3c68baf0a89ec4 Mon Sep 17 00:00:00 2001
>From: Achim Gratz 
>Date: Fri, 18 Jan 2013 14:24:13 +0100
>Subject: [PATCH 1/3] README: document some recent changes in the build
> environment
>
>* setup/README (HOW TO BUILD): Cross compiler package is now named
>  mingw-gcc-g++, also mention package upx as an optional dependency.
>  Document the requirement of libgetopt++ as a subdirectory in the
>  main source tree.  Change the bootstrap stanza to take place in the
>  source tree since an out-of-tree build doesn't seem to work (if it
>  does, it apparently has additional requirements that I haven't
>  figured out).
>---
> setup/README | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/setup/README b/setup/README
>index 2f36fb8..d5b0240 100755
>--- a/setup/README
>+++ b/setup/README
>@@ -5,7 +5,7 @@ HOW TO BUILD:
> -
> Setup should build out-of-the-box on any Cygwin environment that has all the
> required packages installed:
>-  - gcc-mingw-g++
>+  - mingw-gcc-g++
>   - make
>   - mingw-bzip2
>   - mingw-libgcrypt-devel
>@@ -13,6 +13,7 @@ required packages installed:
>   - mingw-zlib
>   - and all packages that are dependencies of the above, i.e.  gcc-mingw-core,
> mingw-runtime, binutils, w*api, etc.
>+  - upx (optional)
> 
> The following additional packages are required if building from CVS, not from
> a source tarball, or if you want to make changes to the build system.
>@@ -22,9 +23,14 @@ a source tarball, or if you want to make changes to the 
>build system.
>   - flex
>   - bison
> 
>+Additionally, libgetopt++ (also available from the cygwin-apps CVS at
>+sourceware.org) must be available directly as a subdirectory
>+libgetopt++ within the setup source directory.
>+
> Build commands:
> 1) Configure using this option
>-   $ /path/to/setup/bootstrap.sh
>+   $ cd /path/to/setup
>+   $ ./bootstrap.sh

If you look at bootstrap.sh, you will see why I'm puzzled why this should be
needed.  The script is intended to be run from the build directory which
should be separate from the source directory.

>A second patch adds two new targets for the makefile to strip and
>compress setup.exe.

"strip" as a target is really not a good idea.

>I#ll also offer a third patch that adds an option to setup.exe to enable
>the use of (local) ini files with a different basename than "setup".
>This is useful to offer (in the same install hierarchy) multiple
>installations, either for having different configurations or to allow
>individual installations to be rolled back.  The idea is to copy the
>setup.ini to something like release_2013-01-18.ini whenever a release is
>made and let setup.ini keep going forward for testing without having to
>duplicate the complete install hierarchy.

Who is the target audience for this?  It doesn't seem like it is worth
complicating setup for this type of feature.

cgf

Re: [PATCH] setup.exe

[Ken Brown]

On 1/18/2013 12:23 PM, Achim Gratz wrote:

+Additionally, libgetopt++ (also available from the cygwin-apps CVS at
+sourceware.org) must be available directly as a subdirectory
+libgetopt++ within the setup source directory.


I don't understand why you need this part.  Checking out setup 
automatically gives you the libgetopt++ subdirectory.


Ken

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Christopher Faylor]

On Thu, Jan 17, 2013 at 05:22:33PM +, Jon TURNEY wrote:
>On 14/09/2012 01:43, Warren Young wrote:
>> On 9/13/2012 5:09 AM, Jon TURNEY wrote:
>>> On 13/09/2012 03:23, Warren Young wrote:
 5. Several build system files refer to iniparse.h, but on my system,
 iniparse.yy yields iniparse.hh, not .h.
>>>
>>> See the note on a "Slightly backward-incompatible change" in the section
>>> "Changes to Yacc and Lex support" in [1]
>> 
>> Lovely.
>> 
>> I don't see a clean solution short of demanding that everyone be on Automake
>> 1.12 then.  Symlink hackery only works when you know which versino of 
>> Automake
>> you are running.  I guess bootstrap.sh could detect Automake 1.11 and below,
>> but ick.
>
>Indeed.
>
>Attached is a patch which fixes things for and requires automake >=1.12.
>
>(Note that automake has some magic to select the version of automake which was
>previously run in a directory, rather than just selecting the latest version
>installed, so you'll need to distclean or 'WANT_AUTOMAKE=1.12 ./bootstrap.sh'
>to build in an already configured directory after applying this patch)
>

>From 478c40eeaa5d6fda2f527072863b5f393ffeca2a Mon Sep 17 00:00:00 2001
>From: Jon TURNEY 
>Date: Thu, 17 Jan 2013 15:13:11 +
>Subject: [PATCH] Require automake 1.12 for consistent naming of generated
> file
>
>Automake <=1.11 and >=1.12 have different behaviour (.h vs .hh) for naming the
>C++ header file generated from a .yy file.
>
>Adapt for the the behaviour of automake 1.12, and require it.
>
>2013-01-17  Jon TURNEY  
>
>   * configure.in: Require automake 1.12.
>   * Makefile.am (BUILT_SOURCES): Update iniparse.h to iniparse.hh.
>   * iniparse.yy: Ditto.
>   * inilex.ll: Ditto.

Go ahead and check in.  Thanks.

cgf

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Jon TURNEY]

On 14/09/2012 01:43, Warren Young wrote:
> On 9/13/2012 5:09 AM, Jon TURNEY wrote:
>> On 13/09/2012 03:23, Warren Young wrote:
>>> 5. Several build system files refer to iniparse.h, but on my system,
>>> iniparse.yy yields iniparse.hh, not .h.
>>
>> See the note on a "Slightly backward-incompatible change" in the section
>> "Changes to Yacc and Lex support" in [1]
> 
> Lovely.
> 
> I don't see a clean solution short of demanding that everyone be on Automake
> 1.12 then.  Symlink hackery only works when you know which versino of Automake
> you are running.  I guess bootstrap.sh could detect Automake 1.11 and below,
> but ick.

Indeed.

Attached is a patch which fixes things for and requires automake >=1.12.

(Note that automake has some magic to select the version of automake which was
previously run in a directory, rather than just selecting the latest version
installed, so you'll need to distclean or 'WANT_AUTOMAKE=1.12 ./bootstrap.sh'
to build in an already configured directory after applying this patch)

>From 478c40eeaa5d6fda2f527072863b5f393ffeca2a Mon Sep 17 00:00:00 2001
From: Jon TURNEY 
Date: Thu, 17 Jan 2013 15:13:11 +
Subject: [PATCH] Require automake 1.12 for consistent naming of generated
 file

Automake <=1.11 and >=1.12 have different behaviour (.h vs .hh) for naming the
C++ header file generated from a .yy file.

Adapt for the the behaviour of automake 1.12, and require it.

2013-01-17  Jon TURNEY  

* configure.in: Require automake 1.12.
* Makefile.am (BUILT_SOURCES): Update iniparse.h to iniparse.hh.
* iniparse.yy: Ditto.
* inilex.ll: Ditto.

Signed-off-by: Jon TURNEY 
---
 Makefile.am  |4 ++--
 configure.in |2 +-
 inilex.ll|2 +-
 iniparse.yy  |2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 621c8ff..0f1498b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -54,10 +54,10 @@ EXTRA_DIST = \
tree-plus.bmp
 
-# iniparse.h is generated from iniparse.yy via bison -d, so it needs to be
+# iniparse.hh is generated from iniparse.yy via bison -d, so it needs to be
 # included here for proper tracking (but not iniparse.cc, since automake
 # knows about that already)
 BUILT_SOURCES = \
setup_version.c \
-   iniparse.h
+   iniparse.hh
 
 CLEANFILES = setup_version.c
diff --git a/configure.in b/configure.in
index 37a3ef2..566a3fd 100644
--- a/configure.in
+++ b/configure.in
@@ -21,5 +21,5 @@ 
 AC_PREREQ(2.60)
 AC_CONFIG_AUX_DIR([cfgaux])
-AM_INIT_AUTOMAKE([subdir-objects foreign no-define -Wall -Wno-portability])
+AM_INIT_AUTOMAKE([1.12 subdir-objects foreign no-define -Wall 
-Wno-portability])
 dnl AM_CONFIG_HEADER(include/autoconf.h)
 m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES(yes)])
diff --git a/inilex.ll b/inilex.ll
index b119049..a70b5b5 100644
--- a/inilex.ll
+++ b/inilex.ll
@@ -24,5 +24,5 @@
 
 #include "ini.h"
-#include "iniparse.h"
+#include "iniparse.hh"
 #include "String++.h"
 #include "IniParseFeedback.h"
diff --git a/iniparse.yy b/iniparse.yy
index c8332ff..684fc47 100644
--- a/iniparse.yy
+++ b/iniparse.yy
@@ -20,5 +20,5 @@
 #include "win32.h"
 #include "ini.h"
-#include "iniparse.h"
+#include "iniparse.hh"
 #include "PackageTrust.h"
 
-- 
1.7.9

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Corinna Vinschen]

On Sep 13 11:26, Christopher Faylor wrote:
> On Wed, Sep 12, 2012 at 10:08:36PM -0600, Eric Blake wrote:
> >On 09/12/2012 09:42 PM, Warren Young wrote:
> >> On 9/12/2012 9:28 PM, Christopher Faylor wrote:
> >>>
> >>> If you're going to do that you really should (re)learn how to submit a
> >>> proper patch.  A patch uses "diff -u" format and contains a ChangeLog.
> >> 
> >> It's been a decade since I last used "cvs diff".  I forgot that it
> >> doesn't include -u, since svn diff does it by default.
> >
> >cvs does it automatically if you supply your ~/.cvsrc with correct
> >contents... What I miss is that even with ~/.cvsrc, you still don't get
> >the automatic paging that 'git diff' gives by default.
> >
> >Yes, cygwin is one of the last holdouts still using CVS, making it more
> >likely that you don't notice an incomplete ~/.cvsrc on new machines as
> >fast.  I'd certainly welcome a conversion to git (and tolerate a
> >conversion to svn); especially since I know that newlib recently enabled
> >a git mirror.  And there's ways of using git as your frontend to a CVS
> >(or svn) repository.
> 
> I did spend a lot of time getting a standalone git repository ready for
> Cygwin a while ago.  I didn't do anything with setup though.
> 
> Since nothing really relies on cygwin-apps, I wouldn't mind moving it over
> to git as long as Corinna agrees.

Since everybody seem to like git more than CVS, go ahead.


Corinna

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

Re: [PATCH] setup.exe case-insensitive package sorting

[Christopher Faylor]

On Wed, Sep 12, 2012 at 10:46:02PM -0600, Warren Young wrote:
>This patch is incomplete, but I've been fighting this for four hours 
>now.  I'm tired, hungry, and frustrated, so in case I don't get back to 
>finishing this, I decided to just post what I have so far.
>
>The point of the patch is to get the Select Packages screen's package 
>lists to sort case-insensitively.
>
>(This saga began after I spent far too long looking for the R package 
>down in the bottom third of the Interpreters category.)
>
>The patch works for the flat views only.  I have yet to figure out how 
>to do the same for the Category tree view, since that is treated as a 
>completely separate exception case in the code.
>
>(Any setup.exe hackers want to opine on whether it's reasonable for it 
>to have required 3 hours of spelunking through the package DB reader, 
>the INI file parser, and the dialog chaining system to find out where to 
>even begin attempting the patch?  Did I make par?  Was there a map to 
>this twisty maze of global variables that I missed?)

You want us to weigh in on your being tired and frustrated and unable
to figure out source code?  I'm going to carefully lay down that loaded
weapon.

I modified "upset" to do case insensitive sorting of packages.  This seems
preferable to adding YA level of complexity to setup.

cgf

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Warren Young]

On 9/13/2012 5:09 AM, Jon TURNEY wrote:

On 13/09/2012 03:23, Warren Young wrote:

5. Several build system files refer to iniparse.h, but on my system,
iniparse.yy yields iniparse.hh, not .h.


See the note on a "Slightly backward-incompatible change" in the section
"Changes to Yacc and Lex support" in [1]


Lovely.

I don't see a clean solution short of demanding that everyone be on 
Automake 1.12 then.  Symlink hackery only works when you know which 
versino of Automake you are running.  I guess bootstrap.sh could detect 
Automake 1.11 and below, but ick.

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Christopher Faylor]

On Wed, Sep 12, 2012 at 10:08:36PM -0600, Eric Blake wrote:
>On 09/12/2012 09:42 PM, Warren Young wrote:
>> On 9/12/2012 9:28 PM, Christopher Faylor wrote:
>>>
>>> If you're going to do that you really should (re)learn how to submit a
>>> proper patch.  A patch uses "diff -u" format and contains a ChangeLog.
>> 
>> It's been a decade since I last used "cvs diff".  I forgot that it
>> doesn't include -u, since svn diff does it by default.
>
>cvs does it automatically if you supply your ~/.cvsrc with correct
>contents... What I miss is that even with ~/.cvsrc, you still don't get
>the automatic paging that 'git diff' gives by default.
>
>Yes, cygwin is one of the last holdouts still using CVS, making it more
>likely that you don't notice an incomplete ~/.cvsrc on new machines as
>fast.  I'd certainly welcome a conversion to git (and tolerate a
>conversion to svn); especially since I know that newlib recently enabled
>a git mirror.  And there's ways of using git as your frontend to a CVS
>(or svn) repository.

I did spend a lot of time getting a standalone git repository ready for
Cygwin a while ago.  I didn't do anything with setup though.

Since nothing really relies on cygwin-apps, I wouldn't mind moving it over
to git as long as Corinna agrees.

cgf

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Jon TURNEY]

On 13/09/2012 03:23, Warren Young wrote:
> 5. Several build system files refer to iniparse.h, but on my system,
> iniparse.yy yields iniparse.hh, not .h.  In a fresh CVS checkout, this causes:
> 
>   $ ./bootstrap.sh
>   ...noise noise noise...
>   $ make
> GEN  setup_version.c
>   make: *** No rule to make target `iniparse.h', needed by `all'.  Stop.
> 
> I was able to fix it with:
> 
>   $ ln -s iniparse.hh iniparse.h
>   $ make iniparse.hh
>   $ make
> 
> If you skip the second step, make(1) yells again, because iniparse.hh hasn't
> been created yet.  You have to force it to create it out of order, since the
> Makefile dependencies are satisfied by the newly-created iniparse.h symlink.
> 
> Having done those two things, you don't need to repeat the hack, even after
> "make distclean", since that doesn't remove either iniparse.hh or iniparse.h. 
> But, I think the correct fix is to just change all the iniparse.h references
> to iniparse.hh.
> 
> That makes me wonder how anyone else has been getting setup.exe to build OOTB
> for the past five years?  (cvs log -r1.1 iniparse.yy)
> 
> Do the current setup.exe maintainers have such hack symlinks on their systems
> already, and haven't needed to do any fresh checkouts in all that time?  Or is
> this a recent Bison change?

Having run across this issue on another project, I think this is due to a
recent, and rather annoying change in automake.

automake 1.12 has this behaviour, automake 1.11 and previous made iniparse.h

See the note on a "Slightly backward-incompatible change" in the section
"Changes to Yacc and Lex support" in [1]

[1] http://lists.gnu.org/archive/html/automake/2012-04/msg00060.html

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Eric Blake]

On 09/12/2012 09:42 PM, Warren Young wrote:
> On 9/12/2012 9:28 PM, Christopher Faylor wrote:
>>
>> If you're going to do that you really should (re)learn how to submit a
>> proper patch.  A patch uses "diff -u" format and contains a ChangeLog.
> 
> It's been a decade since I last used "cvs diff".  I forgot that it
> doesn't include -u, since svn diff does it by default.

cvs does it automatically if you supply your ~/.cvsrc with correct
contents... What I miss is that even with ~/.cvsrc, you still don't get
the automatic paging that 'git diff' gives by default.

Yes, cygwin is one of the last holdouts still using CVS, making it more
likely that you don't notice an incomplete ~/.cvsrc on new machines as
fast.  I'd certainly welcome a conversion to git (and tolerate a
conversion to svn); especially since I know that newlib recently enabled
a git mirror.  And there's ways of using git as your frontend to a CVS
(or svn) repository.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Warren Young]

On 9/12/2012 9:28 PM, Christopher Faylor wrote:


If you're going to do that you really should (re)learn how to submit a
proper patch.  A patch uses "diff -u" format and contains a ChangeLog.


It's been a decade since I last used "cvs diff".  I forgot that it 
doesn't include -u, since svn diff does it by default.


Sorry about the lack of a ChangeLog entry.

Re: [PATCH] setup.exe build instructions outdated; build doesn't bootstrap cleanly

[Christopher Faylor]

On Wed, Sep 12, 2012 at 08:23:58PM -0600, Warren Young wrote:
>It's that time of decade again...time for me to patch setup.exe again.

If you're going to do that you really should (re)learn how to submit a
proper patch.  A patch uses "diff -u" format and contains a ChangeLog.

But, thanks for the heads up.  I'll make appropriate changes.

cgf

Re: PATCH: setup.exe 2.693 installation status incompletely shown in Create Icons/Installation Status screen

[Matthias Andree]

Am 07.04.2010, 18:33 Uhr, schrieb Corinna Vinschen:


On Apr  7 16:01, Matthias Andree wrote:

Greetings,

I propose the attached patch to fix the two issues (installation
status truncated, copyright outdated) I reported on cygwin@ (see
forward below), and a third issue, missing lzma and gcrypt from
README. diffstat:

 ChangeLog |   11 +++
 README|4 +++-
 res.rc|8 
 3 files changed, 18 insertions(+), 5 deletions(-)

The patch is actually against ~2.695 from CVS (already includes
Corinna's fix for privileged rename on reboot).


Thanks, applied.  Just a note for the next time.  Please include the
ChangeLog entry only in cleartext in your mail, not as diff.  In this
case it was easy, but most of the time ChangeLogs don't apply cleanly.


Thanks. Will do.

--
Matthias Andree

Re: PATCH: setup.exe 2.693 installation status incompletely shown in Create Icons/Installation Status screen

[Christopher Faylor]

On Wed, Apr 07, 2010 at 06:33:43PM +0200, Corinna Vinschen wrote:
>On Apr  7 16:01, Matthias Andree wrote:
>> Greetings,
>> 
>> I propose the attached patch to fix the two issues (installation
>> status truncated, copyright outdated) I reported on cygwin@ (see
>> forward below), and a third issue, missing lzma and gcrypt from
>> README. diffstat:
>> 
>>  ChangeLog |   11 +++
>>  README|4 +++-
>>  res.rc|8 
>>  3 files changed, 18 insertions(+), 5 deletions(-)
>> 
>> The patch is actually against ~2.695 from CVS (already includes
>> Corinna's fix for privileged rename on reboot).
>
>Thanks, applied.  Just a note for the next time.  Please include the
>ChangeLog entry only in cleartext in your mail, not as diff.  In this
>case it was easy, but most of the time ChangeLogs don't apply cleanly.

Thanks.

This will be in the version of setup.exe that I will upload.

FYI.

cgf

Re: PATCH: setup.exe 2.693 installation status incompletely shown in Create Icons/Installation Status screen

[Corinna Vinschen]

On Apr  7 16:01, Matthias Andree wrote:
> Greetings,
> 
> I propose the attached patch to fix the two issues (installation
> status truncated, copyright outdated) I reported on cygwin@ (see
> forward below), and a third issue, missing lzma and gcrypt from
> README. diffstat:
> 
>  ChangeLog |   11 +++
>  README|4 +++-
>  res.rc|8 
>  3 files changed, 18 insertions(+), 5 deletions(-)
> 
> The patch is actually against ~2.695 from CVS (already includes
> Corinna's fix for privileged rename on reboot).

Thanks, applied.  Just a note for the next time.  Please include the
ChangeLog entry only in cleartext in your mail, not as diff.  In this
case it was easy, but most of the time ChangeLogs don't apply cleanly.


Thanks again,
Corinna

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

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Dave Korn]

Christopher Faylor wrote:
> On Wed, Nov 04, 2009 at 12:57:05PM -0500, Christopher Faylor wrote:
>> On Wed, Nov 04, 2009 at 06:05:27PM +, Dave Korn wrote:
>>> Christopher Faylor wrote:

 Why not add a new option entirely or some sort of different syntax for
 categories like "Net:" or something?
>>> I think it's probably simplest to add an extra option.  Will be up with
>>> a respin shortly.
>> Thanks.
> 
> Btw, feel free to check that in.  I don't see any reason for another round
> trip on this.

  One down, one to go.  I used '-C'/'--categories' in the end, FTR.

cheers,
  DaveK

Re: [PATCH] Setup.exe: Nicer local package dir browser.

[Dave Korn]

Corinna Vinschen wrote:
> On Nov  4 17:47, Dave Korn wrote:
>>   Overall I think this is a much more friendly user experience.  OK for head?
> 
> In theory, yes, with two tweaks...
> 
>> Index: localdir.cc
>> [...]
>> @@ -213,6 +252,12 @@ LocalDirPage::OnNext ()
>>return IDD_CHOOSE;
>>  }
>>  }
>> +  else if (GetLastError () == ERROR_FILE_NOT_FOUND)
> 
> Shouldn't this also test for ERROR_PATH_NOT_FOUND?

  Yes, I think it should, as I just noticed while replying to Thomas Wolff's
post earlier ...

>> +{
>> +  if (offer_to_create (GetHWND (), local_dir.c_str()))
> 
> And, the directory should only be created if source != IDC_SOURCE_CWD.
> If you "Install from Local Directory", the directory should exist for
> hopefully obvious reasons.

  I tend toward the give-'em-enough-rope school of coding, which says that if
someone deliberately enters the name of a non-existent directory, and then
clicks a button to agree when you offer to create it, what they are saying is
that they want you to create a fresh empty local package directory for them
and install nothing from it.  But I'll make it fail there instead of offer to
create if you think that's for the best.

cheers,
  DaveK

Re: [PATCH] Setup.exe: Nicer local package dir browser.

[Corinna Vinschen]

On Nov  4 17:47, Dave Korn wrote:
>   Overall I think this is a much more friendly user experience.  OK for head?

In theory, yes, with two tweaks...

> Index: localdir.cc
> [...]
> @@ -213,6 +252,12 @@ LocalDirPage::OnNext ()
> return IDD_CHOOSE;
>   }
>   }
> +  else if (GetLastError () == ERROR_FILE_NOT_FOUND)

Shouldn't this also test for ERROR_PATH_NOT_FOUND?

> + {
> +   if (offer_to_create (GetHWND (), local_dir.c_str()))

And, the directory should only be created if source != IDC_SOURCE_CWD.
If you "Install from Local Directory", the directory should exist for
hopefully obvious reasons.


Thanks,
Corinna

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

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Christopher Faylor]

On Wed, Nov 04, 2009 at 12:15:26PM -0600, Yaakov (Cygwin/X) wrote:
>On 04/11/2009 12:05, Dave Korn wrote:
>>Do we actually have any packages that have the same names as categories?
>
>perl
>python
>
>The categories are Capitalized where the packages are not, but I don't 
>think we want to go there.

Aha.  Good point.  Thanks for mentioning this.

cgf

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Yaakov (Cygwin/X)]

On 04/11/2009 12:05, Dave Korn wrote:

   Do we actually have any packages that have the same names as categories?


perl
python

The categories are Capitalized where the packages are not, but I don't 
think we want to go there.



Yaakov

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Christopher Faylor]

On Wed, Nov 04, 2009 at 12:57:05PM -0500, Christopher Faylor wrote:
>On Wed, Nov 04, 2009 at 06:05:27PM +, Dave Korn wrote:
>>Christopher Faylor wrote:
>>>Unfortunately, no, I don't think so.  I like the idea but I don't like
>>>overloading "-p" as it could cause confusion.
>>
>>Do we actually have any packages that have the same names as
>>categories?  I guess we might do one day even if we don't now, but I
>>didn't see the danger of confusion as very great.
>
>No, but it has been a discontiguous namespace and I think it makes sense
>to keep it that way.
>
>>>Why not add a new option entirely or some sort of different syntax for
>>>categories like "Net:" or something?
>>
>>I think it's probably simplest to add an extra option.  Will be up with
>>a respin shortly.
>
>Thanks.

Btw, feel free to check that in.  I don't see any reason for another round
trip on this.

cgf

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Christopher Faylor]

On Wed, Nov 04, 2009 at 06:05:27PM +, Dave Korn wrote:
>Christopher Faylor wrote:
>>Unfortunately, no, I don't think so.  I like the idea but I don't like
>>overloading "-p" as it could cause confusion.
>
>Do we actually have any packages that have the same names as
>categories?  I guess we might do one day even if we don't now, but I
>didn't see the danger of confusion as very great.

No, but it has been a discontiguous namespace and I think it makes sense
to keep it that way.

>>Why not add a new option entirely or some sort of different syntax for
>>categories like "Net:" or something?
>
>I think it's probably simplest to add an extra option.  Will be up with
>a respin shortly.

Thanks.

cgf

Re: [PATCH] Setup.exe: restore commandline localdir option.

[Christopher Faylor]

On Wed, Nov 04, 2009 at 06:02:14PM +, Dave Korn wrote:
>Christopher Faylor wrote:
>
>> I think the original semantics are quite a bit less surprising and the option
>> should be checked first.
>
>  Figured that might be the case, so here it is the other way.
>
>   * localdir.cc (LocalDirSetting::LocalDirSetting): Restore -l option.
>
>  OK now?

Yes.  Thanks.

cgf

Re: [PATCH] Setup.exe: Nicer local package dir browser.

[Dave Korn]

Christopher Faylor wrote:

> I think so but maybe Corinna should ok this since I assume that it changes
> her recently checked in change.

  Yes, it reverts part of it:

>>  It also resolves the what-to-do-when-the-directory-doesn't-exist problem
>> slightly differently.  I didn't want to just automatically create the
>> directory, because I think one of the very common reasons for a directory 
>> that
>> someone has typed into a textbox not to exist is that they typed it wrong, so
>> I wanted to implement a dialog to offer them the choice of creating it or 
>> not.

  Keeps the improved title dialog choice though.

cheers,
  DaveK

Re: [PATCH] Setup.exe: Nicer local package dir browser.

[Christopher Faylor]

On Wed, Nov 04, 2009 at 05:47:48PM +, Dave Korn wrote:
>
>Are we having fun yet?
>
>  This patch makes the local package dir browsing experience nicer.  (It also
>rolls up the previous patch about fixing the localdir option, sorry for being
>lazy; just ignore that very first hunk for the moment.  I could have manually
>chopped it out but then the line numbers would have been off-by-two and I
>could have manually fixed the line numbers but basically I don't like
>falsifying patches because nasty surprises can result!  Chopping off the top
>hunk - or several - of a patch on its own, as I did to generate the other
>patch, is as far as I'm comfortable with post-editing diffs.)
>
>  To do this, it adds a couple of style flags that cause the common controls
>directory browser dialog to adopt the newer styles available on all 2k and
>later OSs, and a browser callback hook in support.  I believe on NT it will
>just harmlessly fall back to the old-style dialog, but hope someone can check
>for me.
>
>  It also resolves the what-to-do-when-the-directory-doesn't-exist problem
>slightly differently.  I didn't want to just automatically create the
>directory, because I think one of the very common reasons for a directory that
>someone has typed into a textbox not to exist is that they typed it wrong, so
>I wanted to implement a dialog to offer them the choice of creating it or not.
> This dialog can also be conveniently reused from within the browser, where
>there's also an editbox to type directory names.
>
>   * localdir.cc (offer_to_create): New function.
>   (browse_cb): Use it to optionally create a directory if the name
>   entered in the textbox fails validation.
>   (browse): Add new dialog style flags and validated editbox.
>   (LocalDirPage::OnNext): Don't automatically create local package
>   dir if it doesn't already exist; offer to instead.
>   * resource.h (IDS_MAYBE_MKDIR): Define new string resource ID.
>   * res.rc (IDS_MAYBE_MKDIR): Define new string resource text.
>
>
>  Overall I think this is a much more friendly user experience.  OK for head?

I think so but maybe Corinna should ok this since I assume that it changes her
recently checked in change.

cgf

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Dave Korn]

Christopher Faylor wrote:

> Unfortunately, no, I don't think so.  I like the idea but I don't like 
> overloading
> "-p" as it could cause confusion.  

  Do we actually have any packages that have the same names as categories?  I
guess we might do one day even if we don't now, but I didn't see the danger of
confusion as very great.

> Why not add a new option entirely or some sort
> of different syntax for categories like "Net:" or something?

  I think it's probably simplest to add an extra option.  Will be up with a
respin shortly.

cheers,
  DaveK

Re: [PATCH] Setup.exe: restore commandline localdir option.

[Dave Korn]

Christopher Faylor wrote:

> I think the original semantics are quite a bit less surprising and the option
> should be checked first.

  Figured that might be the case, so here it is the other way.

* localdir.cc (LocalDirSetting::LocalDirSetting): Restore -l option.

  OK now?

cheers,
  DaveK
Index: localdir.cc
===
RCS file: /cvs/cygwin-apps/setup/localdir.cc,v
retrieving revision 2.29
diff -p -u -r2.29 localdir.cc
--- localdir.cc	4 Nov 2009 15:14:51 -	2.29
+++ localdir.cc	4 Nov 2009 17:43:55 -
@@ -62,7 +62,9 @@ static ControlAdjuster::ControlInfo Loca
 LocalDirSetting::LocalDirSetting ()
 {
   const char *fg_ret;
-  if ((fg_ret = UserSettings::instance().get ("last-cache")))
+  if (std::string (LocalDirOption).size ())
+local_dir = std::string (LocalDirOption);
+  else if ((fg_ret = UserSettings::instance().get ("last-cache")))
 local_dir = std::string (fg_ret);
 }
 

Re: [PATCH] Setup.exe: allow -p option to specify categories as well as packages

[Christopher Faylor]

On Wed, Nov 04, 2009 at 05:47:55PM +, Dave Korn wrote:
>
>Last but not least,
>
>  I found myself wanting to run setup in unattended mode to install absolutely
>everything, so I figured the nicest solution was to allow the -p option to
>accept category names as well as package names, so that I could use "-p All".
> The attached patch does just that.
>
>   * package_meta.cc (packagemeta::isManuallyWanted): Allow category
>   names as well as individual package names in packages_option.
>   * package_db.cc (packagedb::fillMissingCategory): Don't check for
>   manually-wanted packages until after initialising all the categories.
>
>  Tested by running it a few times with no -p options and then with -p options
>for things like "Net" and/or a couple of individual packages mixed in.  OK?

Unfortunately, no, I don't think so.  I like the idea but I don't like 
overloading
"-p" as it could cause confusion.  Why not add a new option entirely or some 
sort
of different syntax for categories like "Net:" or something?

cgf

Re: [PATCH] Setup.exe: New commandline option --only-site

[Christopher Faylor]

On Wed, Nov 04, 2009 at 05:47:32PM +, Dave Korn wrote:
>
>Hello perverts and non-perverts alike!
>
>  Having recently become a pervert, oops I mean provider, I wanted to provide
>a full turnkey installation on a DVD for offline use, and to avoid confusing
>any cygn00bs among them, I wanted it to have a setup.exe that would just
>install from the provided disk and not scare them with any of that stuff about
>external mirrors.  I also wanted it to work regardless of whether there was an
>existing installation on the local drive or not, and when there was one,
>setup.exe would of course pick up the user settings from it, most of which is
>desirable, but I didn't want them to be shown a whole long list of mirrors and
>have to do something potentially offputting like multi-selecting stuff, nor
>want them to be confused by all these external sites showing up (perhaps not
>realising that the local mirror will have been added right at the very bottom
>of the list and won't be visible without scrolling down).
>
>  So, the --only-site option lets you invoke setup.exe in locked-down mode
>where the only site it shows is the one you've specified on the command-line
>using the -s option.  It doesn't bother to download the mirrors list, but it
>doesn't update the local mirrors list either, so it won't wipe out any
>existing one.  I didn't make it disable the "Add" button or skip the mirror
>page altogether, because I didn't want to take away the user's "get out of
>jail free card" for when something goes wrong, or if they want to change from
>installing from a local DVD to installing from a network-drive copy of the
>install disk, and besides if you want that then what you're really looking for
>is probably to use full-blown unattended mode, but maybe we might add another
>option in the future that specifies "Unattended mode for everything except the
>package chooser page"?  Anyway, here it is for your consideration.
>
>   * site.cc: Add #include of BoolOption header from libgetopt.
>   (OnlySiteOption): New BoolOption for command-line -O option.
>   (get_site_list): Respect BoolOption by not downloading mirror
>   list, nor using nor updating cached mirrors.
>
>  OK?

Yes.  Looks like a nice addition.

cgf

Re: [PATCH] Setup.exe: don't download forever in unattended mode.

[Christopher Faylor]

On Wed, Nov 04, 2009 at 05:47:15PM +, Dave Korn wrote:
>
>Hi all,
>
>  In unattended mode, setup.exe automatically answers "yes" to all message
>boxes.  If the message box is asking whether to retry an incomplete download,
>and the reason for the incomplete download is something non-transient, like
>the localhost or the mirror going offline, or a file is missing from the
>mirror, then it'll happily run forever.  This patch adds a simple retry limit
>to the download stage in unattended mode; if it takes more than five goes, it
>gives up.
>
>   * download.cc (do_download_thread): Only retry an incomplete download
>   a limited number of times in unattended mode.
>
>  Tested on a local mirror, by removing a package file.  OK?

Yes, thanks.

cgf

Re: [PATCH] Setup.exe: restore commandline localdir option.

[Christopher Faylor]

On Wed, Nov 04, 2009 at 05:47:21PM +, Dave Korn wrote:
>
>
>Hi again!
>
>  During the recent rejig of settings handling, the command-line "-l" option
>(aka "--local-package-dir") got borked.  This patch restores it, but it might
>not be quite what's wanted, because it changes the semantics slightly; that's
>just the way I happened to want it to work for the stuff I was developing when
>I noticed this bug, so I've sent it that way; it would be equally trivial to
>restore the original semantics if that's preferred.
>
>  In the original semantics, -l specifies a local package dir to use,
>overriding any existing local package dir stored in the user settings.  In the
>new semantics, if there is an existing local package dir on the machine, that
>takes priority.  Like I said, that's just the way I wanted it for my purposes,
>so I didn't have to bother checking (in my enclosing installer, I'm a 3PP :-O
>now!) if there already was one and setting the -l option I invoke setup.exe
>with differently.  For upstream setup.exe, we probably want to keep the
>original semantics but I thought I'd throw it open.
>
>   * localdir.cc (LocalDirSetting::LocalDirSetting): Restore -l option.
>
>  OK this way?  Or the other way round, with the check for LocalDirOption
>moved before the check for last-cache, as it was in the original semantics?

I think the original semantics are quite a bit less surprising and the option
should be checked first.

cgf

>Index: localdir.cc
>===
>RCS file: /cvs/cygwin-apps/setup/localdir.cc,v
>retrieving revision 2.29
>diff -p -u -r2.29 localdir.cc
>--- localdir.cc4 Nov 2009 15:14:51 -   2.29
>+++ localdir.cc4 Nov 2009 16:32:42 -
>@@ -64,6 +64,8 @@ LocalDirSetting::LocalDirSetting ()
>   const char *fg_ret;
>   if ((fg_ret = UserSettings::instance().get ("last-cache")))
> local_dir = std::string (fg_ret);
>+  else if (std::string (LocalDirOption).size ())
>+local_dir = std::string (LocalDirOption);
> }
> 
> void

Re: [PATCH] Setup.exe package search/filter (PING cgf)

[Dave Korn]

Christopher Faylor wrote:

> If you are talking about the commenting out of psd.hasMinRect = true;
> then it looks like that was unintentional.

  Yep, that's exactly what I meant.

> Looks good.  Thanks.

  Thanks, I shall commit it.

cheers,
  DaveK

Re: [PATCH] Setup.exe package search/filter (PING cgf)

[Christopher Faylor]

On Fri, Apr 24, 2009 at 08:31:23PM +0100, Dave Korn wrote:
>Dave Korn wrote:
>>   So here's an updated and retested version of the patch done under the
>> assumption that reverting that change will be acceptable; if not, I can find 
>> a
>> different way of doing it.  Ok for head?
>
>  Doh, no, obviously not, I left a bunch of debugging in.  Hang on ten
>minutes, I'll resend.

Huh.  I guess I thought those were intentional.

Still approved.

cgf

Re: [PATCH] Setup.exe package search/filter (PING cgf)

[Christopher Faylor]

On Fri, Apr 24, 2009 at 08:30:36PM +0100, Dave Korn wrote:
>
>  Prompted by Marcio (thanks!), I went back to the setup.exe filter search 
> patch.
>
>  The one outstanding issue, you may remember, was the need to set a minimum
>size for the setup dialog, to prevent horrible ugliness when the controls
>overdrew each other or went outside the client area.
>
>On investigation, it turns out that we already had a serviceable
>minimum-size mechanism, which was disabled in r2.10 of propsheet.cc
>when cgf was adding the auto-maximization of the chooser page.  The
>ChangeLog comment doesn't refer to this, so PING cgf: was this an
>inadvertent change?  I tried reverting it and it doesn't appear to be
>necessary for the auto-maximization to function correctly, nor could I
>see anything wrong with using the initial layout size as a minimum.

If you are talking about the commenting out of psd.hasMinRect = true;
then it looks like that was unintentional.

>  So here's an updated and retested version of the patch done under the
>assumption that reverting that change will be acceptable; if not, I can find a
>different way of doing it.  Ok for head?
>
>
>2009-03-27  Andrew Punch  
>
>   * PickView.h:  Add #include .
>   (PickView::SetPackageFilter):  Add new function.
>   (PickView::packageFilterString):  Add new string data member.
>   * PickView.cc (PickView::setViewMode):  Use it to filter names.
>   (PickView::insert_category):  Likewise.
>   (PickView::PickView):  Initialise packageFilterString to blank.
>   * res.rc (IDD_CHOOSE):  Add IDC_CHOOSE_SEARCH_EDIT and
>   IDC_CHOOSE_SEARCH_LABEL controls.
>   (IDS_SEARCH_TOOLTIP):  Add new string resource.
>   * resource.h (IDS_SEARCH_TOOLTIP):  New string resource ID.
>   (IDC_CHOOSE_SEARCH_EDIT):  New edit control ID.
>   (IDC_CHOOSE_SEARCH_LABEL):  Mew static text control ID.
>   * choose.cc (ChooserControlsInfo[]):  Add IDC_CHOOSE_SEARCH_LABEL
>   and IDC_CHOOSE_SEARCH_EDIT controls to auto-resize list.
>   (ChooserPage::OnInit):  Add tooltip to search edit box.
>   (ChooserPage::OnMessageCmd):  Handle EN_CHANGE event from
>   IDC_CHOOSE_SEARCH_EDIT.

Looks good.  Thanks.

cgf

Re: [PATCH] Setup.exe package search/filter (PING cgf)

[Dave Korn]

Dave Korn wrote:

>   So here's an updated and retested 

and corrected!

> version of the patch done under the assumption that reverting that change
> will be acceptable; if not, I can find a different way of doing it.  Ok for
> head?

2009-04-24  Andrew Punch  

* PickView.h:  Add #include .
(PickView::SetPackageFilter):  Add new function.
(PickView::packageFilterString):  Add new string data member.
* PickView.cc (PickView::setViewMode):  Use it to filter names.
(PickView::insert_category):  Likewise.
(PickView::PickView):  Initialise packageFilterString to blank.
* res.rc (IDD_CHOOSE):  Add IDC_CHOOSE_SEARCH_EDIT and
IDC_CHOOSE_SEARCH_LABEL controls.
(IDS_SEARCH_TOOLTIP):  Add new string resource.
* resource.h (IDS_SEARCH_TOOLTIP):  New string resource ID.
(IDC_CHOOSE_SEARCH_EDIT):  New edit control ID.
(IDC_CHOOSE_SEARCH_LABEL):  Mew static text control ID.
* choose.cc (ChooserControlsInfo[]):  Add IDC_CHOOSE_SEARCH_LABEL
and IDC_CHOOSE_SEARCH_EDIT controls to auto-resize list.
(ChooserPage::OnInit):  Add tooltip to search edit box.
(ChooserPage::OnMessageCmd):  Handle EN_CHANGE event from
IDC_CHOOSE_SEARCH_EDIT.

2009-04-24  Dave Korn  

* propsheet.cc (PropSheetWndProc):  Re-enable hasMinRect.

  So, is _this_ one OK?

cheers,
  DaveK

Index: PickView.cc
===
RCS file: /cvs/cygwin-apps/setup/PickView.cc,v
retrieving revision 2.32
diff -p -u -r2.32 PickView.cc
--- PickView.cc	8 Apr 2008 23:50:54 -	2.32
+++ PickView.cc	24 Apr 2009 19:21:57 -
@@ -191,8 +191,12 @@ PickView::setViewMode (views mode)
   // "Not installed"
   || (view_mode == PickView::views::PackageSkips &&
   (!pkg.desired && !pkg.installed)))
-
-insert_pkg (pkg);
+{
+  // Filter by package name
+  if (packageFilterString.empty ()
+  || pkg.name.find (packageFilterString) != std::string::npos)
+insert_pkg (pkg);
+}
 }
 }
 
@@ -309,13 +313,23 @@ PickView::insert_category (Category *cat
   (!showObsolete && isObsolete (cat->first)))
 return;
   PickCategoryLine & catline = *new PickCategoryLine (*this, *cat, 1, collapsed);
+  int packageCount = 0;
   for (vector ::iterator i = cat->second.begin ();
i != cat->second.end () ; ++i)
 {
-  PickLine & line = *new PickPackageLine (*this, **i);
-  catline.insert (line);
+  if (packageFilterString.empty () \
+  || (*i && (*i)->name.find (packageFilterString) != std::string::npos))
+	{
+	  PickLine & line = *new PickPackageLine (*this, **i);
+	  catline.insert (line);
+	  packageCount++;
+	}
 }
-  contents.insert (catline);
+  
+  if (packageFilterString.empty () || packageCount)
+contents.insert (catline);
+  else
+delete &catline;
 }
 
 PickView::views&
@@ -502,7 +516,7 @@ PickView::init_headers (HDC dc)
 
 PickView::PickView (Category &cat) : deftrust (TRUST_UNKNOWN),
 contents (*this, cat, 0, false, true), showObsolete (false), 
-hasClientRect (false)
+packageFilterString (), hasClientRect (false)
 {
 }
 
Index: PickView.h
===
RCS file: /cvs/cygwin-apps/setup/PickView.h,v
retrieving revision 2.17
diff -p -u -r2.17 PickView.h
--- PickView.h	24 May 2006 13:01:34 -	2.17
+++ PickView.h	24 Apr 2009 19:21:57 -
@@ -16,6 +16,7 @@
 #ifndef SETUP_PICKVIEW_H
 #define SETUP_PICKVIEW_H
 
+#include 
 #include "win32.h"
 #include "window.h"
 #include "RECTWrapper.h"
@@ -82,6 +83,13 @@ public:
   int header_height;
   PickCategoryLine contents;
   void scroll (HWND hwnd, int which, int *var, int code, int howmany);
+
+  void SetPackageFilter (const std::string &filterString)
+  {
+packageFilterString = filterString;
+  }
+  
+  
   HWND ListHeader (void) const
   {
 return listheader;
@@ -135,6 +143,7 @@ private:
   HWND listheader;
   views view_mode;
   bool showObsolete;
+  std::string packageFilterString;
 
   // Stuff needed to handle resizing
   bool hasClientRect;
Index: choose.cc
===
RCS file: /cvs/cygwin-apps/setup/choose.cc,v
retrieving revision 2.145
diff -p -u -r2.145 choose.cc
--- choose.cc	5 Aug 2008 15:48:55 -	2.145
+++ choose.cc	24 Apr 2009 19:21:57 -
@@ -67,6 +67,8 @@ extern ThreeBarProgressPage Progress;
   Sizing information.
  */
 static ControlAdjuster::ControlInfo ChooserControlsInfo[] = {
+  {IDC_CHOOSE_SEARCH_LABEL, 	CP_LEFT,CP_TOP},
+  {IDC_CHOOSE_SEARCH_EDIT,	CP_LEFT,CP_TOP},
   {IDC_CHOOSE_KEEP, 		CP_RIGHT,   CP_TOP},
   {IDC_CHOOSE_PREV, 		CP_RIGHT,   CP_TOP},
   {IDC_CHOOSE_CURR, 		CP_RIGHT,   CP_TOP},
@@ -172,6 +174,7 @@ ChooserPage::OnInit ()
   AddTooltip (IDC_CHOOSE_EXP, IDS_TRUSTEXP_TOOLTIP);
   Ad

Re: [PATCH] Setup.exe package search/filter (PING cgf)

[Dave Korn]

Dave Korn wrote:
>   So here's an updated and retested version of the patch done under the
> assumption that reverting that change will be acceptable; if not, I can find a
> different way of doing it.  Ok for head?

  Doh, no, obviously not, I left a bunch of debugging in.  Hang on ten
minutes, I'll resend.

cheers,
  DaveK

Re: Patch: Setup.exe - search for package

[Dave Korn]

Andrew Punch wrote:

> So would you like me to do the tooltip, alignment and label - or would
> you prefer to do it?

  Sorry if that wasn't clear; I already added it in the reformatted version of
your patch attached to my last message!

cheers,
  DaveK

Re: Re: Patch: Setup.exe - search for package

[Andrew Punch]

Dave Korn wrote:

Dave Korn wrote:
  

Andrew Punch wrote:


Hi,

I have attached a patch for searching packages in the package selection
screen. The patch is against version 2.573.2.3 - I couldn't get the CVS
head to build due to a libtool version problem.
  

  Thanks for contributing this.  I'm currently up-porting it to CVS HEAD and
giving it some testing.  None of the other maintainers seem to have any
objections, so I'll commit it once I'm sure it works right.  (Your ChangeLog
entry isn't in the standard format but I'll fix that up for you.)



  Ok, it looks pretty good.  Functionality wise, I have a couple of thoughts:
I think the search box needs a label attached and a tool-tip, and IMO I think
it might look nicer if it was left-aligned away from the view controls; what
do you (and everyone else) think?  Secondly, every time you change the
search-box contents in the category view, it collapses all expanded
categories; I'd like to avoid that if we can, but haven't yet looked to see how.


.. or here where you've just gone and added incorrect indentation without
making any changes at all.  You should always match the existing indentation
style of any code you're working with; if it uses TABs, use TABs, if it uses
spaces, use spaces.  And above all, watch out if your editor is set to
auto-convert between the two!

  


Hi,

Looks like I forget to tidy up a few things before I gave the patch to 
you - oh well developing with an 18 month old running round your 
computer will do that :)


I think a tooltip should be added. The alignment does need to be fixed 
up - either left aligned or flush right against the other controls.


A label sounds good to me. I think the usability benefit of knowing what 
this box does outweighs the slight extra visual noise.


I deliberately decided not to tackle the collapse of categories problem 
because it would probably involve messing with the PickView control. I 
would rather look at that in a separate patch and look at the 
Prev/Curr/Exp collapse problem at the same time.


Thanks for giving some detail on the format you want future patches in. 
I'll also keep an eye on whitespace in the future - particularly where I 
have added some debugging code and then subsequently removed that code.


So would you like me to do the tooltip, alignment and label - or would 
you prefer to do it?


-Andrew

Re: Patch: Setup.exe - search for package

[Corinna Vinschen]

On Apr  1 16:50, Dave Korn wrote:
>   One consequence of doing this is that it draws attention to the fact we need
> to implement minimum window size dimensions for the chooser page; it's bad
> enough that you can shrink it down far enough that controls slide off the
> left-hand edge, but it looks even uglier when the right-aligned controls start
> to slide over the left-aligned ones.  If that sounds like the right thing to
> do, I'll roll a patch.

Sounds right to me.


Corinna

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

Re: Patch: Setup.exe - search for package

[Dave Korn]

Dave Korn wrote:
> Dave Korn wrote:
>> Andrew Punch wrote:
>>> Hi,
>>> 
>>> I have attached a patch for searching packages in the package selection
>>>  screen. The patch is against version 2.573.2.3 - I couldn't get the
>>> CVS head to build due to a libtool version problem.
>> Thanks for contributing this.  I'm currently up-porting it to CVS HEAD
>> and giving it some testing.  

>  I think the search box needs a label attached and a tool-tip, and IMO I
> think it might look nicer if it was left-aligned away from the view
> controls; what do you (and everyone else) think?  

  So, I've up-ported to HEAD, adjusted coding style to match its surroundings,
slightly restructured a couple of bits, and added a label and tool-tip and
left-aligned it.  The result is attached for whoever else would like to test it.

  One consequence of doing this is that it draws attention to the fact we need
to implement minimum window size dimensions for the chooser page; it's bad
enough that you can shrink it down far enough that controls slide off the
left-hand edge, but it looks even uglier when the right-aligned controls start
to slide over the left-aligned ones.  If that sounds like the right thing to
do, I'll roll a patch.

> Secondly, every time you
> change the search-box contents in the category view, it collapses all
> expanded categories; I'd like to avoid that if we can, but haven't yet
> looked to see how.

  I decided not to go any further into this one for now, because it's only the
same behaviour that all the other buttons display when using the category
view.  Maybe we'll fix it later, maybe it will prove less tractable, but it's
another patch for another day.

  Revised changelog follows.  What do you think, everyone: OK for HEAD?

2009-03-27  Andrew Punch  

* PickView.h:  Add #include .
(PickView::SetPackageFilter):  Add new function.
(PickView::packageFilterString):  Add new string data member.
* PickView.cc (PickView::setViewMode):  Use it to filter names.
(PickView::insert_category):  Likewise.
(PickView::PickView):  Initialise packageFilterString to blank.
* res.rc (IDD_CHOOSE):  Add IDC_CHOOSE_SEARCH_EDIT and
IDC_CHOOSE_SEARCH_LABEL controls.
(IDS_SEARCH_TOOLTIP):  Add new string resource.
* resource.h (IDS_SEARCH_TOOLTIP):  New string resource ID.
(IDC_CHOOSE_SEARCH_EDIT):  New edit control ID.
(IDC_CHOOSE_SEARCH_LABEL):  Mew static text control ID.
* choose.cc (ChooserControlsInfo[]):  Add IDC_CHOOSE_SEARCH_LABEL
and IDC_CHOOSE_SEARCH_EDIT controls to auto-resize list.
(ChooserPage::OnInit):  Add tooltip to search edit box.
(ChooserPage::OnMessageCmd):  Handle EN_CHANGE event from
IDC_CHOOSE_SEARCH_EDIT.


cheers,
  DaveK

Index: choose.cc
===
RCS file: /cvs/cygwin-apps/setup/choose.cc,v
retrieving revision 2.145
diff -p -u -r2.145 choose.cc
--- choose.cc   5 Aug 2008 15:48:55 -   2.145
+++ choose.cc   1 Apr 2009 15:28:07 -
@@ -67,6 +67,8 @@ extern ThreeBarProgressPage Progress;
   Sizing information.
  */
 static ControlAdjuster::ControlInfo ChooserControlsInfo[] = {
+  {IDC_CHOOSE_SEARCH_LABEL,CP_LEFT,CP_TOP},
+  {IDC_CHOOSE_SEARCH_EDIT, CP_LEFT,CP_TOP},
   {IDC_CHOOSE_KEEP,CP_RIGHT,   CP_TOP},
   {IDC_CHOOSE_PREV,CP_RIGHT,   CP_TOP},
   {IDC_CHOOSE_CURR,CP_RIGHT,   CP_TOP},
@@ -172,6 +174,7 @@ ChooserPage::OnInit ()
   AddTooltip (IDC_CHOOSE_EXP, IDS_TRUSTEXP_TOOLTIP);
   AddTooltip (IDC_CHOOSE_VIEW, IDS_VIEWBUTTON_TOOLTIP);  
   AddTooltip (IDC_CHOOSE_HIDE, IDS_HIDEOBS_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_SEARCH_EDIT, IDS_SEARCH_TOOLTIP);
 }
 
 void
@@ -255,7 +258,14 @@ ChooserPage::changeTrust(trusts aTrust)
 bool
 ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
 {
-  if (code != BN_CLICKED)
+  if (code == EN_CHANGE && id == IDC_CHOOSE_SEARCH_EDIT)
+{
+  std::string value (egetString (GetHWND (), IDC_CHOOSE_SEARCH_EDIT));
+  chooser->SetPackageFilter (value);
+  chooser->refresh ();
+  return true;
+}
+  else if (code != BN_CLICKED && code != EN_CHANGE)
 {
   // Not a click notification, we don't care.
   return false;
Index: resource.h
===
RCS file: /cvs/cygwin-apps/setup/resource.h,v
retrieving revision 2.36
diff -p -u -r2.36 resource.h
--- resource.h  22 Jun 2008 02:37:16 -  2.36
+++ resource.h  1 Apr 2009 15:28:07 -
@@ -33,6 +33,7 @@
 #define IDS_HIDEOBS_TOOLTIP   130
 #define IDS_SIG_INVALID   131
 #define IDS_CRYPTO_ERROR  132
+#define IDS_SEARCH_TOOLTIP133
 
 // Dialogs
 
@@ -161,3 +162,5 @@
 #define IDC_STATUS_HEADER 582
 #define IDC_STATUS583
 #define IDC_STATIC_HEADER 584
+#define IDC_

Re: [PATCH] setup.exe: add autoload and version check for AttachConsole

[Dave Korn]

Christopher Faylor wrote:

> 
> Please don't submit this.  I'll fix the problem but not this way.


  Okeydokey, cancelled, NP :)

cheers,
  DaveK

Re: [PATCH] setup.exe: add autoload and version check for AttachConsole

[Christopher Faylor]

On Wed, Apr 01, 2009 at 12:54:53PM +0100, Dave Korn wrote:
>
>Hi gang,
>
>  AttachConsole (added recently for stdout/stderr handling) doesn't exist on
>win2k, having been introduced in xp/2k3, so setup HEAD currently doesn't run
>there.
>
>  The attached patch fixes the load-time problem by adding an autoload.  That
>won't prevent the run-time crash if the function gets called, of course; so it
>also adds a version check before the call.
>
>  Unless anyone shouts, I'll commit it later today; I think this is basically
>obvious.  Tested by verifying under GDB that it avoids the SEGV and by seeing
>that with the patch the setup.log and setup.log.full files are successfully
>generated.
>
>   * autoload.c (kernel32):  Add autoload entry for AttachConsole.
>   * main.cc (set_cout):  Check IsXpOrBetter before trying to use it.
>   * win32.h (IsXpOrBetter):  New version check macro.

Please don't submit this.  I'll fix the problem but not this way.

cgf

Re: Patch: Setup.exe - search for package

[Dave Korn]

Dave Korn wrote:
> Andrew Punch wrote:
>> Hi,
>>
>> I have attached a patch for searching packages in the package selection
>> screen. The patch is against version 2.573.2.3 - I couldn't get the CVS
>> head to build due to a libtool version problem.
> 
>   Thanks for contributing this.  I'm currently up-porting it to CVS HEAD and
> giving it some testing.  None of the other maintainers seem to have any
> objections, so I'll commit it once I'm sure it works right.  (Your ChangeLog
> entry isn't in the standard format but I'll fix that up for you.)

  Ok, it looks pretty good.  Functionality wise, I have a couple of thoughts:
I think the search box needs a label attached and a tool-tip, and IMO I think
it might look nicer if it was left-aligned away from the view controls; what
do you (and everyone else) think?  Secondly, every time you change the
search-box contents in the category view, it collapses all expanded
categories; I'd like to avoid that if we can, but haven't yet looked to see how.

  Also, a few stylistic issues to bear in mind for any potential future
occasion you're submitting a patch (don't worry about them now, I'll tidy it
up as part of porting it to HEAD):

- Standard system header includes should go at the top of lists of #includes,
not the end:

--- setup-2.573.2.3/PickView.h  2006-05-24 23:01:34.0 +1000
+++ setup-new/PickView.h2009-03-31 21:14:00.37500 +1100
@@ -19,6 +19,7 @@
 #include "win32.h"
 #include "window.h"
 #include "RECTWrapper.h"
+#include 

 #define HMARGIN 10
 #define ROW_MARGIN  5
@@ -82,6 +83,13 @@

- There should be a space between any function name and the following
open-bracket:

@@ -82,6 +83,13 @@
   int header_height;
   PickCategoryLine contents;
   void scroll (HWND hwnd, int which, int *var, int code, int howmany);
+
+  void SetPackageFilter(const std::string &filterString)
+  {
+   packageFilterString = filterString;
+  }
+
+

- Watch out for accidental white-space and other superfluous or unintentional
changes; e.g.

diff -u --exclude '*config.*' --exclude '*Makefile' setup-2.573.2.3/res.rc
setup-new/res.rc
--- setup-2.573.2.3/res.rc  2008-06-19 09:26:20.0 +1000
+++ setup-new/res.rc2009-03-31 21:14:00.390625000 +1100
@@ -316,7 +316,8 @@
 CAPTION "Cygwin Setup - Select Packages"
 FONT 8, "MS Shell Dlg"
 BEGIN
-CONTROL "&Keep",IDC_CHOOSE_KEEP,"Button",BS_AUTORADIOBUTTON |
+EDITTEXT   IDC_CHOOSE_SEARCH_EDIT, 7, 30, 60, 12
+   CONTROL "&Keep",IDC_CHOOSE_KEEP,"Button",BS_AUTORADIOBUTTON |

here you replaced the spaces before CONTROL with a TAB, or here:

@@ -291,8 +300,9 @@
 case IDC_CHOOSE_HIDE:
   chooser->setObsolete (!IsButtonChecked (id));
   break;
+   
+
 default:
-  // Wasn't recognized or handled.
   return false;
 }

where you added some blank lines and deleted a comment; neither of these two
changes are related to or required for the purpose of your patch in any way.

@@ -283,20 +288,20 @@
 }
   else
 {
-  for (set ::const_iterator x
+ for (set ::const_iterator x
   = pkg.categories.begin (); x != pkg.categories.end (); ++x)
-{
+   {
  // Special case - yuck
  if (casecompare(*x, "All") == 0)
-   continue;
+   continue;

  packagedb db;
  PickCategoryLine & catline =
-   *new PickCategoryLine (*this, *db.categories.find (*x), 1);
+   *new PickCategoryLine (*this, *db.categories.find (*x), 1);
  PickLine & line = *new PickPackageLine(*this, pkg);
  catline.insert (line);
  contents.insert (catline);
-}
+   }
 }
 }

.. or here where you've just gone and added incorrect indentation without
making any changes at all.  You should always match the existing indentation
style of any code you're working with; if it uses TABs, use TABs, if it uses
spaces, use spaces.  And above all, watch out if your editor is set to
auto-convert between the two!

  The other thing I wanted to mention is that your ChangeLog entry isn't in
the standard format.  Minor: two spaces between the date and your name, and
your name and your email address.  More significant: we don't put introductory
paragraphs in ChangeLogs, it's not what they're for.  Minor again: each line
(apart from the header with date/name/email) should begin with a TAB; you
should only specify the filename once for each file modified, on the first
line; we use the present tense for verbs; and there are some standard ways of
denoting things like the addition of new functions.  A sentence like "Add
definition of function SetPackageFilter to class PickView" contains a lot of
redundancy when you've already given the class and function names in brackets
immediately beforehand; anyone who understands C++ will already know that.

  So I'd rewrite it like so (email protected just for illustration):

2009-03-27  Andrew Punch  

* PickV

Re: Patch: Setup.exe - search for package

[Dave Korn]

Andrew Punch wrote:
> Hi,
> 
> I have attached a patch for searching packages in the package selection
> screen. The patch is against version 2.573.2.3 - I couldn't get the CVS
> head to build due to a libtool version problem.

  Thanks for contributing this.  I'm currently up-porting it to CVS HEAD and
giving it some testing.  None of the other maintainers seem to have any
objections, so I'll commit it once I'm sure it works right.  (Your ChangeLog
entry isn't in the standard format but I'll fix that up for you.)

cheers,
  DaveK

Re: [PATCH] - setup.exe --text-mode and --current-user-only options

[Reini Urban]

Servaas Goossens schrieb:

Sorry, I wasn't aware of these changes for 1.7. Does this mean that the
1.5 version of setup is not modified anymore? If so, I'll just keep
using my own patched setup.exe.


Internally tracked at http://sourceware.org/bugzilla/show_bug.cgi?id=6689

Please keep discussion on cygwin-apps@

I would say the setup-1.5 will still stay for a while, at least until 
autumn. So why not.


BTW. The patch does not apply cleanly to the latest setup snapshot 2.588
I've added a fixed version to bugzilla



-Original Message-
From: Brian Dessent [mailto:[EMAIL PROTECTED] 
Sent: woensdag 28 mei 2008 12:06

To: Servaas Goossens
Cc: cygwin-apps@cygwin.com
Subject: Re: [PATCH] - setup.exe --text-mode and 
--current-user-only options


Servaas Goossens wrote:


Please consider the following patch to setup. (I tried to follow the
patch submission guidelines, please correct me if I missed 

something).

This definitely conflicts with the direction that setup is moving in. 
In the 1.7 patches that I've been (slowly!) working on, the text/bin
choice is completely removed from setup because it's no 
longer stored in

the registry.  To get textmode with 1.7 you edit fstab.

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

RE: [PATCH] - setup.exe --text-mode and --current-user-only options

[Servaas Goossens]

Sorry, I wasn't aware of these changes for 1.7. Does this mean that the
1.5 version of setup is not modified anymore? If so, I'll just keep
using my own patched setup.exe.

Servaas.

> -Original Message-
> From: Brian Dessent [mailto:[EMAIL PROTECTED] 
> Sent: woensdag 28 mei 2008 12:06
> To: Servaas Goossens
> Cc: cygwin-apps@cygwin.com
> Subject: Re: [PATCH] - setup.exe --text-mode and 
> --current-user-only options
> 
> Servaas Goossens wrote:
> 
> > Please consider the following patch to setup. (I tried to follow the
> > patch submission guidelines, please correct me if I missed 
> something).
> 
> This definitely conflicts with the direction that setup is moving in. 
> In the 1.7 patches that I've been (slowly!) working on, the text/bin
> choice is completely removed from setup because it's no 
> longer stored in
> the registry.  To get textmode with 1.7 you edit fstab.
> 
> Brian
> 

Re: [PATCH] - setup.exe --text-mode and --current-user-only options

[Brian Dessent]

Servaas Goossens wrote:

> Please consider the following patch to setup. (I tried to follow the
> patch submission guidelines, please correct me if I missed something).

This definitely conflicts with the direction that setup is moving in. 
In the 1.7 patches that I've been (slowly!) working on, the text/bin
choice is completely removed from setup because it's no longer stored in
the registry.  To get textmode with 1.7 you edit fstab.

Brian

RE: [PATCH] setup.exe: specify packages on command line

[Thrall, Bryan]

Thrall, Bryan wrote on Thursday, August 09, 2007 12:46 PM:
> In reply to http://cygwin.com/ml/cygwin-apps/2006-03/msg00070.html,
> Igor suggested that cleaning up Frank's patch would get it merged
> into setup.exe. Please see attached patch for my cleaned up version
> (hopefully this is the clean up he was asking for).   
> 
> For the ChangeLog:
> 
> 2007-08-09  Bryan Thrall  <[EMAIL PROTECTED]>
> 
>   * package_db.cc (PackageOption): Option for specifying packages
to
>   add to the Base category from command line.
>   (manuallyAddedToBase): Checks that the given package needs to be
>   added to Base. (fillMissingCategory): Add packages from command
line
> to Base. 
>   * package_meta.h (addToCategoryBase): Declaration.
>   * package_meta.cc (addToCategoryBase): Add Base category to
package.

Actually, maybe Frank should be listed as the contributor? It was
originally his patch, after all.

-- 
Bryan Thrall
FlightSafety International
[EMAIL PROTECTED]

RE: [PATCH] - setup.exe --mirror-list option (nudge, nudge)

[Hicks, Jerry - ACD]

 

-Original Message-
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
On Behalf Of Christopher Faylor
Sent: Friday, July 27, 2007 6:29 PM
To: cygwin-apps@cygwin.com
Subject: Re: [PATCH] - setup.exe --mirror-list option (nudge, nudge)

On Fri, Jul 27, 2007 at 11:49:39AM -0400, Hicks, Jerry - ACD wrote:
> No comments?

> I have two comments but neither is probably anything you want to hear.

> 1) I don't care if the developers want to consider this patch
> but since setup.exe is not supposed to be a generic install
> utility but I could see if they they would rather forgo installing
> it as it doesn't really provide any functionality which would be
> useful for the normal user.  New functionality always carries a
> support burden so these kinds of features have to be considered
> with this in mind.

You're right, I didn't want to hear that.  :-)

I suppose it's ok, I can just patch up the .rc file and build a
local customized setup.exe here in perpetuity.

> 2) It is against site policy for email to be sent with the type
> of disclaimer that you are using.  I mention this because very
> soon we'll actually be blocking things that look like the disclaimer.

Yeah, they do that behind my back to all outbound mail so I'm having
that "inflicted" upon me rather than me "using" it.

Thanks for the flame though.

Unsubscribed,

Jerry.
*
This e-mail and any files transmitted with it may be proprietary 
and are intended solely for the use of the individual or entity to 
whom they are addressed. If you have received this e-mail in 
error please notify the sender. Please note that any views or
opinions presented in this e-mail are solely those of the author 
and do not necessarily represent those of ITT Corporation. The 
recipient should check this e-mail and any attachments for the 
presence of viruses. ITT accepts no liability for any damage 
caused by any virus transmitted by this e-mail.
***

Re: [PATCH] - setup.exe --mirror-list option (nudge, nudge)

[Christopher Faylor]

On Fri, Jul 27, 2007 at 11:49:39AM -0400, Hicks, Jerry - ACD wrote:
> No comments?

I have two comments but neither is probably anything you want to hear.

1) I don't care if the developers want to consider this patch but since
setup.exe is not supposed to be a generic install utility but I could
see if they they would rather forgo installing it as it doesn't really
provide any functionality which would be useful for the normal user.
New functionality always carries a support burden so these kinds of
features have to be considered with this in mind.

2) It is against site policy for email to be sent with the type of
disclaimer that you are using.  I mention this because very soon we'll
actually be blocking things that look like the disclaimer.

cgf

Re: [PATCH] Setup.exe: Mouse wheel

[Robert Collins]

On Sat, 2003-05-31 at 02:05, Max Bowsher wrote:
> Benjamin Riefenstahl wrote:
> > Hi all,
> >
> > Benjamin Riefenstahl <[EMAIL PROTECTED]> writes:
> >> Attached a simple patch to choose.cc to do this.
> >
> > Is nobody interested?
> 
> Setup.exe only has one maintainer, and not even very many people who
> regularly send patches.
> 
> It is most likely that your patch has been overlooked through lack of time.
> 
> Also, note that not everyone has mouse wheels - my laptop has a scroll
> button, so I could neither test, nor effectively comment on your patch.

Lack of time, and lack of feedback - there's been no commentary. Finally
it's not obviously the Right Thing to do. It seems to me we'd want to
trap those events and translate them in the rest of the choose event
loop, not as a special case.

I'll try and dig it up and give a more clear review this weekend.

Rob
-- 
GPG key available at: .


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] Setup.exe: Mouse wheel

[Max Bowsher]

Benjamin Riefenstahl wrote:
> Hi all,
>
> Benjamin Riefenstahl <[EMAIL PROTECTED]> writes:
>> Attached a simple patch to choose.cc to do this.
>
> Is nobody interested?

Setup.exe only has one maintainer, and not even very many people who
regularly send patches.

It is most likely that your patch has been overlooked through lack of time.

Also, note that not everyone has mouse wheels - my laptop has a scroll
button, so I could neither test, nor effectively comment on your patch.


Max.

Re: [PATCH] Setup.exe: Mouse wheel

[Benjamin Riefenstahl]

Hi all,

Benjamin Riefenstahl <[EMAIL PROTECTED]> writes:
> Attached a simple patch to choose.cc to do this.

Is nobody interested?

so long, benny

Re: [PATCH] setup.exe - problem with FTP

[Marcel Telka]

ping

Napísané dňa 2002.07.10 09:10, (autor: Robert Collins):

This will go in but not immediately, I've a couple of pending things
in my
sandbox to sort out first.

Rob
- Original Message -
From: "Marcel Telka" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Wednesday, July 10, 2002 5:01 PM
Subject: Re: [PATCH] setup.exe - problem with FTP


>
> Napísané dňa 08.07.2002 15:27:30 +0200, (autor: Marcel Telka):
> > Hi.
> >
> > I've found little problem in the cygwin setup program:
> >
> > There is no possibility to download files from some FTP servers
(like
> > djb's publicfile ftpd http://cr.yp.to/publicfile/ftpd.html)
because
> > the setup scans for '(' in a PASV FTP reply (code 227).
> >
> > djb's ftpd uses this format for a PASV reply:
> >
> >   227 =h1,h2,h3,h4,p1,p2
> >
> >
> > RFC 1123 (in section 4.1.2.6) says:
> >
> > 
> > The format of the 227 reply to a PASV command is not well
standardized.
> > In particular, an FTP client cannot assume that the parentheses
shown
> > on page 40 of RFC-959 will be present (and in fact, Figure 3 on
page 43
> > omits them). Therefore, a User-FTP program that interprets the
PASV
reply
> > must scan the reply for the first digit of the host and port
numbers.
> > 
> >
> > I've considered this as a bug in the setup and created patch for
> > nio-ftp.cc
> > file (see attachment). Please apply this patch to the CVS tree.
Thanks.
>
> Please.
>
> --
> +---+
> | Marcel Telka   e-mail:   [EMAIL PROTECTED]  |
> |homepage: http://telka.sk/ |
> |jabber:   [EMAIL PROTECTED] |
> +---+
>



--
+---+
| Marcel Telka   e-mail:   [EMAIL PROTECTED]  |
|homepage: http://telka.sk/ |
|jabber:   [EMAIL PROTECTED] |
+---+

Re: [PATCH] setup.exe - problem with FTP

[Robert Collins]

This will go in but not immediately, I've a couple of pending things in my
sandbox to sort out first.

Rob
- Original Message -
From: "Marcel Telka" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Wednesday, July 10, 2002 5:01 PM
Subject: Re: [PATCH] setup.exe - problem with FTP


>
> Napísané dňa 08.07.2002 15:27:30 +0200, (autor: Marcel Telka):
> > Hi.
> >
> > I've found little problem in the cygwin setup program:
> >
> > There is no possibility to download files from some FTP servers (like
> > djb's publicfile ftpd http://cr.yp.to/publicfile/ftpd.html) because
> > the setup scans for '(' in a PASV FTP reply (code 227).
> >
> > djb's ftpd uses this format for a PASV reply:
> >
> >   227 =h1,h2,h3,h4,p1,p2
> >
> >
> > RFC 1123 (in section 4.1.2.6) says:
> >
> > 
> > The format of the 227 reply to a PASV command is not well standardized.
> > In particular, an FTP client cannot assume that the parentheses shown
> > on page 40 of RFC-959 will be present (and in fact, Figure 3 on page 43
> > omits them). Therefore, a User-FTP program that interprets the PASV
reply
> > must scan the reply for the first digit of the host and port numbers.
> > 
> >
> > I've considered this as a bug in the setup and created patch for
> > nio-ftp.cc
> > file (see attachment). Please apply this patch to the CVS tree. Thanks.
>
> Please.
>
> --
> +---+
> | Marcel Telka   e-mail:   [EMAIL PROTECTED]  |
> |homepage: http://telka.sk/ |
> |jabber:   [EMAIL PROTECTED] |
> +---+
>

Re: [PATCH] setup.exe - problem with FTP

[Marcel Telka]

Napísané dňa 08.07.2002 15:27:30 +0200, (autor: Marcel Telka):
> Hi.
> 
> I've found little problem in the cygwin setup program:
> 
> There is no possibility to download files from some FTP servers (like
> djb's publicfile ftpd http://cr.yp.to/publicfile/ftpd.html) because
> the setup scans for '(' in a PASV FTP reply (code 227).
> 
> djb's ftpd uses this format for a PASV reply:
> 
>   227 =h1,h2,h3,h4,p1,p2
> 
> 
> RFC 1123 (in section 4.1.2.6) says:
> 
> 
> The format of the 227 reply to a PASV command is not well standardized.
> In particular, an FTP client cannot assume that the parentheses shown
> on page 40 of RFC-959 will be present (and in fact, Figure 3 on page 43
> omits them). Therefore, a User-FTP program that interprets the PASV reply
> must scan the reply for the first digit of the host and port numbers.
> 
> 
> I've considered this as a bug in the setup and created patch for
> nio-ftp.cc
> file (see attachment). Please apply this patch to the CVS tree. Thanks.

Please.

-- 
+---+
| Marcel Telka   e-mail:   [EMAIL PROTECTED]  |
|homepage: http://telka.sk/ |
|jabber:   [EMAIL PROTECTED] |
+---+

Re: [PATCH] setup.exe - problem with FTP

[Marcel Telka]

On Mon, Jul 08, 2002 at 03:27:30PM +0200, Marcel Telka wrote:
> ChangeLog entry:
> 
> 
> 2002-07-08  Marcel Telka  <[EMAIL PROTECTED]>
> 
>   * nio_ftp.cc (NetIO_FTP::NetIO_FTP): Fixed decoding of the FTP reply (code 227)
 ^
Aaargh. Here | should be '-' instead of '_'. Sorry.

>   to the PASV command.

-- 
+---+
| Marcel Telka   e-mail:   [EMAIL PROTECTED]  |
|homepage: http://telka.sk/ |
|jabber:   [EMAIL PROTECTED] |
+---+

RE: [PATCH] Setup.exe 2.218.2.9 does not like its own source

[Robert Collins]

> -Original Message-
> From: Ton van Overbeek [mailto:[EMAIL PROTECTED]] 
> Sent: Monday, May 20, 2002 3:26 AM

> The attached patch makes setup download the source of setup 
> without complaining.
> 
> I am not totally at home with all the logic in 
> package_meta.cc and its interaction with the chooser to know 
> if this change causes other problems. So I will not be 
> surprised if this is not the 'right' solution.

It's not, it alters the default install logic, so you'll be getting the
wrong version by default when you select install at a category level or
click on a pacakage. ('If (installed)' means that packages won't install
on the first click, which is wrong.)

The correct test is:
(desired->bin.sites.number() || desired->bin.Cached())

i.e. if there are download sites that the desired package can come from
or it's known to be cached then we should turn bin on. This also catches
another bug, where reinstall was a valid choice for a installed package
on a local install, regardless of package availability. Thank you for
tracking down the specific location though - that saved me quite some
time.

The fix I'll be committing is below. It makes a source only package
install the source by default in addition to the bug fix above. I also
noticed another minor bug when iterating through lots of packges
locally.

Rob

Index: package_meta.cc
===
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.22
diff -u -p -r2.22 package_meta.cc
--- package_meta.cc 4 May 2002 04:26:01 -   2.22
+++ package_meta.cc 20 May 2002 01:04:40 -
@@ -318,8 +318,14 @@ packagemeta::set_action (packageversion 
}
   return;
 }
-  else if (desired == installed
-  && (!installed || !(installed->binpicked ||
installed->srcpicked)))
+  else if (desired == installed &&
+  (!installed || 
+   // neither bin nor source are being installed
+   (!(installed->binpicked || installed->srcpicked) &&
+// bin or source are available
+((installed->bin.sites.number() || desired->bin.Cached())
||
+ (installed->src.sites.number() ||
desired->src.Cached()
+  )
 /* Install the default trust version - this is a 'reinstall' for
installed
* packages */
 {
@@ -328,7 +334,10 @@ packagemeta::set_action (packageversion 
   desired = default_version;
   if (desired)
{
- desired->binpicked = 1;
+ if (desired->bin.sites.number() || desired->bin.Cached())
+   desired->binpicked = 1;
+ else
+   desired->srcpicked = 1;
  return;
}
 }
@@ -375,7 +384,10 @@ packagemeta::set_action (packageversion 
  if (n <= versions.number ())
{
  desired = versions[n];
- desired->srcpicked = source;
+ if (desired->src.sites.number() || desired->src.Cached())
+   desired->srcpicked = source;
+ else
+   desired->srcpicked = 0;
  return;
}
}