15 stycznia 2011

"Lustrzane odbicie" polecenia git add -p, czyli co mi pomogło po pewnym code review.

Skończyłem w pracy robić pewne duże zadanie. Zrobiłem commita i pusha do odpowiedniego brancha do naszego gitowego repozytorium . Potem przyszła pora na code review. Kolega z zespołu zauważył parę miejsc, w których należało by jeszcze coś dodać/zmienić/usunąć. Informacje o tych poprawkach wysłał mi na e-mail.

dostałem mniej więcej taką listę:

1. W przypadku dwóch statycznych pól klasy zastosowałeś przedrostek m_, zamiast s_. Dostosować do reszty.
2. Lepiej było by, aby funkcja Foo zwracała nie tylko informacje o błędzie, lecz najlepiej jeszcze określała typ błędu.
3. Niepotrzebne podwójne wywołanie funkcji inicjalizującej  w linii 250 i 253.
4. itd.
5. itd.
..
18. itd

Rzeczy do poprawki były dość proste, lecz było ich jednak dość dużo. Wprowadziłem połowę i stwierdziłem, że sprawdzę, czy przypadkiem niczego w między czasie nie popsułem. Kompilacja przebiegła ok, instalacja - ok, uruchomienie - ok, nagle crash.

No i teraz pytanie, która z moich zmian spowodowała błąd? Dlatego, że w ówczesnym czasie debugowanie w naszym projekcie było bardzo kłopotliwe a wręcz wówczas niemożliwe stwierdziłem, że najszybciej będzie usunąć zmiany i zacząć je wprowadzać po kolei, stopniowo, za każdym razem upewniając się, że wszystko jest w porządku.

Jednak tych zmian było na tyle dużo, w tylu miejscach, że stwierdziłem, że wolałbym uniknąć usuwania tych zmian ręcznie, bądź wprowadzania ich od nowa. No i wybrnąłem z tej sytuacji, gdyż całą procedurę wykonałem praktycznie nie dotykając kodu za pośrednictwem mojego IDE. W tym wszystkim pomógł mi oczywiście GIT :)



Żeby całość lepiej zobrazować, pozwolę się posłużyć przykładem innego pliku, duuużo mniejszego ;)


Jak widzimy na powyższym screenie, polecenie ls mówi nam, że w katalogu znajduje się tylko plik. Za pomocą polecenia cat plik, wyświetlamy jego zawartość (wiedza na temat tego co jest w nim zawarte przyda się na później). Polecenie git status, w tym przypadku mówi, że katalog roboczy jest czysty pod względem zmian, tzn. nic nie jest zaindeksowane, a git śledzi wszystkie pliki znajdujące się w katalogu.

Załóżmy teraz, że zmiany jakie wprowadzamy po naszym review wyglądają tak:


albo przedstawiając innym diff-toolem:



Załóżmy, że któraż z tych czterech zmian powoduje coś złego. W jaki sposób wprowadzać je po kolei by kolejno sprawdzać która?

Na początek zróbmy commit owego pliku lokalnie do repozytorium (spokojnie, git może modyfikować historie i my tutaj z tego będziemy namiętnie korzystać ;) )



a zaraz po tym zróbmy revert owej zmiany bez commitowania jej


W kwestii przypomnienia, git revert nie usuwa poprzednich zmian, lecz nakłada różnicę, tak aby dwa ostatnie commity łącznie "dawały zero". Polecenie git revert domyślnie od razu robi commit owego odwrócenia zmian, chyba że tak jak w tym przypadku podkreślimy by tego nie robiło. Wówczas jednak owa zmiana jest od razu dodawana do indeksu. My jednak i tego nie chcemy, więc usuwamy go ręcznie za pomocą polecenia git reset HEAD plik.

No i teraz generalnie zaczyna się zabawa...

W tym momencie lokalnie na dysku mamy pliki w "dobrym stanie". "Złe zmiany" są wcommitowane i są obecnie na HEADzie, jednak są przykryte revertem, więc wszystko może działać. Możemy zrobić builda i wszystko powinno działać. Jak teraz szybko i bezboleśnie wprowadzić jedną z owych zmian, by móc sprawdzić jak ona wpływa na projekt?

Polecenie git checkout -- nazwa_pliku usuwa zmiany (a tak naprawdę robi checkout owego pliku z HEADa). To samo polecenie z parametrem -p robi to samo tylko na częściach pliku wybieranych w interaktywny sposób. Skup się teraz i zastanów się co dzięki temu osiągniemy...

Będziemy po kolei, w częściach usuwać zmiany z lokalnie zmodyfikowanego pliku, a dlatego że owe zmiany są revertem, to tak naprawdę będziemy dodawać je względem stanu początkowego. Zgadza się? :)

W praktyce wygląda to tak:


Git stara się podsuwać nam owe zmiany w częściach. Na danym kawałku zmiany możemy wykonać wiele akcji, które są symbolizowane skrótami [y,n,q,a,d,/,s,e,?]. Wystarczy wpisać ? lub nacisnąć od razu enter, by pojawił nam się opis owych skrótów.



Jeżeli dany kawałek jest za duży, by o nim w całości od razu zdecydować, powinniśmy go rozdzielić, co możemy zrobić za pomocą splita. Wpisując s dostajemy już tylko kawałek, w tym przypadku pierwszą linie. 



By dodać kawałek (usunąć kawałek reverta), posługujemy się skrótem y, a następnie wychodzimy z trybu interaktywnego za pomocą q (gdyż mamy przecież po każdym wprowadzeniu małej zmiany sprawdzać czy wszystko jest ok).

Zatem w tym momencie puszczamy builda, wgrywamy, uruchamiamy, testujemy. Jeżeli wszystko gra, to znowu odpalamy git checkout -p -- plik, (splitujemy jeżeli trzeba), za pomocą y robimy discard na danym kawałku i znowu robimy builda itd. I tak aż do momentu aż znajdziemy zmianę, która była winna za początkowy crash.

W tym momencie należy już ręcznie poprawić kod, usunąć crasha i zrobić w końcu commita, który zapisze całą naszą zabawę. Jednak, aby to wszystko nie wyglądało dziwnie w historii, powinniśmy połączyć bieżące zmiany z poprzednim commitem (modyfikując w ten sposób lokalna historię). A to wszystko jest bardzo proste...



Całość może wygląda troszkę skomplikowanie, lecz w niektórych przypadkach stanowić będzie wg mnie algorytm o najmniejszym stopniu prawdopodobieństwa zepsucia czegoś jeszcze bardziej, podczas ręcznego usuwania zmian.
Related Posts Plugin for WordPress, Blogger...

