Re: apt

[Alfredo Kengi Kojima <kojima@conectiva.com.br>]

On Thu, 31 Aug 2000, Jason Gunthorpe wrote:

> > The patch is probably not very readable, but I've put the
> > code in cvs, so you can take a look from there. As I've said,
> 
> The patch, is useless, you must have passed the wrong options to diff or
> something, it contained two copies of every files.. Anyhow, I'm looking at
> your CVS.
>

Many of the files have probably gone through some reformating,
as I use an auto-indenting editor. I promise to make a pretty
patch that will suit your coding style tastes, when I send you
the definitive diff :)

> > the code still needs cleaning and it's under heavy development,
> > so I don't expect you to accept it for merging into your tree
> 
> Well, lets see here, I'm just going to go over what you have and rattle
> off what I see here..
> 
> * Root Dir:
>   Dump the RPM specific stuff into an RPM toplevel dir. I like small
>   directories.

That's part of what needs to be cleaned up. There are also
some files in the cvs that aren't supposed to be there.

> * Build Dir:
>    -CPPFLAGS+= -I$(INCLUDE) -I/usr/include/rpm
>    +CPPFLAGS+= -I$(INCLUDE)
>  Eek! nononono, use #include <rpm/foo.h>, do not do that. Especially 
>  do not do that in the file where you have done that. 
> 

I can't do that because the rpm header files do #include "foo.h"
That's rpmlib's fault, I hate it.

> -ONLYSTATICLIBS = yes
> -#ifeq ($(HOST_OS),linux-gnu)
> 
>  Why? You do know to use the shared libaries you just use "export
>  LD_LIBRARY_PATH=`pwd`" in the build dir.. Works with gdb, etc, etc.
> 

Because I can't ship a dynamically linked application that
will require many libraries that are probably not installed
(or worse, are in the wrong version) in the machine of many 
of our potential users. 
But of course, that's another thing that needs cleaning
before a public release.

> * Methods:
> -RPMLIBS=-lrpm -lpopt -static -lz -ldb1 -lbz2
>  
>  Bleck. shlibs, there for a reason. I had a better way to solve that
>  general problem, but it escapes me now.
>

Yeah, I suppose so, but that's temporary too...

> * Tools:
>   + Needs to use proper make files

Yes.

>   + Is not C++

Why is that a problem?

>   + Much of it appears to belong in test/ 
> 

Most of them will disappear. Only genpkglist.c will be kept.

> * Cmdline:
>   + I think your changes to apt-cache may be too extensive. I need to look 
>     closer but it seems the majority of them need to be pushed into the
>     rpm library code, particuarly the format-as-text bit.
> 
> -   if (1) {
> -      ConectivaFactory *factory = new ConectivaFactory;
> -   } else {
> -      DebianFactory *factory = new DebianFactory;
> -   }
> 
>   + Heh, that's just bad style :>
> 

Oh, really? :P 
As I have already said, the code is full of
temporary things that I put there to allow me to get a working
prototype faster, without worrying about minor details. That will be 
eventually replaced by some check, run-time or compile time
that will determine which of the factory classes will be used.

>   Otherwise looks mostly OK, I have some more specific comments below..
> 
> * apt-pkg:
> -   if (0) {//akk
> -      StoreFilename = QuoteString(Version.ParentPkg().Name(),"_:") + '_' +
> -       QuoteString(Version.VerStr(),"_:") + '_' +
> -       QuoteString(Version.Arch(),"_:.") + ".deb";
> -   } else {
> -      StoreFilename = QuoteString(Version.ParentPkg().Name(),"_:") + '-' +
> -       QuoteString(Version.VerStr(),"_:") + '.' +
> -       QuoteString(Version.Arch(),"_:.") + ".rpm";
> -   }
>   Yuk, change this to grab the extension of the original file and
>   preserve it instead of *that*. I'll accept a mini patch to do that
>   immediately.

Ok.

>   + Your changes to cachefile I do not like. The locking mechanism needs
>     to be abstracted properly into '_distro'.

Yes, it will (obviously) be.

