public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [PATCH setup] Add perpetual support for preremove scripts
@ 2022-06-26 16:33 Christian Franke
  2022-06-29 13:13 ` Jon Turney
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Franke @ 2022-06-26 16:33 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Use case: I ITP etckeeper (https://etckeeper.branchable.com/) which I 
frequently use on Debian. For fully automatic operation, it requires 
pre-install and post-install hooks, e.g:

/etc/preremove/0p_000_etckeeper_pre-install.sh
/etc/postinstall/zp_zzz_etckeeper_post-install.sh

This patch adds the missing functionality to run the pre-install hook. 
It is limited to /etc/preremove/0p_* because there is possibly no use 
case for /etc/preremove/zp_*.

'class Perpetual0RemoveFindVisitor' is borrowed from postinstall.cc and 
modified. It still uses the ugly pre-C++11 hack to disable copy-ctor and 
operator=. Possible refactoring like merging all 3 mostly similar 
visitors into one (or if C++11 is now allowed, use lambda functions 
instead) are left for later.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Add-perpetual-support-for-preremove-scripts.patch --]
[-- Type: text/plain, Size: 2945 bytes --]

From cae15278d705807da53b0abdeff5ab6b15fb479c Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sun, 26 Jun 2022 18:01:03 +0200
Subject: [PATCH] Add perpetual support for preremove scripts

Always run all /etc/preremove/0p_* first.
---
 install.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/install.cc b/install.cc
index e58ef4b..6689a08 100644
--- a/install.cc
+++ b/install.cc
@@ -33,6 +33,7 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <process.h>
+#include <algorithm>
 
 #include "ini.h"
 #include "resource.h"
@@ -50,6 +51,8 @@
 #include "archive.h"
 #include "archive_tar.h"
 #include "script.h"
+#include "find.h"
+#include "FindVisitor.h"
 
 #include "package_db.h"
 #include "package_meta.h"
@@ -73,6 +76,28 @@ struct std_dirs_t {
   mode_t mode;
 };
 
+class Perpetual0RemoveFindVisitor : public FindVisitor
+{
+public:
+  explicit Perpetual0RemoveFindVisitor (std::vector<Script> *scripts)
+    : _scripts(scripts)
+  {}
+  virtual void visitFile(const std::string& basePath,
+                         const WIN32_FIND_DATA *theFile)
+    {
+      std::string fn = std::string("/etc/preremove/") + theFile->cFileName;
+      Script script(fn);
+      if (script.is_p("0"))
+	  _scripts->push_back(Script (fn));
+    }
+  virtual ~ Perpetual0RemoveFindVisitor () {}
+protected:
+  Perpetual0RemoveFindVisitor (Perpetual0RemoveFindVisitor const &);
+  Perpetual0RemoveFindVisitor & operator= (Perpetual0RemoveFindVisitor const &);
+private:
+  std::vector<Script> *_scripts;
+};
+
 class Installer
 {
   public:
@@ -80,6 +105,7 @@ class Installer
     Installer();
     void initDialog();
     void progress (int bytes);
+    void preremovePerpetual0 ();
     void preremoveOne (packagemeta &);
     void uninstallOne (packagemeta &);
     void replaceOnRebootFailed (const std::string& fn);
@@ -150,6 +176,24 @@ Installer::StandardDirs[] = {
 
 static int num_installs, num_uninstalls;
 
+void
+Installer::preremovePerpetual0 ()
+{
+  std::vector<Script> perpetual;
+  Perpetual0RemoveFindVisitor visitor (&perpetual);
+  Find (cygpath ("/etc/preremove")).accept (visitor);
+  if (perpetual.empty())
+    return;
+
+  Progress.SetText1 (IDS_PROGRESS_PREREMOVE);
+  Progress.SetText2 ("0/Perpetual");
+  std::sort (perpetual.begin(), perpetual.end());
+  for (std::vector<Script>::iterator i = perpetual.begin (); i != perpetual.end (); ++i) {
+    Progress.SetText3 (i->fullName ().c_str());
+    i->run();
+  }
+}
+
 void
 Installer::preremoveOne (packagemeta & pkg)
 {
@@ -859,6 +903,9 @@ do_install_thread (HINSTANCE h, HWND owner)
   }
 
   /* start with uninstalls - remove files that new packages may replace */
+  Progress.SetBar2(0);
+  myInstaller.preremovePerpetual0 ();
+
   Progress.SetBar2(0);
   for (std::vector <packageversion>::iterator i = uninstall_q.begin ();
        i != uninstall_q.end (); ++i)
-- 
2.36.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Add perpetual support for preremove scripts
  2022-06-26 16:33 [PATCH setup] Add perpetual support for preremove scripts Christian Franke
@ 2022-06-29 13:13 ` Jon Turney
  2022-06-29 15:09   ` Christian Franke
  0 siblings, 1 reply; 6+ messages in thread
From: Jon Turney @ 2022-06-29 13:13 UTC (permalink / raw)
  To: Christian Franke, cygwin-apps

On 26/06/2022 17:33, Christian Franke wrote:
> Use case: I ITP etckeeper (https://etckeeper.branchable.com/) which I 
> frequently use on Debian. For fully automatic operation, it requires 
> pre-install and post-install hooks, e.g:
> 
> /etc/preremove/0p_000_etckeeper_pre-install.sh
> /etc/postinstall/zp_zzz_etckeeper_post-install.sh
> 
> This patch adds the missing functionality to run the pre-install hook. 
> It is limited to /etc/preremove/0p_* because there is possibly no use 
> case for /etc/preremove/zp_*.

Thanks.

I'm not sure what you mean by 'there is possibly no use case': That you 
don't have one currently, or that you've reasoned that there can't be one?

> 'class Perpetual0RemoveFindVisitor' is borrowed from postinstall.cc and 
> modified. It still uses the ugly pre-C++11 hack to disable copy-ctor and 
> operator=. Possible refactoring like merging all 3 mostly similar 
> visitors into one (or if C++11 is now allowed, use lambda functions 
> instead) are left for later.

Yeah, some refactoring would make this simpler and easier to understand. :)

I applied this patch.

Can you please also write a patch for [1] (source in [2]) to document this?

[1] https://cygwin.com/packaging-package-files.html#postinstall
[2] 
https://cygwin.com/git/?p=cygwin-htdocs.git;a=blob;f=packaging-package-files.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Add perpetual support for preremove scripts
  2022-06-29 13:13 ` Jon Turney
