parę sugestii dot. rpmmen

Sob, 27 Mar 1999, 21:06:28 CET

[sobota, 27 marzec 1999], Paweł Gajda napisał(a):

> 
> Montując insta przyjrzałem się nieco rpmmenowi, kilka 
> sugestii/postulatów/pomysłów skierowanych głównie 
> do Pawła Kołodzieja:
> 
> 
> 1. Uczytelnienie kodu 
>  - ident -kr (wcięcia, spacje wokół operatorów, itd) 

dobra, ale postuluje indent -kr -i 8 

>  - zastąpienie kodu w rodzaju(z funkcji preparetoinstall()):
>     rpmdepAddPackage(rpmdep,rpmmen_pkgs.packages[i].h,\
> 					  &(rpmmen_pkgs.packages[i]));
> 
>    czymś takim:
>    TPkgInfo *pkg = &rpmmen_pkgs.packages[i];
>    rpmdepAddPackage(rpmdep, pkg->h, pkg);
> 
>    Przy kilku tego typu linijkach jest to IMO czytelne tylko dla
>    autora. Znika też problem z tymi straszliwymi łamaniami ;-)  	
dobra

> 
> 2. ,,Ubibliotecznienie'' kodu
>  - myślę, że lepiej było by nie operować na statycznych zmiennych,
>    tzn:
>    zbiór funkcji operujących na TPkgSet i dopiero w osobnym
>    module zrobić statyczne zmienne. Przede wszystkim ułatwia to 
>    zrozumienie kodu => IMO ułatwia jego rozwijanie.           
nierozumiem, poprostu nie rozumiem powyższego zdania. Naprawdę staram się
zrozumieć, ale nie mogę... mógłbyś trochę przybliżyć (jakiś bardziej
konkretny przykład)

> 
>  - funkcje inicjujące set_src, set_dst itd udostępniłbym pod 1:
>    rpmmen_init(src, dest)   
Właściwie to set_src będzie w iontf, więc IMHO możę pozostać rpmmen_set_dest
> 
>  - rmpmen_() nazwa niewiele mówiąca => może rpmmen_close() ? 
dobra, to taka pozostałość po byłej pół-obietkowości
> 
> 
> 3. Inne 
> 
> - nie korzystaj z nagłówków pldilib wprost, znajduje się ona 
>   we wczesnej fazie rozwoju i nie jestem w stanie zapewnić stabilności 
>   ich nazw i zawartości. Po prostu: 
>   #include <pldi.h>
OK
> 
> - _wszystkie_ stałe w rodzaju  "coinst.rpmy", "install.stderr",
>   "basesystem" do config.h (co ja się naszukałem...)
dobra
>  
> - nie inicjuj iointf'a => będzie to robione na zewnątrz. 
>   Do testów powinno wylądować to w rpmmenExample.c. 
również nie ma problemu
> 
> - zauważyłem, że rpmen potrzebuje pkgsel i na odwrót, 
>   IMHO nie powinno to mieć miejsca.
tzn. rpmmenlib nie potrzebuje pkgssel'a, potrzebuje go jednynie rpmmenUI.c
który jest używany jedynie w rpmmenExample.c. nie bardzo mam pomysł, co z
tym zorbić, chyba jednym wyjsciem było by rozdzielenie rpmmenUI_run() na
rpmmenUI_run1() i rpmmenUI_run2() i potem:
rmpmenUI_run1();
pkgsel()
rpmmenUI_run2();

> 
>  
> Co zmieniłem:
> 
> - wyrzuciłem na razie czytanie listy pakietów z coinst, w obecnej 
>   fazie rozwoju PLD było by to tylko utrudnieniem(nieustalony skład
>   Base). 
> 
>   Zastaniawiam się, czy rzeczywiście warto w ogóle robić
>   taką listę. Co zyskujemy ? IMHO nie tak wiele, bo jedynie 
>   wcześniejszą weryfikację, czy tocfile odpowiada liście. 
>   Lista z kolei nie będzie raczej tworzona ręcznie, tylko
>   przy pomocy dumptocf(zmodyfikowanego). 

ta funkcja powstała raczej na użytek testów/jako przykład co trzeba zrobić,
aby zaznaczyć pakiet do instalacji. To taki mój wszesny "selektor pakietów"
i oczywiście nie musi być w wersji ostatecznej. Jednak i tak niektóre
pakiety powinny być pre-zaznaczone, a skoro tak, to dlaczego nie użyć do
tego tej funkcji ?
(nie wiem co maiłeś na myśłi pisząc o weryfikacji - ta funkcja nieczego nie
weryfikuje, ona tylko zaznacza które pakiety mają być zainstalowane)

>  
> - ponieważ inst korzysta też w innym miejscu z rpmliba(cpio.gz) to
>   przekierowanie stderr robię wcześniej, czy na stdout rpmlib 
>   też pisze ?   
tylko jeśli w skryptach pakietów są jakeś echa na stdout. W wersji 0.1.7
zarówno stderr jaki i stdout są "sprytnie" przekierowywane do pliku. Ale
jeśli stderr są przekierowywane już wcześniej to proponuję zrobić funkcję
redirect_out(int i); i jeśli i==1 to stdout jes przekierowywane do pliku, 
i==-1 stdout z powrotem na konsole, a i==0 to zamknięcie pliku do którego
było przelkierowywanie, coś jak redirect_out z rpmmenUI.c tylko że
przekierwowyane będzie jedynie stdout. Jeśli się zgadzasz to IMHO tak
funkcja powinna wlecieć do pldiliba.

> 
> - my_openf() jest już zbędne, dorobiłem iointf_chroot()  
miło
> 
> 
> Zauważyłem, że instalacja base wymaga dosyć sporo pamięci
> (zajętość skacze z 700 kB do prawie 3 MB), m.in. 
> dlatego, że cały tocfile jest wczytywany do ramu. 
> Dla base (100 pakietów) to jeszcze ujdzie, ale jak będzie ich 
> 500-600 ? Zastanawiam się, czy nie warto robić
> indeksu odczas czytania tocfile. Co o tym myślisz ?  

Nie bardzo rozumiem o co ci _dokładnie_ chodzi z tym indeksem. W każdym
razie, do np, dokładanie paietów z zależności potrzebne są wszystkie
nagłówki. Tak więc i tak muszą być wszystkie na raz w pamięci. Można się
zastanowić co można z nagłówków zapisywanych w tocfile wywalić (jakie pola),
ale narazie nic mi nie przychodzi do głowy. A i tak w czasie gdy jest 
uruchamiany rpmmen, to jest już po ustawianiu partycji, więc można by już
podmontować swap, ewentualnie zrobić plik ze swapem na partycji na którą
instalujemy.

> Zmieniłem dosyć mocno pldilib, na czym Ty głównie ,,ucierpisz''.
> Zaktualizowałem opis iointf'a, także nie powinno być ze zmianą 
> wielkich problemów, jeżeli by jednak, to albo pytaj, 
> albo ja machnę łatkę i Ci podeślę. 
myślę, że sobie poradzę

> Jestem też w stanie wziąć udział w robieniu wyżej postulowanych 
> poprawek, wymaga to jedynie Twojego ,,błogosławieństwa'' :-) Więc jak ? 
Nie wiem jak bardzo ci się spieszy. Jeśłi potrzebjesz tego na wczoraj, to
zrób, jeśli czwartek wieczorem to wystarczająco wcześnie to ja to zrobię.

PS. Zorbiłem sobie diff'a między tym co wylądowało w pldi a wersją
,,orginalną''. 99% to były zmiany w układzie kodu, więc nie byłem w stanie
odsiać zmian merytorycznych od stylistycznych :(

-- 
Paweł Kołodziej
pkollegu w ids.pl
http://www.ids.pl/~pkollegu  <- tu jest PePeSza (automat dla tłumaczy .pot'ow)