>   + (init.cc) Yuk, put an Init method in _distro and call the right one. 
>     the Init function should create the proper _distro for the system the
>     library was compiled for, overridable with a configuration setting.

Yes, of course.

>   + pkgcachegen.cc has become badly misindented..

I'll clean it up.

>   + sourcelist sux in many horrible aweful ways, it needs to be redone
>     somehow.. if you have some brainstorm, share :>

Well, the obvious way is to abstract it and subclass it in the
canonical way (ie: instantiate through the factory class). I
was going to do that once the other, more important stuff is stable.

>   + distrofactory is mostly OK, see comment below
> 
> **
> 
> General comments..
> 
> 'distro' is not a name I like. I would prefer a name like environment, or
> system, or something like that.
> 

Ok, _system is fine for me.

> putting dep checking and version checking in 'distro' is something I
> *hate*. I think it is much more appropriate to create a new field in the

Yes, I know. But that's yet another temporary kluge. 
Btw, if you see something horrible, you can assume it's temporary.
I think I don't code THAT badly :)

> cache header that indicates the version string style used within that
> cache and then provide multiple version comparision functions that are
> invoked based on that number. If you create a tidy patch to do this I will
> apply it immediately to CVS APT.
> 

Ok, good idea. I will do that when I get the time.

> Generally, most of your patches to apt are correct. The only reason you
> don't already see something like a '_distro' is because I did not know
> what would be needed to abstract :>
>

Sure ;)
 
> Basically, if you make me a nice patch that does the _distro thing with a
> nicer name and makes deb use it then I will apply it and maintain it for
> you. If you want, I would even build something based on what you have now,
> but you'd have to wait a good while :|
>

That's good, thank you. I think the delay is acceptable by me,
if a temporary fork is acceptable by you. :)
 
> The stuff in the RPM directory.. I am willing to consider including it -
> however I will not do so until I can personally test it in detail. [I
> haven't even looked at it yet] If you guys have an english version of your
> distribution available on the internet that should not be a problem at
> all.

The stuff in the RPM directory is still under heavy testing
and development, so it will change a lot. You will probably
get lots of trouble in the initial run. Some of them will
be legitimate dependency problems, due to a few glitches
during isntallation. Some will be due to possible packaging
problems and some due to possible bugs in apt. But we're working
to get these probs fixed.

Anyway, Conectiva 5.1 is available through ftp at:
ftp://ftp.conectiva.com.br/pub/conectiva/5.1

> 
> Please base your work off CVS apt, the 'aliencode' branch which will
> become APT 0.5.0 eventually, that is where I will apply your patches.
>

Ok. Are there any significant changes since apt-0.3.19 to current cvs?

> CVS is at :pserver:anonymous@cvs.debian.org:/cvs/deity module 'apt'
> 
> I'm terribly excited, your rpm directory is much smaller than I had
> anticipated an RPM verison would be - and I'm surprised it is workable at
> all, though I see you haven't fully delt with the filedepends issue..
> 

The file dependencies are fully handled in a rather simple way.
The code just converts the file dependencies into normal dependencies,
by going through the list of packages twice. One to make a list
of files in the Depends tag of each package and another to
put each of these files in the Provides tag of each package
that contains those files.

The pkglist for us will be a lot larger than for debian,
since it includes the file list for each package, but I consider
that to be acceptable.

> I would appreciate a little file describing the key differences between
> RPM and DEB from this perspective.
> 

Not many, as you can see. In the pov of apt, file depencies
are nothing but a dependency that starts with a / character.
We just needed to search who provides these files by hand
and put them in the provides list of the appropriate packages.
Once that's done everything is 100% transparent to apt.


Since all distribution specific code is being abstracted
and totally separated from the main code, it will be possible
to select which distribution apt will support at run time,
by doing some simple checks to see if you run Debian,
Conectiva or whatever. That's the elegant solution, imho.
But it is equally trivial to put one or two #ifdefs to
allow the distribution type to be made compile-time selectable,
with the bonus of a slightly smaller binary.
Which option do you think is better?

Regards,

Alfredo

> Thanks,
> Jason
>