Re: Git bisect does not find commit introducing the bug
> Anyway, I don't think it is feasible to weaken the assumption that there > is only one transition from 'old' ('good') to 'new' ('bad'); this is > what allows O(log(N)) operations. See bisection method of root finding, > that is finding zeros of a continuous function. I don't intended to change default behaviour of git bisect, I can estimate what drawback it could bring. I'd rather implement something like git bisect start --bisect-strategy=missing-feature by default current state is being used. git config value would be also useful. Oleg
Re: Git bisect does not find commit introducing the bug
>>> Then you must adjust your definition of "good": All commits that do not have >>> the feature, yet, are "good": since they do not have the feature in the >>> first place, they cannot have the breakage that you found in the feature. >>> >>> That is exactly the situation in your original example! But you constructed >>> the condition of goodness in such a simplistic way (depending on the >>> presence of a string), that it was impossible to distinguish between "does >>> not have the feature at all" and "has the feature, but it is broken". >> >> Johannes, thank you for correctly identifying the error in my logic. >> Indeed I was using the term 'bad' also for the commit without the >> feature. In order to find the commit introducing the bug in my example >> a new state is needed, which would make 'git bisect' a bit more >> complicated than the user 'most of the time' probably needs. Or do you >> think, it would make sense to ask the user for this state (if e.g 'git >> bisect' would be started with a new parameter)? > If a commit doesn't have the feature, then it is by definition, not > containing a broken feature, and you can simply use the "good" state. > There is no need for a different state. If you can't test the commit > because it's broken in some other way, you can use "git bisect skip" > but that isn't what you want in this case. Commits missing feature == 'good' commit is a very confusing one. Looks like in real life it happens much often, then git developers can imagine. For multi-branch/multi-feature workflow it's pretty easy not to recognize whether it is missing or not developed yet, especially on retrospective view where cherry-picking/squashing/merging is being used. My experience shows most annoying bugs are generating after a heavy merge (evil merge) with conflicts resolutions, where developer is not involved in the knowing what happens on counterpart changes. Then feature can be disappeared after it was worked & tested in its own branches. @Alex, I'm pretty interesting in fixing this weird bisect behaviour as well, as far as I struggled on it last summer and continue struggling so far :) If you want we can join to your efforts on fixing. Cheers, Oleg
Developing git with IDE
Hi *, being last decade working with java & javascript I completely lost relation to c/c++ world. Trying to get into git internals I'm facing with issue what IDE is more suitable for developing git @ macos ? Have googled, but any my search queries following to non-relevant themes, like supporting git in IDEs etc. my first attempt - CLion (as far as I'm Jetbrains fan) - got failed, as far as doesn't support makefile-based projects, only CMake. There are a number of free C/C++ dev tools: Xcode, CodeBlocks, CodeLite. Gnat, Qt creator, Dev C++, C++ Builder (Borland? :), Eclipse, NetBeans... what else? Because of lack my modern C experience, could somebody share his own attempts/thoughts/use cases how more convenient dive into the git development process on the Mac? Tried to find in the git distribution Documentation more information about this, nothing as well... Would be nice to have a kind of 'Getting Started Manual' Thanks for your time, Oleg Taranenko
Re: [PATCH v3] checkout: eliminate unnecessary merge for trivial checkout
Sorry for bothering, why not introduce a brand new option like git checkout -b foo --skip-worktree-merge for such rare optimization use case? On Wed, Sep 14, 2016 at 12:34 AM, Junio C Hamanowrote: > Ben Peart writes: > >> +static int needs_working_tree_merge(const struct checkout_opts *opts, >> + const struct branch_info *old, >> + const struct branch_info *new) >> +{ >> +... >> +} > > I do not think I need to repeat the same remarks on the conditions > in this helper, which hasn't changed since v2. Many "comments" in > the code do not explain why skipping is justified, or what they > claim to check looks to me just plain wrong. > > For example, there is > >/* > * If we're not creating a new branch, by definition we're changing > * the existing one so need to do the merge > */ >if (!opts->new_branch) >return 1; > > but "git checkout" (no other argument) hits this condition. It > disables the most trivial optimization opportunity, because we are > not "creating". > > "By definition, we're changing"? Really? Not quite. > > If you disable this bogus check, "git checkout" (no other argument) > would be allowed to skip the merge_working_tree(), and that in turn > reveals another case that the helper is not checking when > unpack_trees() MUST be called. > > Note: namely, when sparse checkout is in effect, switching from > HEAD to HEAD can nuke existing working tree files outside the > sparse pattern -- YUCK! See penultimate test in t1011 for > an example. > > This yuckiness is not your fault, but needs_working_tree_merge() > logic you added needs to refrain from skipping unpack_trees() call > when sparse thing is in effect. I'd expect "git checkout -b foo" > instead of "git checkout" (no other argument) would fail to honor > the sparse thing and reveal this bug, because the above bogus > "!opts->new_branch" check will not protect you for that case. > > In other words, these random series of "if (...) return 1" are bugs > hiding other real bugs and we need to reason about which ones are > bugs that are hiding what other bugs that are not covered by this > function. As Peff said earlier for v1, this is still an unreadable > mess. We need to figure out a way to make sure we are skipping on > the right condition and not accidentally hiding a bug of failing to > check the right condition. I offhand do not have a good suggestion > on this; sorry. > >> static int merge_working_tree(const struct checkout_opts *opts, >> struct branch_info *old, >> struct branch_info *new, >> int *writeout_error) >> { >> + /* >> + * Optimize the performance of "git checkout -b foo" by avoiding >> + * the expensive merge, index and working directory updates if they >> + * are not needed. >> + */ >> + if (!needs_working_tree_merge(opts, old, new)) >> + return 0; >> + >> int ret; >> struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file)); > > With the change you made at the beginning of this function, it no > longer compiles with -Wdecl-after-stmt, but that is the smallest of > the problems. > > It is a small step in the right direction to move the call to the > helper from the caller to this function, but it is a bit too small. > > Notice that the lines after the above context look like this: > > hold_locked_index(lock_file, 1); > if (read_cache_preload(NULL) < 0) > return error(_("index file corrupt")); > > resolve_undo_clear(); > if (opts->force) { > ret = reset_tree(new->commit->tree, opts, 1, writeout_error); > if (ret) > return ret; > } else { > struct tree_desc trees[2]; > ... > > I would have expected that the check goes inside the "else" thing > that actually does a two-tree merge, and the helper loses the check > with opts->force, at least. That would still be a change smaller > than desired, but at least a meaningful improvement compared to the > previous one. As I have already pointed out, in the "else" clause > there is a check "is the index free of conflicted entries? if so > error out", and that must be honored in !opt->force case, no matter > what your needs_working_tree_merge() says. I also was hoping that > you would notice, when you were told about the unmerged check, by > reading the remainder of the merge_working_tree(), that we need to > call show_local_changes() when we are not doing force and when we > are not quiet---returning early like the above patch will never be > able to call that one downstream in the function. > > Regardless of what the actual checks end up to be, the right place > to do this "optimization" would look more like: > > builtin/checkout.c | 7 ++- > 1 file changed, 6
Re: git bisect for reachable commits only
On Tue, Aug 2, 2016 at 11:00 PM, Junio C Hamano <gits...@pobox.com> wrote: > Oleg Taranenko <olegtarane...@gmail.com> writes: > >> First, assuming the common ancestor is GOOD based on the fact that >> some descendant given as GOOD is pretty bad idea. > > What you claim is fundamentally incompatible with the way "bisect" > works as a O(log(n)) operation. It is likely that your definition > of Good for the purpose of your bug-hunting needs to be rethought if > you want to take advantage of "bisect". Without context it sounds a bit silly, agree. Context was, maybe not explicit stated, based on previous discussion: If we looking in direct path G..B, of course bisect should show its power O(log(n)); BUT, assuming that any predecessor (G~1/G~2...)...is good if this commit G~n has direct path to B, but not via G, (as git does now) is wrong. This is my concern. Ever G~1 may have not feature we are looking for, then we must treated it as BAD in current git bisect session. To be sure we require additional evidence and just opening a new bisect roundrips in case G~n is GOOD. If G~n confirmed as BAD, we need to stop looking in this path (no need to find transition BAG -> BAD) and switch to another possible common ancestor (or next octopus parent) In merge-based development (opposite to rebased one) this can happen very easy >> I have another request to get git bisect more user-friendly, regarding >> rolling back last step or steps, if accidentally 'git bisect bad' or >> 'good' was wrong entered, but I think it worth for another thread. > > Are you aware that you can check $GIT_DIR/BISECT_LOG and replay it > to recreate any previous state of the bisection? That would > probably help. git bisect log / replay is not convenient. First we need to find place where to keep log file (not forget its name), then edit it. If I'm not sure how many steps I did a mistake the troubles doubles, What are obstacles to implement git bisect back ? or git bisect back --steps=2 I don't think it will be significant change in code. It would be a great help especially if hunting in multiply logically loose-coupled repos. Say searching bug in application, possible caused elusive changes on several dependent libraries. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git bisect for reachable commits only
Guys, thanks for discussion, I will try to reply in bulk here. First, assuming the common ancestor is GOOD based on the fact that some descendant given as GOOD is pretty bad idea. It may be, but may not be. In the git-flow like workflows new features (aka branches) are created from trunk (master/develop/...) sporadically, but later they will mutual merging. I would say more probably they have not common base, then have. Second, I don't ask "create a new algorithm to find all transition from good/old to bad/new", not nesessary. If programmer feels something suspicious, he/she can create another bisect session with narrowed commit range. Third, testing of any specific commit can be very expensive operation. In my case - shutdown servers/refresh dbs/clean/rebuild in eclipse/running servers/dropping browser cache/running app in browser/going through some pages/view UI. Some of steps of course are automated, but some not. Anyway I spend 5-10 min for every iteration. So knowing what commit is bad or good is very valuable, then I'm very interested to hunt the bug-introduced commit with minimal count of testing. Scenario 4 (I will keep my previous mail numbering for possible later reference) z1z2---z3 / / \ Gx1x2/---x3---x4--B \ / / y1--y2--y3--y4 This is the happy straight case with closed DAG (hehe, git for scientists) between given G good and B bad commits. Ideal bisect will check first the shortest way between G & B: x1/x2/x3/x4. Let name first-bug commit we are really hunting H and current first-bug candidate as h. If h == x1 or x2 -> stop, found If h == x3, bisect will try to test y2/y3/y4 path only If h == x4, bisect will select shortest path z1/z2 (keeping in mind, that x2 is already tested and is good) If h == z1 - found if h == z2 - looking in path y1/y2/y3 Scenario 5. v1---v2 / \ w1--/---w2---w3-w4--w5 / / / \ / / /z1z2---z3 \ / / // / \ \ C3--C2--C1--G--x1x2/---x3---x4--x5--x6--B \ / / y1--y2--y3--y4 Unhappy case, we have side branches which may introduce bug behaviour, we need to look it through to figure out why it was done, what problem was solved for that and so on. Let looking in shortest path x1-x6. If h == x1..x4 - happy use case of scenario 4. If discover that h == x5, we are forgetting about z/y paths, but first we looking for nearest common commit (C1). As far as we agree that currently is not clear when the new feature was introduced we need to explicit check commit C1 whether it contains a feature we are hunting bug up. if C1 is good then pretty possible bad transition was happend in w2-w5 commits. Else (C1 is bad) assume that there is no transition from good to bad, then assume H == x5 (stop) if C1 is good and h == w4/w5 => stop, else if h == w3, new roundtrip, forgetting about w1 commit(not interesting), testing C2, if bad - stop H == w3, if good, v1/v2 commits are to test. else if h == w2, forgetting C2 testing, just testing C3. If bad, stop, H == w3, if good, w1 to test. Using this approach we can safe working with ever octopus merging (personally I'm not using, but why not) Scenario 6. z1---z2---z3 // \ C1--G--x1x2/--x3 | \ \ / \| \ y1--y2--y3--y4--y5--y6--B \ \ /| \ w1--w2-w3 | \ / v1--v2 Important note. Before start every side circuit based on common ancestor user should be explicitly warned, that this is not just ordinal intermediate bisect commit testing, but possible new round trips with new commit/steps counts For example, if current shortest path is x1-x6, bisect should say about only 6 commits (3 after bisect), not calculating commits in other paths. Reaching node decision, bisect will stay and prompt for testing new common ancestor with clear instructions what happens, if it will be good or bad, (new unchecked commits and new left bisect steps, in case good and stop or switch to other path in case of octopus). I have another request to get git bisect more user-friendly, regarding rolling back last step or steps, if accidentally 'git bisect bad' or 'good' was wrong entered, but I think it worth for another thread. Cheers, Oleg -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git bisect for reachable commits only
Guys, further investigation shows, git bisect is broken from its core... really. Let consider 3rd a bit more complicated scenario #cd .. #rm -rf bisect3 mkdir bisect3 cd bisect3 git init git touch coffee git commit -am "create coffee" git branch tee echo sugar >> coffee git commit -am "add sugar" # we are still in master branch echo "milk" >> coffee git commit -am "milk for coffee" ex +g/sugar/d -cwq coffee # introducing 'bug' git commit -am "somehow remove sugar" echo "mixing..." >> coffee git commit -am "coffee mixing" git checkout tee# get back to coffee without sugar git touch tee git commit -am "tee" git branch cocktail echo "sugar" >> tee git commit -am "sugar for tee" echo "milk" >> tee git commit -am "milk for tee" echo "mixing..." >> tee git commit -am "tee mixing" git checkout cocktail git touch cocktail git commit -am "prepare cocktail" echo orange >>cocktail git commit -am "add orange juice" echo rum >>cocktail git commit -am "add rum" echo mixing >> cocktail git commit -am "cocktail mixing" cat cocktail #orange, rum, mixing git merge tee git merge master git touch serve git commit -am "serving..." git log --full-history --graph --pretty=oneline * 059adf903a2cbc06fe05dda4c916e2c586907f23 serving... * efc89d5253d3126defc7362c25ef069ae9b43fc7 Merge branch 'master' into cocktail |\ | * dd41e230a3cac5d51a1e994747ff470e2af03cae coffee mixing | * c2a44672f1197f34e04cd0fd66434a2b286b574e somehow remove sugar | * f50352cfb6bc4a237b73c95ed7ebca074603ae11 milk for coffee | * 79b253b316cdc3668697afe473610e35b453ab2f add sugar * | 2d626eb5cfaa40a4503be58a5ed27f1ececa6d02 Merge branch 'tee' into cocktail |\ \ | * | 7aba690c6c6f73f1906871c9dbf9737ec11a152b tee mixing | * | eca611a93697359ec7a52f4a045461180bc365c3 milk for tee | * | 7d6844724d0e81751ec1a67c1ffdf0d0fb932350 sugar for tee * | | 6754e816922989d5870ec3452437bbbe6aca4d0f cocktail mixing * | | 5cbbf0f0882c497590838b163210db3a393b647e add rum * | | b46d7d8a361daae382fbef7acabda5416d23da46 add orange juice * | | e571fdd09582e40fc54ffc5a4f112eac2b9f2c8e prepare cocktail |/ / * | 041a5a53704bccc60c489f8c9a4742bad79d5a95 tee |/ * a52a4fa6770d000a9f4e9e297739a6dc88c0cc50 create coffee As you can see, no tricks with amended history, but... git bisect start HEAD 79b2 Bisecting: 8 revisions left to test after this (roughly 3 steps) [6754e816922989d5870ec3452437bbbe6aca4d0f] cocktail mixing cat coffee git bisect bad Bisecting: 2 revisions left to test after this (roughly 1 step) [e571fdd09582e40fc54ffc5a4f112eac2b9f2c8e] prepare cocktail git bisect bad Bisecting: 0 revisions left to test after this (roughly 0 steps) [041a5a53704bccc60c489f8c9a4742bad79d5a95] tee git bisect bad 041a5a53704bccc60c489f8c9a4742bad79d5a95 is the first bad commit commit 041a5a53704bccc60c489f8c9a4742bad79d5a95 Author: Oleg Taranenko <olegtarane...@gmail.com> Date: Mon Aug 1 10:53:52 2016 +0200 tee :00 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 A tee git bisect ever not looked into the path where good commit is declared. Instead it found somehow most common ancestor from whole tree (a52a create coffee), assume it is GOOD commit (why !?) and check only ^1 (not ^2) parent commit for testing as a potential bug commit. No wonder now, I got a disaster result, looking in my heavy enterprise repository. Can somebody take care of this issue? Thanks, Oleg On Sun, Jul 31, 2016 at 2:06 AM, Oleg Taranenko <olegtarane...@gmail.com> wrote: > Hi Junio, > > Thanks for reply. > Let consider two pretty similar use cases. > > SCENARIO 1 > > mkdir bisect > cd bisect/ > git init > git touch coffee > git commit -am "create coffee" > git branch develop > echo sugar >> coffee > git commit -am "add sugar" # we are still in master branch > git checkout develop # get back to coffe without sugar > git touch tee # cooking tee in develop branch > git commit -am "tee" > git merge master # > cat coffee # after merge coffe has sugar > ex +g/sugar/d -cwq coffee # introducing 'bug' by removing sugar from coffee > git commit -am "merged/amended" --amend # the history is amended > echo "sugar" >> tee > git commit -am "sugar for tee" # just advance for measure > > # -- We are getting following state -- > git status # develop branch > git log --full-history --graph --pretty=oneline > * 83e9577b4a5d553fdc16806fdea9757229ea9222 sugar for tee > * 23a4aa69a9d5c03aa14584400b7ee00c4d6
Re: git bisect for reachable commits only
Hi Junio, Thanks for reply. Let consider two pretty similar use cases. SCENARIO 1 mkdir bisect cd bisect/ git init git touch coffee git commit -am "create coffee" git branch develop echo sugar >> coffee git commit -am "add sugar" # we are still in master branch git checkout develop # get back to coffe without sugar git touch tee # cooking tee in develop branch git commit -am "tee" git merge master # cat coffee # after merge coffe has sugar ex +g/sugar/d -cwq coffee # introducing 'bug' by removing sugar from coffee git commit -am "merged/amended" --amend # the history is amended echo "sugar" >> tee git commit -am "sugar for tee" # just advance for measure # -- We are getting following state -- git status # develop branch git log --full-history --graph --pretty=oneline * 83e9577b4a5d553fdc16806fdea9757229ea9222 sugar for tee * 23a4aa69a9d5c03aa14584400b7ee00c4d63 merged/amended |\ | * 4c1caf7cb2417181c035a953afdf2389dd130aef add sugar * | c080fb4df39d721e2f2e0fdd91fe16d8bdd77515 tee |/ * 3c3043b7d0a0d260c78db55b565f26e430aa5c80 create coffee cat coffee # nothing# discovering coffee has no sugar git checkout 4c1c # but we remember it should to have cat coffee # ..."sugar" git bisect start git bisect good git bisect bad develop # 23a4 cat coffee # nothing git bisect bad # c080 cat coffee # nothing git bisect bad # c080fb4df39d721e2f2e0fdd91fe16d8bdd77515 is the first bad commit commit c080fb4df39d721e2f2e0fdd91fe16d8bdd77515 Author: Oleg Taranenko <olegtarane...@gmail.com> Date: Fri Jul 29 09:08:47 2016 +0200 tee :00 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 A tee We are getting first bad commit c080, but git bisect fails. We remember is was introduced in the 23a4 commit via wrong merge and history amending. SCENARIO 2 cd .. mkdir bisect2 cd bisect2 git init git touch coffee git commit -am "create coffee" echo sugar >> coffee # we are still in master branch git commit -am "add sugar" git branch develop echo milk >> coffee git commit -am "add milk to coffee" # get back to coffe without sugar git checkout develop ex "+g/sugar/d" -cwq coffee echo milk >> coffee git commit -am "coffee: replace sugar with milk" # cooking tee in develop branch git touch tee git commit -am "tee" git checkout master git merge develop #Here we are getting real conflict cat coffee #<<<<<<< HEAD #sugar #=== #>>>>>>> develop #milk #resolving git checkout develop --theirs -- coffee cat coffee # milk git commit -am "conflict resolved" echo "sugar" >> tee git commit -am "sugar for tee" # just advance for measure -- State - git log --full-history --graph --pretty=oneline * b88a3cb3df58fc018d635d559d212707e953f84d sugar for tee * 138824139c0237fe05419d4f40a693e4c19405a3 conflict resolved |\ | * e1ddbfe05d632d6f12dd7ff9d9b61475c2cde867 tee | * ddfb5188c98b8fc803a036ac4eee0610e2bba53f coffee: replace sugar with milk * | 0e1c55363e5b2fb04a6072fa470f90770b3eee22 add milk to coffee |/ * 465d0c68c597f1534c3c1e19ed9a086c5da190ae add sugar * 24b73ce9085a6d411c06c08cca0536dc8f2239c7 create coffee cat coffee # only milk, no sugar... bug git checkout 792d cat coffee # OK, milk & sugar git bisect start git bisect good git bisect bad master # e1dd cat coffee # milk only git bisect bad # ddfb cat coffee # milk only git bisect bad # first bad commit !! It happens, git really found that somebody (me) was replaced sugar with milk, because ancestor of both branches already has sugar, and commit ddfb explicit removes it. As we can see, both strategies can coexisting, and now I ever can't state for sure, which one is more intuitive correct. I think if repo has relative straight history, more productive to use bisect with auto search in un-reachable commits. For messy repositories (especially, with lots of aliens code) more safe to use --reachable bisecting strategy. Then, I suggest as well additional to defaulting via 'git config bisect.reachable true/false' use per bisect session switch git bisect start --[un-]reachable-commits # which will override default setting Thanks you for reading to this point, Oleg On Fri, Jul 29, 2016 at 8:03 PM, Junio C Hamano <gits...@pobox.com> wrote: > Oleg Taranenko <olegtarane...@gmail.com> writes: > >> What I suggest change logic of bi
git bisect for reachable commits only
Hi all, I just found an interesting post how git bisect can trouble humans. Full story is here https://groups.google.com/forum/#!topic/git-users/v3__t42qbKE My suggestion to fix it (ditto) What I suggest change logic of bisecting to something like git config bisect.reachable true This behaviour will reduce bisecting only to reachable commits between bad/good points and will never goes away. Maybe if commit will be found at merge commit git bisect can suggest open another round trip to looking into not reachable way, but anyway it should be explicit confirmed by user. BTW, if bad commit is merge point with more than 2 parents, how git bisect will find "first wrong commit" at the current state? Thanks for the attention, Oleg -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html