This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] incremental rebase


On 11/24/2014 3:19 PM, Ken Brown wrote:
On 11/23/2014 3:17 PM, Achim Gratz wrote:

In my attempt to implement post-postinstall scripts for Ken to use for
TeXlive it turned out that the same hack I used for the pre-postinstall
perpetual scripts wouldn't really work.  So I've implemented the
stratified postinstall already, at the moment limited to three strata:
0, default and z; as well as perpetual and runonce scripts (on each
stratum).  No provisions for query and trigger scripts is made at the
moment, as these can be implemented with some manual effort with the two
script types that are already provided.  This is extensible, although at
the moment I have to configure the available strata twice, so there
needs to be some more refactoring in that area.

Since I've coded this up on my new Linux box which already uses gcc-4.9
I had to do some preparatory fixes (the log macro clashing with the log
function) leading up to the meat of the implementation.  The patch
implementing the stratified postinstall is here:

http://repo.or.cz/w/cygwin-setup.git/commitdiff/d1df7acc1dce40c97ddfaa2de38542a3a269004e


Thanks for doing this!  A couple of comments:

bool
 Script::isAScript (const std::string& file)
 {
-    /* file may be /etc/postinstall or etc/postinstall */
-    if (casecompare(file, ETCPostinstall, sizeof(ETCPostinstall)-1) &&
-       casecompare(file, ETCPostinstall+1, sizeof(ETCPostinstall)-2))
-      return false;
-    if (file.c_str()[file.size() - 1] == '/')
+    // is a directory path
+    if (file.size() == file.rfind('/'))
       ^^^^^^^^^^^^^^^
Shouldn't this be file.size() - 1?

+bool
+Script::match (const std::string& stratum, const std::string& type)
 {
-
+  if ("done" == scriptExtension)
+    return false;
+  bool matchedStratum = false;
+  if (stratum.size())
+    {
+      std::size_t found = stratum.find(scriptStratum);
+      matchedStratum = ((std::string::npos != found) &&
+                       (std::string::npos !=
stratum.substr(found,1).find_first_of(allowedStrata)));

Is this last test necessary?  The Script constructor guarantees that the
stratum is allowable.

+    }
+  else
+    matchedStratum = true;
+  bool matchedType = true;
+  if (type.size())
+    {
+      std::size_t found = type.find(scriptType);
+      matchedType = ((std::string::npos != found) &&
+                    (std::string::npos !=
type.substr(found,1).find_first_of(allowedTypes)));

Ditto.

Two more comments:

1. You should do something to make sure that incremental rebase (or whatever version of autorebase gets chosen) is run first among the perpetual postinstall scripts in stratum 0. One possibility would be to also allow stratum 1, and require that no packages other than incremental rebase use stratum 0. Alternatively, give the incremental rebase script a name that guarantees that it comes first in whatever order the perpetual scripts are run in.

2. You've used the underscore for two unrelated purposes: On the one hand, it separates the prefix from the rest of the script name. On the other hand, it denotes the default stratum. This makes it impossible to create a perpetual postinstall script in the default stratum if someone would ever want to do this. Maybe use a hyphen instead of underscore for the separator?

Ken


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]