setup.exe rebase patch

setup.exe rebase patch

[Jason Tishler]

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

Attached is a patch that adds the rebase functionality to setup.exe.
I would like to get some feedback before I start to resolve the following
issues:

    o How to handle a missing config file (i.e., /etc/setup/rebase.conf)?
    o Need to handle in-use files.
    o How to handle corruptible (by rebasing) files?
    o How should dependent DLLs be handled?

and some other more niggling items.

I re-installed 30 out of the 33 packages that contain DLLs and my system
is functioning normally.  Python can even fork too!  I only skipped curl,
postgresql, and rxvt because the DLLs contained by these packages get
corrupted by rebasing.

If you would like to test out a setup.exe which includes the rebase
functionality, then a pre-built version is available at:

    http://www.tishler.net/jason/software/setup-rebase/setup-rebase-2002-02-02-1.exe

Before you run this setup.exe, you should download the default rebase
configuration file:

    http://www.tishler.net/jason/software/setup-rebase/rebase.conf

to /etc/setup.

I also have a stand-alone rebase.exe that I would like to contribute to
Cygwin so that users can rebase without having to run setup.exe.

Here is the current usage statement from rebase.exe:

    usage: rebase [-i|-r|-u] [-d CygwinRootDir] ImageFileName ...
    where:
        -i => install ImageFileName(s)
        -r => re-install ImageFileName(s) (default)
        -u => uninstall ImageFileName(s)
        -d => set Cygwin root directory (defaults to ".")

A pre-built version of rebase.exe is available at:

    http://www.tishler.net/jason/software/setup-rebase/rebase.exe

I see the following uses for this program:

    o makefiles
    o resync system (hopefully never needed)
    o used by a one-shot converter script

The following is an example of a postinstall script, rebase.bat, that
could rebase the entire user's system without have to reinstall
"everything":

    gzip -d -c /etc/setup/*.lst.gz | grep "\.dll$" | sed "s=^=/=" | xargs -n 1 cygpath -wa >rebase.tmp
    rebase -r -f rebase.tmp
    del rebase.tmp

The above assumes that I have added a "-f" option to rebase.exe and
changed the default value for the "-d" option to be the Cygwin root
directory obtained from the Registry.

Any feedback on the above would be greatly appreciated.

Thanks,
Jason

[-- Attachment #2: rebase.patch --]
[-- Type: text/plain, Size: 4576 bytes --]

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/winsup/cinstall/Makefile.in,v
retrieving revision 2.49
diff -u -p -r2.49 Makefile.in
--- Makefile.in	2002/01/27 06:36:06	2.49
+++ Makefile.in	2002/02/07 11:58:26
@@ -69,11 +69,12 @@ libmingw32.a	:= $(mingw_build)/libmingw3
 libuser32	:= $(w32api_lib)/libuser32.a
 libkernel32	:= $(w32api_lib)/libkernel32.a
 libcomctl32	:= $(w32api_lib)/libcomctl32.a
+libimagehlp	:= $(w32api_lib)/libimagehlp.a
 
 ALL_DEP_LDLIBS	:= $(ZLIB) $(BZ2LIB) $(w32api_lib)/libole32.a $(w32api_lib)/libwsock32.a \
 		$(w32api_lib)/libnetapi32.a $(w32api_lib)/libadvapi32.a \
 		$(w32api_lib)/libuuid.a $(libkernel32) $(libuser32) \
-		$(libcomctl32) $(libmingw32)
+		$(libcomctl32) $(libimagehlp) $(libmingw32)
 
 ALL_LDLIBS	:= ${patsubst $(mingw_build)/lib%.a,-l%,\
 	      ${patsubst $(w32api_lib)/lib%.a,-l%,\
@@ -99,13 +100,17 @@ OBJS = \
 	compress_bz.o \
 	compress_gz.o \
 	concat.o \
+	config_file.o \
+	config_file_reader.o \
+	config_file_writer.o \
 	cygpackage.o \
 	desktop.o \
 	dialog.o \
 	diskfull.o \
 	download.o \
-	find.o \
 	filemanip.o \
+	find.o \
+	free_list.o \
 	fromcwd.o \
 	geturl.o \
 	hash.o \
@@ -120,6 +125,7 @@ OBJS = \
 	localdir.o \
 	log.o \
 	main.o \
+	main.o \
 	mingw_getopt.o \
 	mkdir.o \
 	mklink2.o \
@@ -127,10 +133,10 @@ OBJS = \
 	msg.o \
 	net.o \
 	netio.o \
-	nio-ie5.o \
 	nio-file.o \
 	nio-ftp.o \
 	nio-http.o \
+	nio-ie5.o \
 	package_db.o \
 	package_meta.o \
 	package_source.o \
@@ -142,6 +148,7 @@ OBJS = \
 	postinstall.o \
 	proppage.o \
 	propsheet.o \
+	rebaser.o \
 	res.o \
 	rfc1738.o \
 	root.o \
@@ -153,6 +160,7 @@ OBJS = \
 	splash.o \
 	state.o \
 	threebar.o \
+	used_list.o \
 	version.o \
 	win32.o \
 	window.o \
Index: install.cc
===================================================================
RCS file: /cvs/src/src/winsup/cinstall/install.cc,v
retrieving revision 2.39
diff -u -p -r2.39 install.cc
--- install.cc	2002/01/27 06:36:06	2.39
+++ install.cc	2002/02/07 11:58:32
@@ -62,6 +62,7 @@ static const char *cvsid = "\n%%% $Id: i
 #include "package_source.h"
 
 #include "port.h"
+#include "rebaser.h"
 
 #include "threebar.h"
 extern ThreeBarProgressPage Progress;
@@ -291,6 +292,8 @@ install_one_source (packagemeta & pkgm, 
 		    }
 		}
 	    }
+	  else
+	    rebaser::get_instance ()->rerebase (cygpath (prefixPath, fn, 0));
 
 	  progress (tmp->tell ());
 	  num_installs++;
Index: log.cc
===================================================================
RCS file: /cvs/src/src/winsup/cinstall/log.cc,v
retrieving revision 2.7
diff -u -p -r2.7 log.cc
--- log.cc	2002/01/22 12:30:50	2.7
+++ log.cc	2002/02/07 11:58:32
@@ -34,6 +34,7 @@ static const char *cvsid =
 #include "concat.h"
 #include "mkdir.h"
 #include "mount.h"
+#include "rebaser.h"
 
 struct LogEnt
 {
@@ -116,6 +117,8 @@ exit_setup (int exit_code)
 
   if (exit_msg)
     note (NULL, exit_msg);
+
+  rebaser::delete_instance ();
 
   log (LOG_TIMESTAMP, "Ending cygwin install");
 
Index: package_meta.cc
===================================================================
RCS file: /cvs/src/src/winsup/cinstall/package_meta.cc,v
retrieving revision 2.15
diff -u -p -r2.15 package_meta.cc
--- package_meta.cc	2002/01/22 09:03:55	2.15
+++ package_meta.cc	2002/02/07 11:58:32
@@ -42,6 +42,7 @@ static const char *cvsid = "\n%%% $Id: p
 #include "cygpackage.h"
 #include "package_meta.h"
 #include "package_db.h"
+#include "rebaser.h"
 
 static const char *standard_dirs[] = {
   "/bin",
@@ -167,6 +168,7 @@ packagemeta::uninstall ()
 	      log (LOG_BABBLE, "unlink %s", d);
 	      SetFileAttributes (d, dw & ~FILE_ATTRIBUTE_READONLY);
 	      DeleteFile (d);
+	      rebaser::get_instance ()->unrebase (d);
 	    }
 	  /* Check for Windows shortcut of same name. */
 	  d = concat (d, ".lnk", NULL);
