Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Fri, 17 Mar 2006, Max Bowsher wrote:

> Igor Peshansky wrote:
> > On Thu, 16 Mar 2006, Max Bowsher wrote:
> >
> > Speaking of in-use DLLs, did you have a chance to look at
> > ?
>
> OK, I'll have a look.

Thanks.

> >> So, I think we should always rename preremove scripts, because we
> >> certainly don't want a failed preremove script to be removed by the
> >> later file-removal phase - it might be wanted for debugging.
> >
> > I would even go one step further, and cancel the uninstall of the current
> > package and all packages it depends on if the preremove failed.  But
> > that should be a separate patch.

Would this work, BTW?

> >> As for postinstall scripts ... I think ideally it would be a separate
> >> operation (c.f. 'dpkg --configure --pending'), but I guess we can go
> >> with the simple solution for now, and defer a more complex solution
> >> until someone has the inclination.
> >
> > So, does this mean "please check in"?
>
> I'd like preremove scripts to be renamed on failure. But, OK, you check
> in as-is, and I'll follow-up with a change for that.

Hmm, it's easy enough to rename scripts on failure -- it is simply a
question of propagating the information of whether this is a preremove
script or not to the script object itself.  Since all preremove scripts
are run from try_run_script(), and none of the postinstall scripts are, it
should be reasonably easy to add a rename_on_failure field (and
constructor argument that defaults to false), and set it to true in
the Script() constructor in try_run_script().  Once I commit this one in,
I can look into adding the rename_on_failure code...

While we're at it, I'd also like to clean up the constant mentions of
strings "/etc/postinstall" (in at least 3 places) and "/etc/preremove" (in
two).  Those should be defined in script.h as macros, most likely...

Another patch coming up...
Igor
-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Max Bowsher]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Igor Peshansky wrote:
> On Thu, 16 Mar 2006, Max Bowsher wrote:
> 
> Speaking of in-use DLLs, did you have a chance to look at
> ?

OK, I'll have a look.

>> So, I think we should always rename preremove scripts, because we
>> certainly don't want a failed preremove script to be removed by the
>> later file-removal phase - it might be wanted for debugging.
> 
> I would even go one step further, and cancel the uninstall of the current
> package and all packages it depends on if the preremove failed.  But
> that should be a separate patch.
> 
>> As for postinstall scripts ... I think ideally it would be a separate
>> operation (c.f. 'dpkg --configure --pending'), but I guess we can go
>> with the simple solution for now, and defer a more complex solution
>> until someone has the inclination.
> 
> So, does this mean "please check in"?

I'd like preremove scripts to be renamed on failure. But, OK, you check
in as-is, and I'll follow-up with a change for that.

Max.

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.2.1 (Cygwin)

iD8DBQFEGpFbfFNSmcDyxYARAkJVAJ4kDwP9yUuwcDLwwYNGiLzoXKt1EACgzJIk
BjR25ByI8bMaFEEeL9yOECw=
=fdn7
-END PGP SIGNATURE-

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Thu, 16 Mar 2006, Max Bowsher wrote:

> Igor Peshansky wrote:
> > On Tue, 14 Mar 2006, Max Bowsher wrote:
> >
> >> Igor Peshansky wrote:
> >>> On Thu, 9 Mar 2006, Dave Korn wrote:
> >>>
>  On 09 March 2006 23:14, Max Bowsher wrote:
> 
> >>>   * script.cc (Script::run): Fix inverted test for abnormal exit.
> >>>   Do not rename to ".done" unless completed successfully.
> >> And ping (attached as "setup-script-exit-code-fix.patch").
> > Do we necessarily want to try to re-run failed scripts the next
> > time setup installs some packages?
> >>> Why not?  It would make recovery from a hosed install because of
> >>> in-use DLLs easy enough -- just re-run setup and select "Keep",
> >>> which will only rerun the postinstall scripts.

Speaking of in-use DLLs, did you have a chance to look at
?

