Mailing-List: contact cygwin-apps-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-apps-owner AT cygwin DOT com
Message-ID: <027701c1bd28$c4666170$0200a8c0@lifelesswks>
From: "Robert Collins" <robert DOT collins AT itdomain DOT com DOT au>
To: "Jason Tishler" <jason AT tishler DOT net>,
   "Cygwin-Apps" <cygwin-apps AT sources DOT redhat DOT com>
References: <20020207134119 DOT GB1804 AT dothill DOT com>
Subject: Re: setup.exe rebase patch
Date: Sun, 24 Feb 2002 22:45:32 +1100
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit


===
----- Original Message -----
From: "Jason Tishler" <jason AT tishler DOT 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