Index: root.cc
===================================================================
RCS file: /cvs/src/src/winsup/cinstall/root.cc,v
retrieving revision 2.8
diff -u -p -r2.8 root.cc
--- root.cc	2001/12/23 12:13:29	2.8
+++ root.cc	2002/02/07 11:58:32
@@ -36,6 +36,7 @@ static const char *cvsid =
 #include "concat.h"
 #include "log.h"
 #include "root.h"
+#include "rebaser.h"
 
 static int rb[] = { IDC_ROOT_TEXT, IDC_ROOT_BINARY, 0 };
 static int su[] = { IDC_ROOT_SYSTEM, IDC_ROOT_USER, 0 };
@@ -180,6 +181,8 @@ RootPage::OnNext ()
     return -1;
 
   NEXT (IDD_LOCAL_DIR);
+
+  rebaser::set_cygwin_root_dir (get_root_dir ());
 
   log (0, "root: %s %s %s", get_root_dir (),
        (root_text == IDC_ROOT_TEXT) ? "text" : "binary",

[-- Attachment #3: rebase.ChangeLog --]
[-- Type: text/plain, Size: 806 bytes --]

2002-02-07  Jason Tishler  <jason@tishler.net>

	* Makefile.in (libimagehlp): New variable.
	(ALL_DEP_LDLIBS): Add libimagehlp.
	(OBJ): Add rebase objects.  Resort objects.
	* install.cc (install_one_source): Add call to rebaser::rerebase.
	* log.cc (exit_setup): Add call to rebaser::delete_instance.
	* package_meta.cc (packagemeta::uninstall): Add call to
	rebaser::unrebase.
	* root.cc (RootPage::OnNext): Add call to rebaser::set_cygwin_root_dir.
	* config_file.cc: New file.
	* config_file.h: New file.
	* config_file_reader.cc: New file.
	* config_file_reader.h: New file.
	* config_file_writer.cc: New file.
	* config_file_writer.h: New file.
	* free_list.cc: New file.
	* free_list.h: New file.
	* rebaser.cc: New file.
	* rebaser.h: New file.
	* used_list.cc: New file.
	* used_list.h: New file.

[-- Attachment #4: rebase.tar.bz2 --]
[-- Type: application/octet-stream, Size: 5312 bytes --]

Re: setup.exe rebase patch

[Robert Collins]

I'm snowed under right now. I'll be paying more attention late next
week...

Sorry for the delay.

Rob

Re: setup.exe rebase patch

[Jason Tishler]

Rob,

On Sat, Feb 09, 2002 at 06:30:12PM +1100, Robert Collins wrote:
> I'm snowed under right now. I'll be paying more attention late next
> week...
> 
> Sorry for the delay.

No problem -- thanks for the heads up.  Sometimes "no news is good news,"
but sometimes...  I was starting to wonder which one applied in this
case. :,)

Jason

CVS setup.exe doesn't update zlib

[Michael A Chase]

I've been using the latest CVS version of setup.exe for a couple weeks now.
The only real problem I'm seeing is that it won't update zlib.  I thought I
saw a proposed patch for that go by a while ago, but I don't have it now.
--
Mac :})
** I normally forward private questions to the appropriate mail list. **
Ask Smarter: http://www.tuxedo.org/~esr/faqs/smart-questions.htm
Give a hobbit a fish and he eats fish for a day.
Give a hobbit a ring and he eats fish for an age.

Re: CVS setup.exe doesn't update zlib

[Brian Keener]

Michael A Chase wrote:
> I've been using the latest CVS version of setup.exe for a couple weeks now.
> The only real problem I'm seeing is that it won't update zlib.  I thought I
> saw a proposed patch for that go by a while ago, but I don't have it now.
>
I think I have the patch but it was in Download I thought and I don't think it 
is that it won't update zlib (although you might be seeing a different problem) 
but I think it is not updating whatever is the last package I select for update 
and then only if I am doing an install from local directory - it seems to 
download it and install from the internet just fine - I've been trying to debug 
it but my skills are beginner at best.

The other couple of things I've noticed are cosmetic and personal and 
definitely not show stoppers although I really don't like that if I select the 
Test option that all the packages that don't have a test show up as uninstall 
as opposed to keep.  Also cosmetically speaking I think if I am doing an 
Install from Local directory and the Source file does not exist I should not 
have and option for the Source to be installed.

bk

Re: CVS setup.exe doesn't update zlib

[Robert Collins]