> > Perhaps renaming to ".failed" would be better that not renaming.
> >
> > Max.
>    Perhaps setup should check when you first start it up whether
>  there are any postinstall scripts left lying around from last time
>  and offer to run them for you then and there?  Failed postinstalls
>  should be run to completion *before* next updating the package!
> >>>
> >>> Why?  I'm not so certain.  So your preremove will fail -- who cares,
> >>> it would also fail if "cygwin" is upgraded and is uninstalled before
> >>> the preremove script is run.  Next time you upgrade, it'll be like
> >>> the initial install all over again, and the *new* postinstall will
> >>> run.  If you didn't touch the package, however, the postinstall that
> >>> failed before will be re-run.  If it failed because of something in
> >>> the environment when setup was run (e.g., a stale DLL in memory), it
> >>> will now succeed and will be renamed to .done.  If it fails again,
> >>> we'll know something was wrong, and will recommend looking at the
> >>> output in setup.log.full.
> >>
> >> I'm concerned about introducing weird subtle edge cases. For example:
> >>
> >> Upgrade package in which old version has preremove, new version does
> >> not. preremove fails. Next time setup runs, stale preremove zaps parts
> >> of the upgraded package.
> >>
> >> Granted, it's a fairly tenuous situation, but at least the current
> >> behaviour is very predictable: scripts are only re-run manually.
> >
> > Umm, we can never have stale postinstalls and preremoves, since those
> > are part of the manifest (pkg.lst) and will be removed by the
> > "uninstall" part of the upgrade.  The only way we can have a stale
> > postinstall is if it no longer has the name in the manifest (i.e., is
> > renamed to ".done").  Am I missing something?
>
> Ah, I hadn't thought of that.
>
> So, I think we should always rename preremove scripts, because we
> certainly don't want a failed preremove script to be removed by the
> later file-removal phase - it might be wanted for debugging.

I would even go one step further, and cancel the uninstall of the current
package and all packages it depends on if the preremove failed.  But
that should be a separate patch.

> As for postinstall scripts ... I think ideally it would be a separate
> operation (c.f. 'dpkg --configure --pending'), but I guess we can go
> with the simple solution for now, and defer a more complex solution
> until someone has the inclination.

So, does this mean "please check in"?
Igor
-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Max Bowsher]

Igor Peshansky wrote:
> On Tue, 14 Mar 2006, Max Bowsher wrote:
> 
>> Igor Peshansky wrote:
>>> On Thu, 9 Mar 2006, Dave Korn wrote:
>>>
 On 09 March 2006 23:14, Max Bowsher wrote:

>>> * script.cc (Script::run): Fix inverted test for abnormal exit.
>>> Do not rename to ".done" unless completed successfully.
>> And ping (attached as "setup-script-exit-code-fix.patch").
> Do we necessarily want to try to re-run failed scripts the next time
> setup installs some packages?
>>> Why not?  It would make recovery from a hosed install because of in-use
>>> DLLs easy enough -- just re-run setup and select "Keep", which will only
>>> rerun the postinstall scripts.
>>>
> Perhaps renaming to ".failed" would be better that not renaming.
>
> Max.
   Perhaps setup should check when you first start it up whether there
 are any postinstall scripts left lying around from last time and offer
 to run them for you then and there?  Failed postinstalls should be run
 to completion *before* next updating the package!
>>> Why?  I'm not so certain.  So your preremove will fail -- who cares, it
>>> would also fail if "cygwin" is upgraded and is uninstalled before the
>>> preremove script is run.  Next time you upgrade, it'll be like the initial
>>> install all over again, and the *new* postinstall will run.  If you didn't
>>> touch the package, however, the postinstall that failed before will be
>>> re-run.  If it failed because of something in the environment when setup
>>> was run (e.g., a stale DLL in memory), it will now succeed and will be
>>> renamed to .done.  If it fails again, we'll know something was wrong, and
>>> will recommend looking at the output in setup.log.full.
>> I'm concerned about introducing weird subtle edge cases. For example:
>>
>> Upgrade package in which old version has preremove, new version does
>> not. preremove fails. Next time setup runs, stale preremove zaps parts
>> of the upgraded package.
>>
>> Granted, it's a fairly tenuous situation, but at least the current
>> behaviour is very predictable: scripts are only re-run manually.
> 
> Umm, we can never have stale postinstalls and preremoves, since those are
> part of the manifest (pkg.lst) and will be removed by the "uninstall" part
> of the upgrade.  The only way we can have a stale postinstall is if it no
> longer has the name in the manifest (i.e., is renamed to ".done").  Am I
> missing something?

