Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simpler solution to Juggling Commits 2 not recognized #28

Closed
jedbrown opened this issue Feb 17, 2013 · 8 comments
Closed

Simpler solution to Juggling Commits 2 not recognized #28

jedbrown opened this issue Feb 17, 2013 · 8 comments

Comments

@jedbrown
Copy link

This 3-command sequence achieves the desired outcome

git rebase newImage master # checkout and fast-forward master
git commit --amend
git cherry-pick caption

This 4-command sequence also achieves the same thing

git checkout master
git reset --hard newImage
git commit --amend
git cherry-pick caption

Neither of the above are recognized. The expected solution seems to be

git checkout master
git cherry-pick newImage # identical except for the date
git commit --amend
git cherry-pick caption

although this also works

git rebase newImage master
git commit --amend
git commit --amend  # yes, twice
git cherry-pick caption
@pcottle
Copy link
Owner

pcottle commented Feb 17, 2013

Thanks for the heads up @jedbrown. Cherry-pick should be disabled in all of these levels now -- have you refreshed recently? It's a cheap hack but at least it will guide people towards the solution that the level expects.

You're totally right though -- the solution is uber specific about the number of times a commit has been amended, which is somewhat irrelevant to the goal of the level (which is to amend one commit and then get everything in the right order). Right now I can compare the working directory to the "solution" tree in a few ways (everything, all branches, specific branches, just master, etc) but I haven't implemented anything that does relative comparisons (aka C2 was amended once more than C3, with nothing else mattering).

I might try to crank that out this weekend because it will be nice for levels going forward, but it's a somewhat non-trivial graph traversal algorithm so unfortunately it's not a one step solution.

Until it's fixed though, this level kinda sucks. The solution even requires you to manually drag stuff around, and that's a whole other problem...

pcottle added a commit that referenced this issue Feb 18, 2013
@pcottle
Copy link
Owner

pcottle commented Feb 18, 2013

Ok @jedbrown I'm about halfway there. I just made a hash-agnostic tree comparison function that will bubble up both trees correctly but only compare the base ID of each. This essentially just means that the "order" of each branch has to be correct -- for example:

if treeA has a git log on master that produces:

C4''' -> C3'' -> C2'^6 -> C1' -> C0

and treeB's git log on master produces:

C4 -> C3 -> C2 -> C1 -> C0

Then with the hash-agnostic comparison they are equal. However if treeA had C1' come before C3''', then this comparison would fail.

The next step would be to implement some kind of assert mechanism so I can compare the ordering of two trees and then ensure one commit has been amended once more than its parents / children...

@pcottle
Copy link
Owner

pcottle commented Mar 10, 2013

So after writing half of an assertion engine for the tree compare, I realized that there's no good way to fix this :-/

If you do the simpler solution @jedbrown, it's impossible to tell from the final graph if the commit was actually amended or the user just cherry-picked the two commits. So I have no way to accept the simpler solution unless I keep some kind of weird state about which commands have been executed on which commits, which would get SUPER hairy really fast.

This is unfortunate but I think this isn't doable for now :-/ I'm closing this for the time being, but I hope to come up with something eventually. It's a tricky issue, but I'm more concerned about noobies accidentally passing the level over the git experts not having their optimal solutions being recognized

@pcottle pcottle closed this as completed Mar 10, 2013
@pcottle
Copy link
Owner

pcottle commented Mar 10, 2013

That being said, I think there is value in providing support for something where the commit line:

C2''' -> C3'' <- master

is equivalent to

C2'' -> C3' <- master

Aka hash-relative comparison. I'll aim for that....

@pcottle
Copy link
Owner

pcottle commented Mar 10, 2013

Wow! So I just built up an assertion engine, so a tree like this:

Screen shot 2013-03-10 at 12 10 21 PM

is EQUIVALENT to a tree like this:

Screen shot 2013-03-10 at 12 12 22 PM

Because I solve the level with hash-agnostic tree comparison and then hash assertions of the form:

    "master": [
      function(data) {
        return data.C2 > data.C3;
      },
      function(data) {
        return data.C2 > data.C1;
      }
    ]

Essentially saying that the hashes on C2 have to be more than C1 and C3. If that is true and the hash-stripped version of the trees match, then you pass the level!

So while I can't "know" if a commit has been amended yet @jedbrown, I can at least give people a break for taking one extra step. Phew! I think I outdid myself this time...

@mangobrain
Copy link

Juggling commits 2 is still broken IMHO. The original complaint that the following simple solution is not recognised is still valid:

git rebase newImage master # checkout and fast-forward master
git commit --amend
git cherry-pick caption

The objective text says C2 only needs to be amended once, which is exactly what has happened. However, in the resulting graph, master will contain C2' and C3' (NOT C2'' and C3'). This level rewards you for cheating (amending C2 twice) or figuring out the exact expected command sequence, not for solving the stated problem!

@pcottle
Copy link
Owner

pcottle commented Jul 20, 2015

Yeah @mangobrain, again I wish I had a way to mark which commits were amended and then use that in the final level comparison. There are obvious shortcomings to the comparison modes we have, but I think this is the most general comparison mode we can use for now (and better than the pre-issue state).

The reason why I never built this up is that it requires piping a new boolean value (isAmended) through every state transition (cherry-picking, rebasing, git push/pull, etc) and tree serialization / deserialization, etc etc. I'd happily accept a PR that whips this all up, but don't think its worth the effort at the moment right now.

Besides, the overall goal of LGB is to help newbies learn -- usually the people that figure out the alternate solutions are smart enough to see where my comparison methods fall short (like yourself) :P

@skogshjort
Copy link

skogshjort commented Oct 9, 2024

Hi!

I'm new to git, and did jump ahead a bit so my understanding of interactive rebase is not so good. So my instinct to solve Juggling Commits was the following

git checkout newImage                    # switch to the earlier commit
git commit --amend                       # make changes
git rebase --onto HEAD caption~ caption  # is this where others are cherry picking?

or

git checkout newImage                    # switch to the earlier commit
git commit --amend                       # make changes
git checkout caption                     # switch to the later commit
git rebase newImage                      # a bit easier to type

which to me is more intuitive way of swapping (although I am a complete beginner, so I might think about this the wrong way) - partly because the proposed solution temporarily has the branch "caption" point to the commit C2 (which contains the changes for newImage) which I find confusing and demanding on my brain's working memory.

Did I think of a more intuitive way to solve this or did I misunderstand the assignment?

Hugs, and thank you so much for this project! I don't expect any answer to this but would be flattered to get one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants