This is the mail archive of the
cygwin-apps@cygwin.com
mailing list for the Cygwin project.
Re: setup.exe rebase patch
===
----- 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