Ah, I hadn't thought of that.

So, I think we should always rename preremove scripts, because we
certainly don't want a failed preremove script to be removed by the
later file-removal phase - it might be wanted for debugging.

As for postinstall scripts ... I think ideally it would be a separate
operation (c.f. 'dpkg --configure --pending'), but I guess we can go
with the simple solution for now, and defer a more complex solution
until someone has the inclination.

Max.

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Tue, 14 Mar 2006, Max Bowsher wrote:

> Igor Peshansky wrote:
> > On Thu, 9 Mar 2006, Dave Korn wrote:
> >
> >> On 09 March 2006 23:14, Max Bowsher wrote:
> >>
> > * script.cc (Script::run): Fix inverted test for abnormal exit.
> > Do not rename to ".done" unless completed successfully.
>  And ping (attached as "setup-script-exit-code-fix.patch").
> >>> Do we necessarily want to try to re-run failed scripts the next time
> >>> setup installs some packages?
> >
> > Why not?  It would make recovery from a hosed install because of in-use
> > DLLs easy enough -- just re-run setup and select "Keep", which will only
> > rerun the postinstall scripts.
> >
> >>> Perhaps renaming to ".failed" would be better that not renaming.
> >>>
> >>> Max.
> >>   Perhaps setup should check when you first start it up whether there
> >> are any postinstall scripts left lying around from last time and offer
> >> to run them for you then and there?  Failed postinstalls should be run
> >> to completion *before* next updating the package!
> >
> > Why?  I'm not so certain.  So your preremove will fail -- who cares, it
> > would also fail if "cygwin" is upgraded and is uninstalled before the
> > preremove script is run.  Next time you upgrade, it'll be like the initial
> > install all over again, and the *new* postinstall will run.  If you didn't
> > touch the package, however, the postinstall that failed before will be
> > re-run.  If it failed because of something in the environment when setup
> > was run (e.g., a stale DLL in memory), it will now succeed and will be
> > renamed to .done.  If it fails again, we'll know something was wrong, and
> > will recommend looking at the output in setup.log.full.
>
> I'm concerned about introducing weird subtle edge cases. For example:
>
> Upgrade package in which old version has preremove, new version does
> not. preremove fails. Next time setup runs, stale preremove zaps parts
> of the upgraded package.
>
> Granted, it's a fairly tenuous situation, but at least the current
> behaviour is very predictable: scripts are only re-run manually.

Umm, we can never have stale postinstalls and preremoves, since those are
part of the manifest (pkg.lst) and will be removed by the "uninstall" part
of the upgrade.  The only way we can have a stale postinstall is if it no
longer has the name in the manifest (i.e., is renamed to ".done").  Am I
missing something?
Igor
-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Max Bowsher]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Igor Peshansky wrote:
> On Thu, 9 Mar 2006, Dave Korn wrote:
> 
>> On 09 March 2006 23:14, Max Bowsher wrote:
>>
>   * script.cc (Script::run): Fix inverted test for abnormal exit.
>   Do not rename to ".done" unless completed successfully.
 And ping (attached as "setup-script-exit-code-fix.patch").