===
----- Original Message -----
From: "Brian Keener" <bkeener@thesoftwaresource.com>
>  Also cosmetically speaking I think if I am doing an
> Install from Local directory and the Source file does not exist I
should not
> have and option for the Source to be installed.

This _may_ be fixed.

Rob

Re: CVS setup.exe doesn't update zlib

[Jan Nieuwenhuizen]

"Michael A Chase" <mchase@ix.netcom.com> writes:

> I've been using the latest CVS version of setup.exe for a couple weeks now.
> The only real problem I'm seeing is that it won't update zlib.  I thought I
> saw a proposed patch for that go by a while ago, but I don't have it now.

But I have it, it got backed-up on the internet.

    http://sources.redhat.com/ml/cygwin-patches/2002-q1/msg00198.html

Greetings,    
Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: setup.exe rebase patch

[Robert Collins]

===
----- Original Message -----
From: "Jason Tishler" <jason@tishler.net>

Some ideas follow:

> Attached is a patch that adds the rebase functionality to setup.exe.
> I would like to get some feedback before I start to resolve the
following
> issues:
>
>     o How to handle a missing config file (i.e.,
/etc/setup/rebase.conf)?

Create one with sensible defaults. If no sensible defaults exist, add a
new tab.

>     o Need to handle in-use files.

Copy the file, rebase it, schedule for reboot replacement (see
install.cc for the mechanics).

>     o How to handle corruptible (by rebasing) files?

Hmm. Can we detect the corruption? Does OpenLibrary fail or something
nicely obvious like that?

>     o How should dependent DLLs be handled?

Recursively, as long as they are in the cygwin tree.

> and some other more niggling items.

Can you list them?

> I re-installed 30 out of the 33 packages that contain DLLs and my
system
> is functioning normally.  Python can even fork too!

Cool. This will probably help Ralf's KDE project too.

> I also have a stand-alone rebase.exe that I would like to contribute
to
> Cygwin so that users can rebase without having to run setup.exe.

Neato. This sounds like a cygutils thingo to me, or a new package.
Chuck? Ohhhh Chuck?

One important thing is to have setup's rebasing and the rebase
commandline tool *both* store their work in rebase.conf. That way they
won't require full rebasing when someone fiddles with something.

I've read through your patch. Can I ask you to redo it against HEAD?
(After updating cygpath etc to the String class.

Some feedback/thoughts on the current patch (not including what will
change during updating).

1) log.cc - don't clean up there. If you need to cleanup, have a static
object whose destructor should get called. It's on my mental todo list
to find out why the destructors aren't called (even if exit() is used
:--[). Like the initialisation of globals issue a while back, lets not
work around a compiler fault, but rather address the real issue. In your
case, a trivial wrapper (ie rebaser_deleter) that wraps the instance
pointer should do it. I also note the need to save the .conf file, I'd
suggest that that occurs at the end of the install process, not when the
user exits setup. (Setup might crash, they might do another run, run a
command line rebase...)
2) I really don't like the rebaser::set_cygwin_root_dir construct: why
not use get_root_dir () when you need the cygwin root? (Any why do you
need it? Surely you only need cygpath (filename) ?
3) config file should use io_streams, not stdio please.
4) New errors should go into the resource.rc stringtable. I'm guilty of
this myself, but no need to let it get worse :}.
5) If you've a complex .conf file, you might want to write a .y and .l
combo for it instead of manually parsing.
6) config_file_reader has a static buffer (errk).
7) I don't really grok your class hierarchy. Can you please explain it?
(I don't want to criticize or suggest alterations until I do grok it).
It *feels* procedural to me.
8) You seem to duplicate some io_stream functionality - i.e.
config_file_writer::rename. I don't see why a if (io_stream::rename
(temp_filename,real_filename) isn't enough.
9) Should we rename my 'list.h' to 'vector.h' and generalise your list
handling code into a template?
10) Please capitalise the first letter of words in class names. Setup
has been quite haphazard, I'm trying to be more consistent :}. Also, I
prefer things like FreeList to free_list - I find them easier.
11) All .h files should compile correctly if they are the only
translation unit. I'm not sure if that is or is not the case for your
files just now. Again, this is something I don't expect you to know, as
I'm still introducing it into setup.

Hope thats enough feedback for ya!
Rob

Re: setup.exe rebase patch

[Charles Wilson]

Robert Collins wrote:


>>I also have a stand-alone rebase.exe that I would like to contribute
>>
> to
> 
>>Cygwin so that users can rebase without having to run setup.exe.
>>
> 
> Neato. This sounds like a cygutils thingo to me, or a new package.
> Chuck? Ohhhh Chuck?


Ummm...do we really want to fork the code into two separate locations? 
It seems to me that the best thing to do would be the following:

winsup/cinstall/Makefile.am:
bin_PROGRAMS: setup rebase
setup_SOURCES: ..... rebase.cc rebase.l rebase.y rebase.h .....
rebase_SOURCES: rebase.cc rebase.l rebase.y rebase.h
rebase_CFLAGS: -dREBASE_STANDALONE

in rebase.cc:

#if REBASE_STANDALONE
int main(int argc, char * argv[]) {
...
}
#endif

This way, the SAME code is used by both the standalone rebase.exe and 
setup.exe.  Otherwise, we'll continually have synchronization problems 
between the two (e.g. what if the format of /etc/rebase.conf changes?)


> One important thing is to have setup's rebasing and the rebase
> commandline tool *both* store their work in rebase.conf. That way they
> won't require full rebasing when someone fiddles with something.
> 

Yeah -- which is why they should both come from the same code.  In the 
same location.  I *strongly* advocate adding rebase.exe to the cinstall 
directory since it will be so tightly tied to setup.exe's behavior...

--Chuck

Re: setup.exe rebase patch

[Jason Tishler]

On Sun, Feb 24, 2002 at 01:41:13PM -0500, Charles Wilson wrote:
> Robert Collins wrote:
> >Neato. This sounds like a cygutils thingo to me, or a new package.
> >Chuck? Ohhhh Chuck?
> 
> Ummm...do we really want to fork the code into two separate locations? 

No, they should somehow share the common code.