9 komentarzy:

  1. Tak pro forma dodam, że właściwym odpowiednikiem "git add -p" jest "git reset -p" (dostępny od wersji 1.6.5). Jest też "git stash -p".

    Twoje podejście jest trochę dziwnie pokręcone. Po co revert (dopiero co zrobionego commita), po co checkouty?

    W ogóle dobrą praktyką są w miarę częste inkrementacyjne commity z pracy, najlepiej gdy są kompilowalne (wiadomo, czasem bywa to trudne, ale warto). W każdym razie wtedy łatwiej spojrzeć w przeszłość, a jak wszystko gra, to squashujemy wedle potrzeb przed pushem. No ale różnie bywa i czasem na bieżąco commitów się nie robi. Ale właśnie "git add -p" nas potrafi (przynajmniej cześciowo) uratować i możemy zmiany commitować krokowo (oczywiście z głową). Po każdym commicie stashujemy i sprawdzamy czy nam działa (lub sprawdzamy przed commitem robiąc stash -k by zachować indeks). Potem "git stash pop" (z ewentualnym rozwiązaniem konfliktu, gdy zmienialiśmy coś -- trzeba pamiętać, że przy -k część już stage'owana też jest stashowana) i w kółko aż do naprawy wszystkich potknięć i pełnego zadowolenia.

    Jeżeli łatwiej jest ujmować zmian, no to wrzucamy cały plik do indeksu i lecimy analogicznie z "git reset -p".

    OdpowiedzUsuń
  2. "Skończyłem w pracy robić pewnego dużego taska. Zrobiłem commita i pusha do odpowiedniego brancha do naszego gitowego repozytorium . Potem przyszła pora na code review."

    Większość z użytych słów naprawdę ma polskie odpowiedniki.

    OdpowiedzUsuń
  3. @Anonimowy:

    Masz na pewno rację w tym, że stężenie angielskich słów spolszczanych na siłę w tym zdaniu jest co najmniej duże.

    Powiem jednak szczerze, że nie pokusiłbym się na tłumaczenie słów "commit", "push", "branch". O ile ostatnie z tych słów możnaby spokojnie tłumaczyć na "gałąź", to jednak powiem szczerze... nie spotykam się na co dzień, by ktoś mówiąc o gicie (albo w ogóle o jakimkolwiek systemie kontroli wersji), mówił że "wysłał pliki na gałąź".

    Co do słów "commit" i "push", to mogę powiedzieć, że gdybym pisał o np. SVN, to posługując się wyrażeniem "wysłałem do repozytorium" czytelnik (przynajmniej ten bardziej zorientowany) miałby pewność, co zrobiłem.

    Naprawdę nie wiem jakich odpowiedników słów "commit" i "push" używać, by czytelnik po przetłumaczeniu ich nie mylił tych dwóch pojęć.

    Jednak zwróciłeś uwagę na istotny fakt. Podejrzewam, że nie tylko ja zmagam się z sytuacjami, w których to próbuję opisywać po polsku narzędzia/technologie, do której brak stosownych polskich materiałów referencyjnych. Postaram się zwrócić większą uwagę, by "nie przeginać" w żadną stronę.

    Tak sobie myślę... "a może to jest znak, że taki tekst powinien tutaj w całości powstać po angielsku"?

    OdpowiedzUsuń
  4. @Przemoc:

    Przez chwilę miałem nadzieję, że to co zaproponowałeś faktycznie będzie lepszym rozwiązaniem i pewnie by tak było, gdyby istniało także polecenie git stash apply -p, jednak taki przełącznik w tym kontekście nie istnieje.

    Co do polecenia git reset -p, faktycznie to bardziej jemu należy się miano "lustrzanego odbicia", więc chyba bedę musiał się zastanowić nad zmianą tytułu posta ;)

    Generalnie tym wszystkim chciałem osiągnąć efekt, by mieć możliwość posiadania takiego miejsca do którego moglibyśmy po kawałku dodawać odrobinę kodu i sprawdzać, czy wszystko jest ok. Chciałem by tym miejscem był mój katalog roboczy i stąd te wszystkie moje wygibasy. Dość łatwo jest osiągnąć efekt, w którym takim miejscem było by jakieś inne repozytorium. Git jest gitem, dlatego owe repozytorium mogło by być nawet na tym samym komputerze, wówczas używanie add -p, oraz commitowanie po kawałku załatwiło by sprawę. Jednakże chciałem osiągnąć wszystko użyciem jednego repo. Tak jak zauważyłeś, istnieje możliwość stashowania częściowego, dzięki temu możemy zostawiać w katalogu roboczym to co chcemy testować, a wszystko innego trzymać na stashu.

    To rozwiązanie mnie osobiście jednak nie pasuje ze względów... wydajnościowych.

    Git stash (przynajmniej w moim środowisku) jest poleceniem dość wolnym (średni czas jego wywołania u mnie to 30-40s (wydaje mi się, że czas zależy chyba głównie od wielkości plików, choć nie sprawdzałem czy przypadkiem (w co jednak wątpie) od rozmiaru repozytorium. W każdym razie git gc --aggresive nie pomaga). Ponadto należy wspomnieć, że pomimo występowania przełącznika -p (--patch), git stash nie przyjmuje jako argumentu nazwy pliku (da się to jakoś obejść?), a to generalnie sprawia, że jeżeli mamy dużo zmian w katalogu roboczym, git stash -p, najpierw zacznie analizować te wszystkie zmiany a potem nam te wszystkie zmiany podsuwać. Pół biedy jeżeli nasz plik zostanie nam zaproponowany w trybie interaktywnym jako pierwszy ;) (choć, gdybyś powiedział, że mając gita nie należy dopuszczać sytuacji, w których to mamy za dużo niezacomitowanych zmian, to generalnie przyznałbym Ci rację, jednak czasami istnieją sytuacje w których czegoś takiego nie da się uniknąć, np. aplikowanie dużego patcha, który o zgrozo nie chce się za cholerę kompilować i nie wiadomo dlaczego).

    Generalnie więc uważam, że co najmniej istnieją przypadki, które mogą przemawiać za sposobem przedstawionym przeze mnie.

    Tak czy inaczej, warto zauważyć jedno... w innych systemach kontroli wersji już pewnie dawno by człowiek utknął z podobnym problemem... a tutaj rozwiązań jest nawet kilka ;)


    Bardzo dziękuję za komentarz, stanowi całkiem niezłe uzupełnienie. Może później wyedytuję całą notkę wspominając o tym o czym pisałeś.

    OdpowiedzUsuń
  5. Kolejno postaram się odnieść do poruszanych przez Ciebie problemów.

    0. Sam jestem "bojownikiem" o używanie polskiej terminologii, szczególnie w mediach reprezentatywnych, że się tak wyrażę. Za co zresztą bywałem karcony na OSnews.pl w przypadku niusów o nowych wersjach jądra. Niemniej blog to takie publiczne dzielenie się jakimiś skrawkami informacji, nie muszą być w pełni rzetelne (choć lepiej gdy są), nie muszą być w pełni profesjonalnie zredagowane (choć lepiej gdy są), etc., dlatego blogów (bez obrazy) nie uznaję raczej za media "reprezentatywne" (co nie znaczy, że są bezwartościowe). Ludzie z branży w rozmowach między sobą używają oryginalnych terminów, bo te są z reguły krótsze i nie pozostawiają wątpliwości co do znaczenia (choć bywają wyjątki), polonizując je jednak zazwyczaj przynajmniej w zakresie fleksyjnym. Stąd z formalnego punktu widzenia potworki językowe typu: zmerge'owałeś już? chwila, stashuję, leci pull, przejrzę, poprawie jak trza, commitnę i zapushuję zaraz, itd. funkcjonują w naszym języku i nie widzę powodu by się o nie czepiać na blogach.

    1. Faktycznie, git apply nie obsługuje przełącznika -p. AFAIK na LM gita przewijał się stosowny patch (czy patchset) wprowadzający to, ale nie pamiętam jaka tam dyskusja była. W każdym razie stash zaczyna być używany w nieco innych celach niż pierwotnie planowano go wykorzystywać i stąd być może pewien opór z brnięciem z tym dalej. Tak sobie tylko koncypuję. Niuanse taki jak ten z działaniem -k czy ten, o którym dalej wspomnę, wynikają z celu jakim są: 2 commity na HEADzie i czyszczenia katalogu roboczego oraz indeksu (chyba że damy właśnie -k).

    2. Git naturalnie ma pewne oczekiwanie wobec środowiska, w którym pracuje. Szybkie wyciąganie statusu plików (lstat() w *nixach) to jedno z istotnych i raczej powszechnie znane, ale wciąż warto o nim wspomnieć.

    3. Nie ma powodu angażować tutaj kolejnych repozytoriów. Mamy w końcu repozytorium lokalne i całą gamę możliwości operowania na nim. Ja nie mówię (piszę), że Twoje rozwiązanie problemu było złe, raczej nieco nietypowe. "git stash -p" podałem jako ciekawostkę, sam z niego nie korzystam, ale z "git add -p" owszem a nawet namiętnie (nawet płakałem już parę razy, że nie spotkałem jeszcze dobrego GUI z tą funkcją, co byłoby pomocne w trudnych przypadkach) i to za jego użyciem (lub resetu) optowałem. Jeżeli nie chciałeś przy okazji odtwarzać historii, to innym rozwiązaniem dla jazdy w przód byłby jednorazowy stash i "git checkout -p stash@{0} -- file" stosowany tak jak sam proponowałeś. Dla jazdy w tył w istocie dochodzimy do "tymczasowego" zacommitowania całości pracy i zabaw z "git checkout -p HEAD^ -- file" w celu ujmowania zmian. Te skróty bez wielu commitów są fajne na szybko, ale tracimy okazje do udokumentowania, co właściwie powinniśmy byli zrobić wcześniej.

    3'. BTW Dobrą praktyką są topic-branche, także do refaktoryzacji i wszelkiej maści reworków, które zgodnie z prawami Murphy'ego nie muszę (wbrew pozorom?) działać tak samo/równie dobrze jak przed zmianami. A jeżeli nasze sumienie dopuszcza rebranchowanie (trzeba być realistami, ludzie lubią "liniowość"), to wciąż jestem zwolennikiem merge'owania bez przewijki (--no-ff), bo taki merge'owy commit (choć z pozoru zbędny, bo nie rozwiązujący żadnych konfliktów) pozwala nam ładnie wydzielić cały patchset. W przypadku pojedynczega patcha oczywiście nie ma to sensu.

    4. Stashowanie to w zasadzie commitowanie i od zwykłego commitowania nie powinno być szybsze, ale wolniejsze być może. Odstashowywanie też swoje zajmuje, szczególnie jak uwzględnimy opcję --index, którą nowi w świecie gita ze zdumieniem odkrywają, gdy pop/apply niszczy im misternie wyselekcjonowane hunki.

    OdpowiedzUsuń
  6. 4'. Warto przy okazji poruszyć, że status też może kuleć, bo to taki commit bez commitowania (i jeszcze niedawno dostłownie tak było). Dlatego w prostych przypadkach lepszą alternatywą jest mój zamiennik (choć pewnie inni też na to powpadali), który nazywam quick-status. Dodaj sobie do .gitconfig w sekcji [alias] linijkę:

    qs = "!f() { echo staged:; git diff --cached --name-status; echo unstaged:; git diff --name-status; }; f"

    Wówczas "git qs" da Ci wyjście podobne do "git status -s", jednakże bez kolorów ale i commitowego narzutu.

    4''. Posiadając pewną wiedzę a priori, a gdy pracujemy na naszym katalogu roboczym powinno tak być, możemy przyspieszyć gitowe operacje oznaczając wybrane pliki by były olewane. Służy do tego "git update-index --assume-unchanged files..." (do odwracania służy opcja --no-assume-unchanged). Jeżeli chcesz sprawdzić co jest oznaczone, wystarczy użyć "git ls-files -v" i popatrzeć gdzie są małe literki.
    Teraz znowu UWAGA na stash. Jeżeli masz pliki oznaczone jako do olewki, a tak naprawdę są w nich jakieś zmiany, to wraz ze stashem stracisz je. Paru się już na tym przejechało (ja na szczęście nie), więc lepiej być tego świadomym od samego początku. Takie zachowanie jest tu w zasadzie naturalne z punktu widzenia semantyki stasha, niemniej powinno być domyślnie ostrzeżenie (konfigurowalne poprzez konfigurację), które można by obchodzić z użyciem jakiejś opcji, np. --discard-changes-in-files-assumed-as-unchanged. ;)

    5. Duże patche... Ich po prostu nie powinno być. Zmiany należy dzielić na kompilowalne kroki. Czasem wymaga to dodatkowej pracy, która niewprawnym wydaje się zbędna, ale naprawdę warto, bo łatwiej analizować mniejsze zmiany (co za truizm) w przypadku jakichkolwiek problemów, a te statystycznie... wiadomo.

    6. Twój sposób zadziałał, więc bez wątpienia nadaje się do ponownego użycia. Z tym pokręceniem to chyba trochę się pośpieszyłem, za co przepraszam, bo mogło być to źle odebrane (mimo braku jakichkolwiek intencji deprecjonowania Twojego rozwiązania). W końcu nie znam dogłębnie specyfiki Twojego repozytorium, co tam dokładnie jest i jak z tym pracujesz/pracujecie.

    7. Rozproszoności git nie ma na wyłączność, ale index chyba tak, a jeżeli nie, to bez wątpienia jest to rzadkość wsród konkurencyjnych DVCS-ów. Parafrazując znane powiedzenie: index robi różnicę.

    X. Zapytam jeszcze tylko ogólnie, czy korzystasz z gita może pod Windowsem? Ja co prawda nie, ale mimo nieustannych ulepszeń, chyba wciąż nie jest to najlepszy system do pracy z nim.

    Kurde, trochę popłynąłem (musiałem rozbić na 2, bo nie mieściłem się w 4096 znakach, a i tak dostałem request-uri too large w 1., niemniej opublikował się, dziwne) i spokojnie z tych komentarzy mógłbym zrobić git-ową notkę u siebie. Może jeszcze to kiedyś zrobię. :)

    OdpowiedzUsuń
  7. Zapomniałem odnieść się do jeszcze jednego!

    "git gc --aggressive". Nigdy tego nie rób, chyba że jednorazowo po jakimś imporcie z innego repo jak svn lub podobnej sytuacji. W ten sposób niszczysz wszystkie delty, które były dotychczas ustalone w repozytorium i odtwarzasz wszystko od nowa, co w 99,999% przypadków sensu nie ma najmniejszego, a w dużym repozytorium zajmuje, za przeproszeniem, w cholerę czasu. To czego tak naprawdę chcesz i powinieneś używać (i tak raczej rzadko, bo gc rutynowo robi paczki, ale istotnie jedna większa może przyspieszyć i zmniejszyć repo) to:

    git repack -A -d --window=100

    Okno można sobie jeszcze zwiększać, ale czas rośnie niewspółmiernie do uzyskiwanych rezultatów zazwyczaj. Zwiększanie opcji --depth (z domyślnego 50) w praktyce ma naprawdę minimalny wpływ na końcowy efekt, więc lepiej ją sobie darować.

    Trochę więcej na ten temat od samego Linusa:
    http://thread.gmane.org/gmane.comp.gcc.devel/94565/focus=94613

    OdpowiedzUsuń
  8. Powiem szczerze, że mieć takie komentarze pod postem to sama przyjemność :) Dziękuję za wszelkie uwagi.

    Co do rzeczy o których wspomniałeś, postaram się w wolnej chwili je przeanalizować. Przyznam jednak szczerze, że o większości z nich nawet nie słyszałem, tym bardziej się cieszę z owych informacji :)

    OdpowiedzUsuń
  9. Skoro już tutaj pociągnąłem temat (a wciąż nie mam własnej notki...), to dodam jeszcze małe addendum. Mail Torvaldsa w wątku o jądrze 2.6.38-rc1 nt. gita, fetch/pull i paczek, który IMO warto przeczytać. W myśl zasady: nawet jeżeli Ty to wiesz, to być może niektórzy Twoi czytelnicy (jeszcze) nie.

    http://thread.gmane.org/gmane.linux.kernel/1088915/focus=1088997

    OdpowiedzUsuń