@ 2022-06-29 15:09   ` Christian Franke
  2022-06-29 18:35     ` Christian Franke
  2022-07-01 17:03     ` Christian Franke
  0 siblings, 2 replies; 6+ messages in thread
From: Christian Franke @ 2022-06-29 15:09 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

Jon Turney wrote:
> On 26/06/2022 17:33, Christian Franke wrote:
>> Use case: I ITP etckeeper (https://etckeeper.branchable.com/) which I 
>> frequently use on Debian. For fully automatic operation, it requires 
>> pre-install and post-install hooks, e.g:
>>
>> /etc/preremove/0p_000_etckeeper_pre-install.sh
>> /etc/postinstall/zp_zzz_etckeeper_post-install.sh
>>
>> This patch adds the missing functionality to run the pre-install 
>> hook. It is limited to /etc/preremove/0p_* because there is possibly 
>> no use case for /etc/preremove/zp_*.
>
> Thanks.
>
> I'm not sure what you mean by 'there is possibly no use case': That 
> you don't have one currently, or that you've reasoned that there can't 
> be one?
>

I don't have one currently and found none which is useful in practice, 
but cannot prove that there is none. If desired, I could provide a patch 
which adds 'zp_*' support.


>> 'class Perpetual0RemoveFindVisitor' is borrowed from postinstall.cc 
>> and modified. It still uses the ugly pre-C++11 hack to disable 
>> copy-ctor and operator=. Possible refactoring like merging all 3 
>> mostly similar visitors into one (or if C++11 is now allowed, use 
>> lambda functions instead) are left for later.
>
> Yeah, some refactoring would make this simpler and easier to 
> understand. :)
>
> I applied this patch.
>

Thanks. I found a minor GUI issue during testing: Script filename 
display persists during package remove phase. Fixed with attached patch.


> Can you please also write a patch for [1] (source in [2]) to document 
> this?
>
> [1] https://cygwin.com/packaging-package-files.html#postinstall
> [2] 
> https://cygwin.com/git/?p=cygwin-htdocs.git;a=blob;f=packaging-package-files.html
>

Of course. I will possibly wait until my ITP of etckeeper is accepted to 
have a real world example for the doc.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Clear-filename-on-GUI-after-running-perpetual-prerem.patch --]
[-- Type: text/plain, Size: 575 bytes --]

From dd31653b99c3a970fd145d2ffbad5809de7e0273 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Wed, 29 Jun 2022 16:51:47 +0200
Subject: [PATCH] Clear filename on GUI after running perpetual preremove
 scripts

---
 install.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/install.cc b/install.cc
index 6689a08..0ceb05f 100644
--- a/install.cc
+++ b/install.cc
@@ -192,6 +192,7 @@ Installer::preremovePerpetual0 ()
     Progress.SetText3 (i->fullName ().c_str());
     i->run();
   }
+  Progress.SetText3 ("");
 }
 
 void
-- 
2.36.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Add perpetual support for preremove scripts
  2022-06-29 15:09   ` Christian Franke