> It seems to me that the best thing to do would be the following:
> 
> [snip very reasonable proposal]
>
> Yeah -- which is why they should both come from the same code.  In the 
> same location.  I *strongly* advocate adding rebase.exe to the cinstall 
> directory since it will be so tightly tied to setup.exe's behavior...

cinstall is fine with me.  winsup/utils is another possibility.
I'm having problems deciding which is better.  I'm willing to go along
with the consensus.  Unless, Chris would like to make an executive
decision. :,)

Thanks,
Jason

Re: setup.exe rebase patch

[Christopher Faylor]

On Mon, Feb 25, 2002 at 10:36:24AM -0500, Jason Tishler wrote:
>On Sun, Feb 24, 2002 at 01:41:13PM -0500, Charles Wilson wrote:
>> Robert Collins wrote:
>> >Neato. This sounds like a cygutils thingo to me, or a new package.
>> >Chuck? Ohhhh Chuck?
>> 
>> Ummm...do we really want to fork the code into two separate locations? 
>
>No, they should somehow share the common code.
>
>> It seems to me that the best thing to do would be the following:
>> 
>> [snip very reasonable proposal]
>>
>> Yeah -- which is why they should both come from the same code.  In the 
>> same location.  I *strongly* advocate adding rebase.exe to the cinstall 
>> directory since it will be so tightly tied to setup.exe's behavior...
>
>cinstall is fine with me.  winsup/utils is another possibility.
>I'm having problems deciding which is better.  I'm willing to go along
>with the consensus.  Unless, Chris would like to make an executive
>decision. :,)

The program itself sounds more like something for cygutils than for
winsup/utils but if it needs to be tightly integrated with setup.exe, it
should probably just live in cinstall.

cgf

Re: setup.exe rebase patch

[Jason Tishler]

Rob,

On Sun, Feb 24, 2002 at 10:45:32PM +1100, Robert Collins wrote:
> ----- Original Message -----
> From: "Jason Tishler" <jason@tishler.net>
> 
> Some ideas follow:
> 
> > Attached is a patch that adds the rebase functionality to setup.exe.
> > I would like to get some feedback before I start to resolve the
> > following issues:
> >
> >     o How to handle a missing config file (i.e., /etc/setup/rebase.conf)?
> 
> Create one with sensible defaults. If no sensible defaults exist, add a
> new tab.

It is easy to arrive at one with sensible defaults.  What I was "really"
trying to ask above was the following:

    o What should create this file?  A postinstall script, a new rebase
      package, etc?
    o What should setup.exe do if the file is missing?  Proceed without
      rebasing (i.e., ignore missing rebase.conf), create a new (default)
      one, complain and exit, etc?

I guess that I was trying to ask a more philosophical question of how
coupled rebase was going to be with setup.exe.  It sounds like you are
comfortable with a tight coupling.  Is this correct?

> >     o Need to handle in-use files.
> 
> Copy the file, rebase it, schedule for reboot replacement (see
> install.cc for the mechanics).

I already had a plan for the above -- I just didn't want to implement it
until I got some feedback as to whether or not my rebase approach was
acceptable.  What I posted was more a proof of concept or prototype than
a finished patch.  I wanted to make sure that I wasn't off in "left
field" before I started to dot I's and cross T's.

> >     o How to handle corruptible (by rebasing) files?
> 
> Hmm. Can we detect the corruption?

Yes, objdump can, so...

> Does OpenLibrary fail or something nicely obvious like that?

I haven't tried LoadLibrary(), etc. but I presume that one of the
APIs can.  However, isn't this too little, too late?  What can we do
when the corruption occurs?  Rollback to the copy in the archive?  But,
then this image will *not* be rebased. :,(

I would like to propose the following alternative -- at least until
someone figures out how to prevent the rebase corruption problem.  To
refresh your memory, it seems that some *stripped* images  are not
rebasable, but "all" unstripped one are.

I recommend adding the requirement to setup.html that requires package
maintainer to test whether or not their stripped relocatable images
(e.g., DLLs) are rebasable.  If not, then they should release their
packages with those files unstripped.

For example, the Python DLLs are rebasable after stripping, so I
don't need to do anything special for the Python package.  However,
the PostgreSQL DLLs are not, so I need to release PostgreSQL with
*unstripped* DLLs.

Does the above sound like a reasonable interim solution?

> >     o How should dependent DLLs be handled?
> 
> Recursively, as long as they are in the cygwin tree.

Is this really necessary?  If we are going to only rebase Cygwin DLLs,
then the dependencies will ultimately be rebased when their package is
(re)installed.  If so, does setup.exe install dependencies first or
just alphabetically?  If alphabetically, then the dependent DLLs may
not be on disk yet and hence, cannot be rebased at the moment.

Regardless of the resolution to the above, can we declare this a second
order item to be dealt with during subsequent iterations?

> > and some other more niggling items.
> 
> Can you list them?

Here is my (dirty) laundry list so far:

    Deal with in-use files.
    Get config file prefix from registry.
    Warn on unrebases if base and/or size does not match used list entry.
    Refactor free_list and used_list to use common base class.
    Refactor used_list::find() and used_list::remove().
    Add dirty flag and write only if dirty.

I don't feel that any of them are shop stoppers.

> > I re-installed 30 out of the 33 packages that contain DLLs and my
> system
> > is functioning normally.  Python can even fork too!
> 
> Cool. This will probably help Ralf's KDE project too.
> 
> > I also have a stand-alone rebase.exe that I would like to contribute
> to
> > Cygwin so that users can rebase without having to run setup.exe.
> 
> Neato. This sounds like a cygutils thingo to me, or a new package.
> Chuck? Ohhhh Chuck?

See my (forthcoming) response to Chuck's email.

> One important thing is to have setup's rebasing and the rebase
> commandline tool *both* store their work in rebase.conf. That way they
> won't require full rebasing when someone fiddles with something.

This is already done!

In fact, since I posted my patch, I have been "eating my own dog
food."  I religiously have been using either a setup.exe with the
rebase functionality or my stand-alone rebase (e.g., for snapshots,
CVS builds, etc.) for all (rebase tolerant) installs on my main system.
I felt strongly that I should live with this "solution" before sic-ing
it on the general public.  So far, so good.

> I've read through your patch. Can I ask you to redo it against HEAD?
> (After updating cygpath etc to the String class.

Yes, I guess that this is good news?

> Some feedback/thoughts on the current patch (not including what will
> change during updating).
> 
> 1) log.cc - don't clean up there. If you need to cleanup, have a static
> object whose destructor should get called.

This is already done too (see rebaser.cc:338).  Note that my stand-alone
rebase.exe is already using this successfully.

> It's on my mental todo list
> to find out why the destructors aren't called (even if exit() is used
> :--[).

The above is why I called the clean-up routine explicitly.

> Like the initialisation of globals issue a while back, lets not
> work around a compiler fault, but rather address the real issue.

However, I thought that it was an artifact of calling ExitProcess()
instead of exit().

> In your
> case, a trivial wrapper (ie rebaser_deleter) that wraps the instance
                              ^^^^^^^^^^^^^^^
> pointer should do it.

Hmm...  Where did you get the above name?  rebaser.cc:338 perchance? :,)