>>> Do we necessarily want to try to re-run failed scripts the next time
>>> setup installs some packages?
> 
> Why not?  It would make recovery from a hosed install because of in-use
> DLLs easy enough -- just re-run setup and select "Keep", which will only
> rerun the postinstall scripts.
> 
>>> Perhaps renaming to ".failed" would be better that not renaming.
>>>
>>> Max.
>>   Perhaps setup should check when you first start it up whether there
>> are any postinstall scripts left lying around from last time and offer
>> to run them for you then and there?  Failed postinstalls should be run
>> to completion *before* next updating the package!
> 
> Why?  I'm not so certain.  So your preremove will fail -- who cares, it
> would also fail if "cygwin" is upgraded and is uninstalled before the
> preremove script is run.  Next time you upgrade, it'll be like the initial
> install all over again, and the *new* postinstall will run.  If you didn't
> touch the package, however, the postinstall that failed before will be
> re-run.  If it failed because of something in the environment when setup
> was run (e.g., a stale DLL in memory), it will now succeed and will be
> renamed to .done.  If it fails again, we'll know something was wrong, and
> will recommend looking at the output in setup.log.full.


I'm concerned about introducing weird subtle edge cases. For example:

Upgrade package in which old version has preremove, new version does
not. preremove fails. Next time setup runs, stale preremove zaps parts
of the upgraded package.

Granted, it's a fairly tenuous situation, but at least the current
behaviour is very predictable: scripts are only re-run manually.


Max.

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.1 (Cygwin)

iD8DBQFEFuQVfFNSmcDyxYARAlgKAKC66G+IST6shyrayRdINqP9+MHyRwCeM1bW
9/T/dk+FCynLASmQ0p5HsEM=
=0T78
-END PGP SIGNATURE-

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Thu, 9 Mar 2006, Max Bowsher wrote:

> Igor Peshansky wrote:
> >> 2006-01-23  Igor Peshansky  <[EMAIL PROTECTED]>
> >>
> >>* script.cc (Script::run): Fix inverted test for abnormal exit.
> >
> > Ping (attached as "setup-script-exit-code-fix.patch-1").
>
> Clearly correct, please commit.

Thanks, done.

> >> 2006-01-31  Igor Peshansky  <[EMAIL PROTECTED]>
> >>
> >>* script.cc (Script::run): Fix inverted test for abnormal exit.
> >>Do not rename to ".done" unless completed successfully.
> >
> > And ping (attached as "setup-script-exit-code-fix.patch").
>
> Do we necessarily want to try to re-run failed scripts the next time
> setup installs some packages?
>
> Perhaps renaming to ".failed" would be better that not renaming.

See the discussion in .
Igor
-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

RE: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Thu, 9 Mar 2006, Dave Korn wrote:

> On 09 March 2006 23:14, Max Bowsher wrote:
>
> >>>   * script.cc (Script::run): Fix inverted test for abnormal exit.
> >>>   Do not rename to ".done" unless completed successfully.
> >>
> >> And ping (attached as "setup-script-exit-code-fix.patch").
> >
> > Do we necessarily want to try to re-run failed scripts the next time
> > setup installs some packages?

Why not?  It would make recovery from a hosed install because of in-use
DLLs easy enough -- just re-run setup and select "Keep", which will only
rerun the postinstall scripts.

> > Perhaps renaming to ".failed" would be better that not renaming.
> >
> > Max.
>
>   Perhaps setup should check when you first start it up whether there
> are any postinstall scripts left lying around from last time and offer
> to run them for you then and there?  Failed postinstalls should be run
> to completion *before* next updating the package!

Why?  I'm not so certain.  So your preremove will fail -- who cares, it
would also fail if "cygwin" is upgraded and is uninstalled before the
preremove script is run.  Next time you upgrade, it'll be like the initial
install all over again, and the *new* postinstall will run.  If you didn't
touch the package, however, the postinstall that failed before will be
re-run.  If it failed because of something in the environment when setup
was run (e.g., a stale DLL in memory), it will now succeed and will be
renamed to .done.  If it fails again, we'll know something was wrong, and
will recommend looking at the output in setup.log.full.
Igor
-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

RE: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Dave Korn]

On 09 March 2006 23:14, Max Bowsher wrote:


>>> * script.cc (Script::run): Fix inverted test for abnormal exit.
>>> Do not rename to ".done" unless completed successfully.
>> 
>> And ping (attached as "setup-script-exit-code-fix.patch").
> 
> Do we necessarily want to try to re-run failed scripts the next time
> setup installs some packages?
> 
> Perhaps renaming to ".failed" would be better that not renaming.
> 
> Max.


  Perhaps setup should check when you first start it up whether there are any