@ 2022-06-29 18:35     ` Christian Franke
  2022-07-02 12:35       ` Jon Turney
  2022-07-01 17:03     ` Christian Franke
  1 sibling, 1 reply; 6+ messages in thread
From: Christian Franke @ 2022-06-29 18:35 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

Christian Franke wrote:
> Jon Turney wrote:
>> On 26/06/2022 17:33, Christian Franke wrote:
>>> ...
>>> This patch adds the missing functionality to run the pre-install 
>>> hook. It is limited to /etc/preremove/0p_* because there is possibly 
>>> no use case for /etc/preremove/zp_*.
>>
>> Thanks.
>>
>> I'm not sure what you mean by 'there is possibly no use case': That 
>> you don't have one currently, or that you've reasoned that there 
>> can't be one?
>>
>
> I don't have one currently and found none which is useful in practice, 
> but cannot prove that there is none. If desired, I could provide a 
> patch which adds 'zp_*' support.
>

Meantime I realized that this is one of these cases where discussion may 
take longer than implementation. Attached is a patch ...


>> ...
>>
>> I applied this patch.
>>
>
> Thanks. I found a minor GUI issue during testing: Script filename 
> display persists during package remove phase. Fixed with attached patch.

... which should be applied on top of this last patch.


[-- Attachment #2: 0001-Also-run-stratum-z-perpetual-preremove-scripts.patch --]
[-- Type: text/plain, Size: 3527 bytes --]

From 7e3350f633f18e5639a109e0d779473e949ebe57 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Wed, 29 Jun 2022 19:57:26 +0200
Subject: [PATCH] Also run stratum 'z' perpetual preremove scripts

---
 install.cc | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/install.cc b/install.cc
index 0ceb05f..1fdc699 100644
--- a/install.cc
+++ b/install.cc
@@ -76,26 +76,28 @@ struct std_dirs_t {
   mode_t mode;
 };
 
-class Perpetual0RemoveFindVisitor : public FindVisitor
+class PerpetualRemoveFindVisitor : public FindVisitor
 {
 public:
-  explicit Perpetual0RemoveFindVisitor (std::vector<Script> *scripts)
-    : _scripts(scripts)
+  PerpetualRemoveFindVisitor (std::vector<Script> *scripts, const std::string& stratum)
+    : _scripts(scripts),
+      stratum(stratum)
   {}
   virtual void visitFile(const std::string& basePath,
                          const WIN32_FIND_DATA *theFile)
     {
       std::string fn = std::string("/etc/preremove/") + theFile->cFileName;
       Script script(fn);
-      if (script.is_p("0"))
+      if (script.is_p(stratum))
 	  _scripts->push_back(Script (fn));
     }
-  virtual ~ Perpetual0RemoveFindVisitor () {}
+  virtual ~ PerpetualRemoveFindVisitor () {}
 protected:
-  Perpetual0RemoveFindVisitor (Perpetual0RemoveFindVisitor const &);
-  Perpetual0RemoveFindVisitor & operator= (Perpetual0RemoveFindVisitor const &);
+  PerpetualRemoveFindVisitor (PerpetualRemoveFindVisitor const &);
+  PerpetualRemoveFindVisitor & operator= (PerpetualRemoveFindVisitor const &);
 private:
   std::vector<Script> *_scripts;
+  const std::string stratum;
 };
 
 class Installer
@@ -105,7 +107,7 @@ class Installer
     Installer();
     void initDialog();
     void progress (int bytes);
-    void preremovePerpetual0 ();
+    void preremovePerpetual (const std::string& stratum);
     void preremoveOne (packagemeta &);
     void uninstallOne (packagemeta &);
     void replaceOnRebootFailed (const std::string& fn);
@@ -177,16 +179,16 @@ Installer::StandardDirs[] = {
 static int num_installs, num_uninstalls;
 
 void
-Installer::preremovePerpetual0 ()
+Installer::preremovePerpetual (const std::string& stratum)
 {
   std::vector<Script> perpetual;
-  Perpetual0RemoveFindVisitor visitor (&perpetual);
+  PerpetualRemoveFindVisitor visitor (&perpetual, stratum);
   Find (cygpath ("/etc/preremove")).accept (visitor);
   if (perpetual.empty())
     return;
 
   Progress.SetText1 (IDS_PROGRESS_PREREMOVE);
-  Progress.SetText2 ("0/Perpetual");
+  Progress.SetText2 ((stratum + "/Perpetual").c_str ());
   std::sort (perpetual.begin(), perpetual.end());
   for (std::vector<Script>::iterator i = perpetual.begin (); i != perpetual.end (); ++i) {
     Progress.SetText3 (i->fullName ().c_str());
@@ -905,7 +907,7 @@ do_install_thread (HINSTANCE h, HWND owner)
 
   /* start with uninstalls - remove files that new packages may replace */
   Progress.SetBar2(0);
-  myInstaller.preremovePerpetual0 ();
+  myInstaller.preremovePerpetual ("0");
 
   Progress.SetBar2(0);
   for (std::vector <packageversion>::iterator i = uninstall_q.begin ();
@@ -917,6 +919,9 @@ do_install_thread (HINSTANCE h, HWND owner)
     Progress.SetBar2(std::distance(uninstall_q.begin(), i) + 1, uninstall_q.size());
   }
 
+  Progress.SetBar2(0);
+  myInstaller.preremovePerpetual ("z");
+
   Progress.SetBar2(0);
   for (std::vector <packageversion>::iterator i = uninstall_q.begin ();
        i != uninstall_q.end (); ++i)
-- 
2.36.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Add perpetual support for preremove scripts
  2022-06-29 15:09   ` Christian Franke
  2022-06-29 18:35     ` Christian Franke
@ 2022-07-01 17:03     ` Christian Franke
  1 sibling, 0 replies; 6+ messages in thread