> I also note the need to save the .conf file, I'd
> suggest that that occurs at the end of the install process, not when the
> user exits setup. (Setup might crash, they might do another run, run a
> command line rebase...)

Agreed, I had this concern too.  I will add this to my todo list.

> 2) I really don't like the rebaser::set_cygwin_root_dir construct: why
> not use get_root_dir () when you need the cygwin root? (Any why do you
> need it? Surely you only need cygpath (filename) ?

This is for the stand-alone case which I wasn't sure was going to be
able to easily share code with setup.exe.  This concern appears to
be mitigated.  I can certainly have rebaser always retrieve the Cygwin
root from the registry if this is deemed the better approach.

> 3) config file should use io_streams, not stdio please.

Sigh...  Oh, where oh where has my little standard C++ library gone?
Oh, where oh where has it gone? :,)

I only used stdio because I developed the stand-alone rebase first to
speed development.  I had always intended to replace the *printf() calls
during integration.

I have adding this to my todo list.

> 4) New errors should go into the resource.rc stringtable. I'm guilty of
> this myself, but no need to let it get worse :}.

I presume that you mean the *printf() messages?  I have added this to my
todo list.  However, will this be OK for the stand-alone version?  That
is the stand-alone version a console app while setup.exe is a GUI one.

> 5) If you've a complex .conf file, you might want to write a .y and .l
> combo for it instead of manually parsing.

Sorry, I don't have any experience with YACC and lex.  But, I will learn
how to use them if it deemed necessary.

Can this be handled as a second order item too?

BTW, I have been using the silly MS .ini file format.  I am quite open to
suggestions regarding better formats.

> 6) config_file_reader has a static buffer (errk).

Agreed that this is ugly -- in fact, this is the one area that I'm least
happy with.  Note I didn't want to get bogged down with parsing issues
during the proof of concept phase but I have added this to my list too.
Since this is encapsulated I can easily change this implementation
without any rippling effects.

What I needed is push back (onto the input stream) functionality.  Any
suggestions on a cleaner implementation is appreciated.

> 7) I don't really grok your class hierarchy. Can you please explain it?

There is not much to it so I will attempt to explain it textually.  I can
send you a UML (i.e., Visio) diagram, if necessary.

The rebaser class uses the singleton design pattern.  This seemed
appropriate and minimized the impact of adding the rebase functionality
to setup.exe.  rebaser uses (as in has-a) free_list and used_list.
rebase also uses config_file_reader and config_file_writer.
config_file_reader and config_file_writer inherit from config_file.

That's it -- there is not much to grok.  Although, I readily admit that
trying to understand new code without any comments and no documentation
is never fun (and easy).

> (I don't want to criticize or suggest alterations until I do grok it).
> It *feels* procedural to me.

Sniff, sniff -- you used the P word! :,(

I tried to find the appropriate abstractions in the problem domain.
I tried to encapsulate implementations behind reasonable interfaces to
minimize the impact of implementations changes.  Etc, etc.  However,
I'm open to design changes too, if deemed necessary.

> 8) You seem to duplicate some io_stream functionality - i.e.
> config_file_writer::rename. I don't see why a if (io_stream::rename
> (temp_filename,real_filename) isn't enough.

Again due to stand-alone development -- to be dealt with during
integration.

> 9) Should we rename my 'list.h' to 'vector.h'

vector does seem to better capture the semantics.  But, what is the
relevance to my list code?  Are you suggesting that I use your stuff.
I tried but I needed STL map-like functionality.

> and generalise your list handling code into a template?

It's already on my todo list.  However, I wish that I never had to write
this kind of code.  Besides being a waste of time, there is no way that
I could ever come close to what the STL developers achieved.

> 10) Please capitalise the first letter of words in class names. Setup
> has been quite haphazard, I'm trying to be more consistent :}. Also, I
> prefer things like FreeList to free_list - I find them easier.

The above made me smile!  I completely agree with you.  However, I was
just trying to follow the GNU coding style.  Specifically, the following
item is relevant here:

    http://www.gnu.org/prep/standards_26.html#SEC26

> 11) All .h files should compile correctly if they are the only
> translation unit. I'm not sure if that is or is not the case for your
> files just now. Again, this is something I don't expect you to know, as
> I'm still introducing it into setup.

OK, I didn't know this.  How does one test this?  I just tried and g++
doesn't seem to like to compile *.h files alone.

> Hope thats enough feedback for ya!

Yes!  Thank you very much for your time.

Jason

Re: setup.exe rebase patch

[Robert Collins]

===
----- Original Message -----
From: "Jason Tishler" <jason@tishler.net>