postinstall scripts left lying around from last time and offer to run them for
you then and there?  Failed postinstalls should be run to completion *before*
next updating the package!


cheers,
  DaveK
-- 
Can't think of a witty .sigline today

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Max Bowsher]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Igor Peshansky wrote:
>> 2006-01-23  Igor Peshansky  <[EMAIL PROTECTED]>
>>
>>  * script.cc (Script::run): Fix inverted test for abnormal exit.
> 
> Ping (attached as "setup-script-exit-code-fix.patch-1").

Clearly correct, please commit.



>> 2006-01-31  Igor Peshansky  <[EMAIL PROTECTED]>
>>
>>  * script.cc (Script::run): Fix inverted test for abnormal exit.
>>  Do not rename to ".done" unless completed successfully.
> 
> And ping (attached as "setup-script-exit-code-fix.patch").

Do we necessarily want to try to re-run failed scripts the next time
setup installs some packages?

Perhaps renaming to ".failed" would be better that not renaming.

Max.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.1 (Cygwin)

iD8DBQFEELa2fFNSmcDyxYARAtZgAKCMt94Mi7e30wXEBuVWFMiKPmn5IwCghUTV
zlM8FB3Pj9Ka7DMKsMG7F44=
=dpUu
-END PGP SIGNATURE-

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Thu, 9 Mar 2006, Igor Peshansky wrote:

> P.S. A note for the archives: now that setup has a proper mechanism to
> detect failed postinstall scripts... [snip]

Whoops, I guess I misremembered that the patch has not yet been applied.
So...

On Mon, 23 Jan 2006, Igor Peshansky wrote:

> Just noticed that the postinstall mechanism in setup inverts the test for
> abnormal exit.  The attached one-liner patch fixes that.  ChangeLog below.
>   Igor
> ==
> 2006-01-23  Igor Peshansky  <[EMAIL PROTECTED]>
>
>   * script.cc (Script::run): Fix inverted test for abnormal exit.

Ping (attached as "setup-script-exit-code-fix.patch-1").

On Tue, 31 Jan 2006, Igor Peshansky wrote:

> Dave's advice in , made
> me realize that setup will always rename the script to ".done", even if it
> didn't complete properly (i.e., exited with a non-zero exit code).  Now
> that we detect the code correctly, let's also use that to not rename
> failed scripts for easier detection (patch attached).  ChangeLog below.
>
> Note that this will put a bit of extra burden on package maintainers to
> ensure that their scripts always return a zero exit code.  Most scripts
> already do that, though.
>   Igor
> ==
> 2006-01-31  Igor Peshansky  <[EMAIL PROTECTED]>
>
>   * script.cc (Script::run): Fix inverted test for abnormal exit.
>   Do not rename to ".done" unless completed successfully.

And ping (attached as "setup-script-exit-code-fix.patch").

In light of the "extra burden" consideration, let's look at those
separately.
Igor
-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"Index: script.cc
===
RCS file: /cvs/cygwin-apps/setup/script.cc,v
retrieving revision 2.21
diff -u -p -r2.21 script.cc
--- script.cc   14 Oct 2005 04:00:50 -  2.21
+++ script.cc   23 Jan 2006 17:07:02 -
@@ -243,7 +243,7 @@ Script::run() const
   if (to_log && !file_out.isEmpty ())
 log(LOG_BABBLE) << file_out << endLog;
 
-  if (!retval)
+  if (retval)
 log(LOG_PLAIN) << "abnormal exit: exit code=" << retval << endLog;;
 
   /* if file exists then delete it otherwise just ignore no file error */
Index: script.cc
===
RCS file: /cvs/cygwin-apps/setup/script.cc,v
retrieving revision 2.21
diff -u -p -r2.21 script.cc
--- script.cc   14 Oct 2005 04:00:50 -  2.21
+++ script.cc   31 Jan 2006 14:39:40 -
@@ -243,14 +243,15 @@ Script::run() const
   if (to_log && !file_out.isEmpty ())
 log(LOG_BABBLE) << file_out << endLog;
 