From: Christian Franke @ 2022-07-01 17:03 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

Christian Franke wrote:
> Jon Turney wrote:
> ...
>> Can you please also write a patch for [1] (source in [2]) to document 
>> this?
>>
>> [1] https://cygwin.com/packaging-package-files.html#postinstall
>> [2] 
>> https://cygwin.com/git/?p=cygwin-htdocs.git;a=blob;f=packaging-package-files.html
>>
>
> Of course. I will possibly wait until my ITP of etckeeper is accepted 
> to have a real world example for the doc.
>

Patch attached. Written under the assumption that "[PATCH] Also run 
stratum 'z' perpetual preremove scripts" and "[ITP] etckeeper 1.18.17-1" 
will eventually be accepted :-)


[-- Attachment #2: 0001-Add-perpetual-pre-remove-scripts.patch --]
[-- Type: text/plain, Size: 3634 bytes --]

From b8225603a5d66760445c04ec14861764deb1489f Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Fri, 1 Jul 2022 18:52:17 +0200
Subject: [PATCH] Add perpetual pre-remove scripts

---
 packaging-package-files.html | 40 +++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/packaging-package-files.html b/packaging-package-files.html
index 446e62db..5b214f53 100755
--- a/packaging-package-files.html
+++ b/packaging-package-files.html
@@ -223,27 +223,35 @@ etc...
   after it is installed.
 </p>
 
-<h3>Perpetual post-install scripts</h3>
+<h3>Perpetual post-install and pre-remove scripts</h3>
 <p>
-  In addition to the ordinary ("run-once") post-install scripts described above,
-  the setup program supports "perpetual" post-install scripts.  These are run on
-  every invocation of setup, as long as the package is still installed.
-  Perpetual post-install scripts are distinguished from run-once scripts by
+  In addition to the ordinary ("run-once") scripts described above,
+  the setup program supports "perpetual" post-install and pre-remove scripts.
+  These are run on every invocation of setup, as long as the package is still
+  installed.  Perpetual scripts are distinguished from run-once scripts by
   having names that start with "0p_" or "zp_".  Those that start with "0p_" are
-  run before the run-once scripts, and those that start with "zp_" are run after
-  the run-once scripts.  Examples include
-  <code>0p_000_autorebase.dash</code> (provided by the <code>_autorebase</code> package)
-  and <code>0p_update-info-dir.dash</code> (provided by the <code>info</code> package).
+  run before the run-once scripts, and those that start with "zp_" are run
+  after the run-once scripts.  Examples include
+  <code>postinstall/0p_000_autorebase.dash</code> (provided by the
+  <code>_autorebase</code> package),
+  <code>postinstall/0p_update-info-dir.dash</code> (provided by the
+  <code>info</code> package),
+  <code>postinstall/zp_zzz_etckeeper_post-install.sh</code> and
+  <code>preremove/0p_000_etckeeper_pre-install.sh</code> (provided by the
+  <code>etckeeper</code> package).
 </p>
 <p>
   For those package maintainers wanting to employ perpetual scripts, the first
-  thing to keep in mind is to only use this feature for things that really can't
-  be done with run-once scripting.  Any perpetual script should minimize the
-  resources used (use dash instead of bash for instance) and exit at the
-  earliest possible moment if no action is required.  Scripts of type "0p_" must
-  be able to run with the Base packages installed but the post-install scripts
-  not yet executed; in practical terms that rules out using bash scripts.  This
-  limitation does not apply to scripts of type "zp_".
+  thing to keep in mind is to only use this feature for things that really
+  can't be done with run-once scripting.  Any perpetual script should minimize
+  the resources used (use dash instead of bash for instance) and exit at the
+  earliest possible moment if no action is required.  Post-install scripts of
+  type "0p_" must be able to run with the Base packages installed but the
+  remaining post-install scripts not yet executed; in practical terms that
+  rules out using bash scripts.  Pre-remove scripts of type "zp_" must be able
+  to run with the other pre-remove scripts already executed.  These limitations
+  do not apply to post-install scripts of type "zp_" and pre-remove scripts of
+  type "0p_".
 </p>
 <p>
   See <a href="https://cygwin.com/ml/cygwin-apps/2014-12/msg00148.html">this
-- 
2.36.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Add perpetual support for preremove scripts
  2022-06-29 18:35     ` Christian Franke
@ 2022-07-02 12:35       ` Jon Turney
  0 siblings, 0 replies; 6+ messages in thread
From: Jon Turney @ 2022-07-02 12:35 UTC (permalink / raw)
  To: Christian Franke, cygwin-apps

On 29/06/2022 19:35, Christian Franke wrote:
> Christian Franke wrote:
>> Jon Turney wrote:
>>> On 26/06/2022 17:33, Christian Franke wrote:
>>>> ...
>>>> This patch adds the missing functionality to run the pre-install 
>>>> hook. It is limited to /etc/preremove/0p_* because there is possibly 
>>>> no use case for /etc/preremove/zp_*.
>>>
>>> Thanks.
>>>
>>> I'm not sure what you mean by 'there is possibly no use case': That 
>>> you don't have one currently, or that you've reasoned that there 
>>> can't be one?
>>>
>>
>> I don't have one currently and found none which is useful in practice, 
>> but cannot prove that there is none. If desired, I could provide a 
>> patch which adds 'zp_*' support.
>>
> 
> Meantime I realized that this is one of these cases where discussion may 
> take longer than implementation. Attached is a patch ...
> 
> 
>>> ...
>>>
>>> I applied this patch.
>>>
>>
>> Thanks. I found a minor GUI issue during testing: Script filename 
>> display persists during package remove phase. Fixed with attached patch.
> 
> ... which should be applied on top of this last patch.

Thanks.  Applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-02 12:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-26 16:33 [PATCH setup] Add perpetual support for preremove scripts Christian Franke
2022-06-29 13:13 ` Jon Turney
2022-06-29 15:09   ` Christian Franke
2022-06-29 18:35     ` Christian Franke
2022-07-02 12:35       ` Jon Turney
2022-07-01 17:03     ` Christian Franke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).