> Rob,
>
> On Sun, Feb 24, 2002 at 10:45:32PM +1100, Robert Collins wrote:
> > ----- Original Message -----
> > From: "Jason Tishler" <jason@tishler.net>
> >
> > Some ideas follow:
> >
> > > Attached is a patch that adds the rebase functionality to
setup.exe.
> > > I would like to get some feedback before I start to resolve the
> > > following issues:
> > >
> > >     o How to handle a missing config file (i.e.,
/etc/setup/rebase.conf)?
> >
> > Create one with sensible defaults. If no sensible defaults exist,
add a
> > new tab.
>
> It is easy to arrive at one with sensible defaults.  What I was
"really"
> trying to ask above was the following:
>
>     o What should create this file?  A postinstall script, a new
rebase
>       package, etc?
>     o What should setup.exe do if the file is missing?  Proceed
without
>       rebasing (i.e., ignore missing rebase.conf), create a new
(default)
>       one, complain and exit, etc?
>
> I guess that I was trying to ask a more philosophical question of how
> coupled rebase was going to be with setup.exe.  It sounds like you are
> comfortable with a tight coupling.  Is this correct?

Yes. I think that setup.exe based rebasing should be optional, but
default to on. A flag in rebase.conf to control this would be good,
along with a dialogue box and a tick :]. Not needed for the initial
merge though.

> > >     o Need to handle in-use files.
> >
> I wanted to make sure that I wasn't off in "left
> field" before I started to dot I's and cross T's.

That's cool. I'd like to refactor what I did for install.cc into
io_stream::copy anyway, but thats a different story.

> > Does OpenLibrary fail or something nicely obvious like that?
>
> I haven't tried LoadLibrary(), etc. but I presume that one of the
> APIs can.  However, isn't this too little, too late?  What can we do
> when the corruption occurs?  Rollback to the copy in the archive?
But,
> then this image will *not* be rebased. :,(

I was thinking of the following process (for all dlls we attempt to
rebase):
0) Check that it hasn't been rebased already.
1) Copy
2) Rebase
3) Check for corruption - mark as occupying it's own prior space if
corrupt and then stop, otherwise mark it's new space in the used list.
4) Copy back over the original, with copy-on-reboot on failure.

> I recommend adding the requirement to setup.html that requires package
> maintainer to test whether or not their stripped relocatable images
> (e.g., DLLs) are rebasable.  If not, then they should release their
> packages with those files unstripped.

Ok.

> Does the above sound like a reasonable interim solution?

It doesn't solve the problem of locally created packages. In a nutshell,
while setup.html is a great guide, we cannot afford to break a system
that hasn't followed it (annoying results is ok, breaking is not).

> > >     o How should dependent DLLs be handled?
> > Recursively, as long as they are in the cygwin tree.
> Is this really necessary?

Yes. Well, I think it's prudent.

> Regardless of the resolution to the above, can we declare this a
second
> order item to be dealt with during subsequent iterations?

We can yes, it's certainly not urgent. My concept rough concept on the
process was that rebasing would be moved out of the install routines
(because uninstalls free up used list space) and be made to occur after
all the files have been installed, we'd have a complete list of .dll's
that need may rebasing, and whose dependents should be checked at the
same time.

> > > and some other more niggling items.
> > Can you list them?
> Here is my (dirty) laundry list so far:
>
>     Deal with in-use files.
>     Get config file prefix from registry.

cygpath will (should :}) retrieve that automatically. There are
equivalent cygwin-exported functions (Or do you rebase cygwin1.dll as
well :]]).

>     Warn on unrebases if base and/or size does not match used list
entry.
>     Refactor free_list and used_list to use common base class.
>     Refactor used_list::find() and used_list::remove().
>     Add dirty flag and write only if dirty.

Cool.

> I don't feel that any of them are shop stoppers.

Agreed. Missing the $1.5 Million to buy the property is a shop stopper
:}.

> > I've read through your patch. Can I ask you to redo it against HEAD?
> > (After updating cygpath etc to the String class.
>
> Yes, I guess that this is good news?

Yes.

> > Some feedback/thoughts on the current patch (not including what will
> > change during updating).
> >
> > 1) log.cc - don't clean up there. If you need to cleanup, have a
static
> > object whose destructor should get called.
>
> This is already done too (see rebaser.cc:338).  Note that my
stand-alone
> rebase.exe is already using this successfully.

Gotcha, ok the log.cc call should go then - or does this introduce sife
effects?

> > Like the initialisation of globals issue a while back, lets not
> > work around a compiler fault, but rather address the real issue.
>
> However, I thought that it was an artifact of calling ExitProcess()
> instead of exit().

Yah. However exit() shows the ame behaviour (I've tested).

> Hmm...  Where did you get the above name?  rebaser.cc:338 perchance?
:,)

Yup. I did read the code after all :}. I didn't edit my message all that
well though.

> > 2) I really don't like the rebaser::set_cygwin_root_dir construct:
why
> > not use get_root_dir () when you need the cygwin root? (Any why do
you
> > need it? Surely you only need cygpath (filename) ?
>
> This is for the stand-alone case which I wasn't sure was going to be
> able to easily share code with setup.exe.  This concern appears to
> be mitigated.  I can certainly have rebaser always retrieve the Cygwin
> root from the registry if this is deemed the better approach.

Ah. Well within setup io_stream is the answer as they

> > 4) New errors should go into the resource.rc stringtable. I'm guilty
of
> > this myself, but no need to let it get worse :}.
>
> I presume that you mean the *printf() messages?  I have added this to
my
> todo list.  However, will this be OK for the stand-alone version?
That
> is the stand-alone version a console app while setup.exe is a GUI one.

If we push back onto the String class, then yes it can. The functions
are not GUI dependent (AFAIK).

> > 5) If you've a complex .conf file, you might want to write a .y and
.l
> > combo for it instead of manually parsing.
>
> Sorry, I don't have any experience with YACC and lex.  But, I will
learn
> how to use them if it deemed necessary.
>
> Can this be handled as a second order item too?

Yes. It may not even be needed, but it can be useful.

> BTW, I have been using the silly MS .ini file format.  I am quite open
to
> suggestions regarding better formats.

Something like
keyword
category
    keyword
    keyword

would work. I.e.
version=1
used
    /bin/cygwin1.dll
        start 0x000334400
        finish 0x342200000