-  if (!retval)
+  if (retval)
 log(LOG_PLAIN) << "abnormal exit: exit code=" << retval << endLog;;
 
   /* if file exists then delete it otherwise just ignore no file error */
   io_stream::remove (String ("cygfile://") + scriptName + ".done");
 
-  io_stream::move (String ("cygfile://") + scriptName,
-   String ("cygfile://") + scriptName + ".done");
+  if (!retval)
+io_stream::move (String ("cygfile://") + scriptName,
+ String ("cygfile://") + scriptName + ".done");
 
   return retval;
 }

Re: [PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

On Mon, 23 Jan 2006, Igor Peshansky wrote:

> Just noticed that the postinstall mechanism in setup inverts the test for
> abnormal exit.  The attached one-liner patch fixes that.  ChangeLog below.

Dave's advice in , made
me realize that setup will always rename the script to ".done", even if it
didn't complete properly (i.e., exited with a non-zero exit code).  Now
that we detect the code correctly, let's also use that to not rename
failed scripts for easier detection (patch attached).  ChangeLog below.

Note that this will put a bit of extra burden on package maintainers to
ensure that their scripts always return a zero exit code.  Most scripts
already do that, though.
Igor
==
2006-01-31  Igor Peshansky  <[EMAIL PROTECTED]>

* script.cc (Script::run): Fix inverted test for abnormal exit.
Do not rename to ".done" unless completed successfully.

-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"Index: script.cc
===
RCS file: /cvs/cygwin-apps/setup/script.cc,v
retrieving revision 2.21
diff -u -p -r2.21 script.cc
--- script.cc   14 Oct 2005 04:00:50 -  2.21
+++ script.cc   31 Jan 2006 14:39:40 -
@@ -243,14 +243,15 @@ Script::run() const
   if (to_log && !file_out.isEmpty ())
 log(LOG_BABBLE) << file_out << endLog;
 
-  if (!retval)
+  if (retval)
 log(LOG_PLAIN) << "abnormal exit: exit code=" << retval << endLog;;
 
   /* if file exists then delete it otherwise just ignore no file error */
   io_stream::remove (String ("cygfile://") + scriptName + ".done");
 
-  io_stream::move (String ("cygfile://") + scriptName,
-   String ("cygfile://") + scriptName + ".done");
+  if (!retval)
+io_stream::move (String ("cygfile://") + scriptName,
+ String ("cygfile://") + scriptName + ".done");
 
   return retval;
 }

[PATCH] setup: fix abnormal exit test for postinstall scripts

[Igor Peshansky]

Hi,

Just noticed that the postinstall mechanism in setup inverts the test for
abnormal exit.  The attached one-liner patch fixes that.  ChangeLog below.
Igor
==
2006-01-23  Igor Peshansky  <[EMAIL PROTECTED]>

* script.cc (Script::run): Fix inverted test for abnormal exit.

-- 
http://cs.nyu.edu/~pechtcha/
  |\  _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-.  ;-;;,_Igor Peshansky, Ph.D. (name changed!)
 |,4-  ) )-,_. ,\ (  `'-'   old name: Igor Pechtchanski
'---''(_/--'  `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"Index: script.cc
===
RCS file: /cvs/cygwin-apps/setup/script.cc,v
retrieving revision 2.21
diff -u -p -r2.21 script.cc
--- script.cc   14 Oct 2005 04:00:50 -  2.21
+++ script.cc   23 Jan 2006 17:07:02 -
@@ -243,7 +243,7 @@ Script::run() const
   if (to_log && !file_out.isEmpty ())
 log(LOG_BABBLE) << file_out << endLog;
 
-  if (!retval)
+  if (retval)
 log(LOG_PLAIN) << "abnormal exit: exit code=" << retval << endLog;;
 
   /* if file exists then delete it otherwise just ignore no file error */