> > 6) config_file_reader has a static buffer (errk).
>
> Agreed that this is ugly -- in fact, this is the one area that I'm
least
> happy with.  Note I didn't want to get bogged down with parsing issues
> during the proof of concept phase but I have added this to my list
too.
> Since this is encapsulated I can easily change this implementation
> without any rippling effects.
>
> What I needed is push back (onto the input stream) functionality.  Any
> suggestions on a cleaner implementation is appreciated.

Why do you need push back capability? Use .peek() if you think you need
that. push back is deliberatly not supported due to the _slow_ behaviour
of remote URL's.

> > 7) I don't really grok your class hierarchy. Can you please explain
it?
>
> There is not much to it so I will attempt to explain it textually.  I
can
> send you a UML (i.e., Visio) diagram, if necessary.

That would work too.

> > (I don't want to criticize or suggest alterations until I do grok
it).
> > It *feels* procedural to me.
>
> Sniff, sniff -- you used the P word! :,(
>
> I tried to find the appropriate abstractions in the problem domain.
> I tried to encapsulate implementations behind reasonable interfaces to
> minimize the impact of implementations changes.  Etc, etc.  However,
> I'm open to design changes too, if deemed necessary.

What feels procedural is separate classes for serialisation. I _feel_
(And can there for be told to get my nose out of it) that a better
encapsulation would be for the used list and free list to have
[de]serialisation capability, and the config_file class to shrink to
simply finding the config file, identifying the beginning of (say) a
used list section, and then passing the buck. Would you care to comment
on this as opposed to your current model?

> > 9) Should we rename my 'list.h' to 'vector.h'
>
> vector does seem to better capture the semantics.  But, what is the
> relevance to my list code?  Are you suggesting that I use your stuff.
> I tried but I needed STL map-like functionality.

I'm suggesting I get my stuff out of your way :}. Your code seems
traditional list based, which mine isn't (anymore), so your code could
generalise to be a list/map template.

> > and generalise your list handling code into a template?
>
> It's already on my todo list.  However, I wish that I never had to
write
> this kind of code.  Besides being a waste of time, there is no way
that
> I could ever come close to what the STL developers achieved.

Agreed. I've already indicated that I'd like to use the real STL rather
than my half-way code. Unfortunately, we are dependent on a mingw
libc++, and I don't have the time to volunteer to be a maintainer. Also,
cinstall wouldn't compile-in-one hit without making the sources
build-in-one-tree smart enough to build two c++ libs during bootstrap.
See Chris and my rather unsatisfactory discussion on this. (I.e. no
resolution, status quo maintained).

> > 10) Please capitalise the first letter of words in class names.
Setup
> > has been quite haphazard, I'm trying to be more consistent :}. Also,
I
> > prefer things like FreeList to free_list - I find them easier.
>
> The above made me smile!  I completely agree with you.  However, I was
> just trying to follow the GNU coding style.  Specifically, the
following
> item is relevant here:
>
>     http://www.gnu.org/prep/standards_26.html#SEC26

Erk. Well I use VI :]. cinstall isn't a Gnu Project, and I've always
taken the standards as guidelines rather than godlines.

> > 11) All .h files should compile correctly if they are the only
> > translation unit. I'm not sure if that is or is not the case for
your
> > files just now. Again, this is something I don't expect you to know,
as
> > I'm still introducing it into setup.
>
> OK, I didn't know this.  How does one test this?  I just tried and g++
> doesn't seem to like to compile *.h files alone.

Usually what I do is test a given header with the following c++ file:
===
#include "header.h"
===

:}.

Rob

Re: setup.exe rebase patch

[Jason Tishler]

Rob,

On Tue, Feb 26, 2002 at 09:54:37AM +1100, Robert Collins wrote:
> ----- Original Message -----
> From: "Jason Tishler" <jason@tishler.net>
> > I guess that I was trying to ask a more philosophical question of how
> > coupled rebase was going to be with setup.exe.  It sounds like you are
> > comfortable with a tight coupling.  Is this correct?
> 
> Yes. I think that setup.exe based rebasing should be optional, but
> default to on. A flag in rebase.conf to control this would be good,
> along with a dialogue box and a tick :]. Not needed for the initial
> merge though.

OK, sounds good.  I'm not a GUI guy -- I hope that Gary will come to my
rescue. :,)

> > I haven't tried LoadLibrary(), etc. but I presume that one of the
> > APIs can.  However, isn't this too little, too late?  What can we do
> > when the corruption occurs?  Rollback to the copy in the archive?
> > But, then this image will *not* be rebased. :,(
> 
> I was thinking of the following process (for all dlls we attempt to
> rebase):
> 0) Check that it hasn't been rebased already.

What is the purpose of the above?  I already rebase DLLs into their
previous space during reinstalls, if they still fit.

> 1) Copy
> 2) Rebase
> 3) Check for corruption - mark as occupying it's own prior space if
> corrupt and then stop, otherwise mark it's new space in the used list.

What happens when the prior space is already taken?

> We can yes, it's certainly not urgent. My concept rough concept on the
> process was that rebasing would be moved out of the install routines
> (because uninstalls free up used list space) and be made to occur after
> all the files have been installed, we'd have a complete list of .dll's
> that need may rebasing, and whose dependents should be checked at the
> same time.

Is the "complete list" approach above required for the first iteration?

> (Or do you rebase cygwin1.dll as well :]]).

Yes, I can rebase cygwin1.dll too.  This is why I converted the
stand-alone rebase to a Mingw app.

> > > 1) log.cc - don't clean up there. If you need to cleanup, have a
> > > static object whose destructor should get called.
> >
> > This is already done too (see rebaser.cc:338).  Note that my
> > stand-alone rebase.exe is already using this successfully.
> 
> Gotcha, ok the log.cc call should go then - or does this introduce sife
> effects?

Currently, the config file won't be saved.  This won't be a problem once
the config file is saved as soon as the installation phase is finished.

> > > 2) I really don't like the rebaser::set_cygwin_root_dir construct:
> > > why not use get_root_dir () when you need the cygwin root? (Any
> > > why do you need it? Surely you only need cygpath (filename) ?
> >
> > This is for the stand-alone case which I wasn't sure was going to be
> > able to easily share code with setup.exe.  This concern appears to
> > be mitigated.  I can certainly have rebaser always retrieve the Cygwin
> > root from the registry if this is deemed the better approach.
> 
> Ah. Well within setup io_stream is the answer as they

Huh?  You didn't finish the above sentence.

> > I tried to find the appropriate abstractions in the problem domain.
> > I tried to encapsulate implementations behind reasonable interfaces to
> > minimize the impact of implementations changes.  Etc, etc.  However,
> > I'm open to design changes too, if deemed necessary.
> 
> What feels procedural is separate classes for serialisation. I _feel_
> (And can there for be told to get my nose out of it) that a better
> encapsulation would be for the used list and free list to have
> [de]serialisation capability, and the config_file class to shrink to
> simply finding the config file, identifying the beginning of (say) a
> used list section, and then passing the buck. Would you care to comment
> on this as opposed to your current model?

Ah, I see your point.  I guess that it depends on how you want to slice
and dice.  I have used the load/save paradigm many times before and in
general prefer it.  However, in this case I don't quite think of the
config file data as object state.  What happens if there are syntax
errors, etc.?  I think that a config file object should deal with these
kinds of issues (although is could delegate some of the responsibility to
the list objects).  Additionally, with my current approach I can change
the config file format without affecting the list classes.

Nevertheless, if you feel strongly, then I will change my perspective.

> > > 9) Should we rename my 'list.h' to 'vector.h'
> >
> > vector does seem to better capture the semantics.  But, what is the
> > relevance to my list code?  Are you suggesting that I use your stuff.
> > I tried but I needed STL map-like functionality.
> 
> I'm suggesting I get my stuff out of your way :}. Your code seems
> traditional list based, which mine isn't (anymore), so your code could
> generalise to be a list/map template.

OK, sounds good.

> > > 10) Please capitalise the first letter of words in class names.
> > > Setup has been quite haphazard, I'm trying to be more consistent :}.
> > > Also, I prefer things like FreeList to free_list - I find them easier.
> >
> > The above made me smile!  I completely agree with you.  However, I was
> > just trying to follow the GNU coding style.  Specifically, the
> > following item is relevant here:
> >
> >     http://www.gnu.org/prep/standards_26.html#SEC26
> 
> Erk. Well I use VI :]. cinstall isn't a Gnu Project, and I've always
> taken the standards as guidelines rather than godlines.

Since Cygwin follows the GNU coding style, I just assumed that setup.exe
did too.

Thanks,
Jason

Re: setup.exe rebase patch

[Christopher Faylor]

On Tue, Feb 26, 2002 at 04:26:53PM -0500, Jason Tishler wrote:
>>>>10) Please capitalise the first letter of words in class names.  Setup
>>>>has been quite haphazard, I'm trying to be more consistent :}.  Also, I
>>>>prefer things like FreeList to free_list - I find them easier.
>>>
>>>The above made me smile! I completely agree with you.  However, I was
>>>just trying to follow the GNU coding style.  Specifically, the
>>>following item is relevant here:
>>>
>>>http://www.gnu.org/prep/standards_26.html#SEC26
>>
>>Erk.  Well I use VI :].  cinstall isn't a Gnu Project, and I've always
>>taken the standards as guidelines rather than godlines.
>
>Since Cygwin follows the GNU coding style, I just assumed that
>setup.exe did too.

Well, I do consider cinstall a cygwin project so I would hope that we do
adhere to GNU coding style as much as possible.  I really don't want too
much variation.

However, since it's C++, I don't think that the coding standard really is
100% appropriate, so let Rob's Rules Prevail.

cgf

Re: setup.exe rebase patch

[Charles Wilson]

Jason Tishler wrote:


>>(Or do you rebase cygwin1.dll as well :]]).
>>
> 
> Yes, I can rebase cygwin1.dll too.  This is why I converted the
> stand-alone rebase to a Mingw app.


Urk.  If we follow Earnie's suggestion to include the output of 'objdump 
-S1' with cygwin1.dlll snapshots and the (main)cygwin package, then 
rebasing cygwin1.dll will break that.  Seems like rebasing cygwin1.dll 
itself should never be the default behavior...

http://cygwin.com/ml/cygwin-developers/2002-02/msg00029.html

--Chuck

Re: setup.exe rebase patch

[Christopher Faylor]

On Tue, Feb 26, 2002 at 09:49:01PM -0500, Charles Wilson wrote:
>Jason Tishler wrote:
>
>
>>>(Or do you rebase cygwin1.dll as well :]]).
>>>
>>
>>Yes, I can rebase cygwin1.dll too.  This is why I converted the
>>stand-alone rebase to a Mingw app.
>
>
>Urk.  If we follow Earnie's suggestion to include the output of 'objdump 
>-S1' with cygwin1.dlll snapshots and the (main)cygwin package, then 
>rebasing cygwin1.dll will break that.  Seems like rebasing cygwin1.dll 
>itself should never be the default behavior...
>
>http://cygwin.com/ml/cygwin-developers/2002-02/msg00029.html

I suppose that if we were really clever we could detect where cygwin was
offset to and readjust the symbol file accordingly as needed.

However, I agree that rebasing shouldn't be the default behavior.  In fact,
I wonder if I should make cygwin non-rebaseable.  It would load faster if
I did that.

cgf

Re: setup.exe rebase patch

[Jason Tishler]

On Tue, Feb 26, 2002 at 09:59:45PM -0500, Christopher Faylor wrote:
> However, I agree that rebasing shouldn't be the default behavior.  In fact,
> I wonder if I should make cygwin non-rebaseable.  It would load faster if
> I did that.

OK.  Then it would be best if you could make cygwin1.dll non-rebaseable.
Otherwise, rebase will have to maintain a blacklist of DLLs too.

I can easily leave a hole for cygwin1.dll (and other DLLs).  However,
the question then becomes how much space should be reserved?  Right now
0x120000 is "needed" -- unfortunately, in the future, more may be needed.
What happens then?

Adding rebase functionality has uncovered more issues than I initially
imagined.  Assistance determining the essential requirements would be
greatly appreciated.

Thanks,
Jason