2

In relation to my question here: Git; code disappeared after merge

It happened again, and I don't understand how. The issue is, that a line of code was changed in a commit, but the old line exists in the current repo-version of the file, which seems really weird. Does anyone have any insight into how this is happening, and how we avoid it?

The line in question is the function header for the create_marking method

The commit that changed the line (as expected):

commit 925ec3c11006ccca37cf684443d0fad3e1781dca
Author: xxxx
Date:   Tue Jun 4 14:55:52 2013

    Comment

diff --git a/lib/services/MarkingService.class.php b/lib/services/MarkingService.class.php
index 66a33f7..acf830a 100644
--- a/lib/services/MarkingService.class.php
+++ b/lib/services/MarkingService.class.php
@@ -34,18 +34,12 @@ class MarkingService \{
         return '[' . implode(',', self::getCoordinates($marking)) . ']';
     \}
\}

-    public function create_marking($category, $timestamp, $field_id, $lat, $lon, $accuracy, $lat2 = null, $lon2 = null, $accuracy2 = null, $spread = null, $depth = null, $comment = null) \{
+    public function create_marking($category, $timestamp, $field_id, $coordinates, $spread = null, $depth = null, $comment = null) \{
         $this->validate_category($category);
         $this->validate_timestamp($timestamp);

These next two commits are THE ONLY commits in the history, after the one above, and as far as I can tell, they do not in any way change the line in question:

commit 2ab5fb14765caa269c027e8c57b11232b0441625
Author: xxx
Date:   Thu Jul 4 13:58:26 2013

    Comment

diff --git a/lib/services/MarkingService.class.php b/lib/services/MarkingService.class.php
index 66a33f7..c553876 100644
--- a/lib/services/MarkingService.class.php
+++ b/lib/services/MarkingService.class.php
@@ -8,32 +8,6 @@ class MarkingService {
         return $instance;
     }

-    public static function getCoordinates(lmMarking $marking)
-    {
-        $returnValue = array($marking->getLatitude() . ',' . $marking->getLongitude());
-        
-        if (0 < $marking->getLatitude2()) {
-            $returnValue[] = $marking->getLatitude2() . ',' . $marking->getLongitude2();
-        }
-        
-        return $returnValue;
-    }
-    
-    public static function getCoordinatesJavascript(lmMarking $marking)
-    {
-        return '[' . implode(',', self::getCoordinates($marking)) . ']';
-    }
-
     public function create_marking($category, $timestamp, $field_id, $lat, $lon, $accuracy, $lat2 = null, $lon2 = null, $accuracy2 = null, $spread = null, $depth = null, $comment = null) {
         $this->validate_category($category);
         $this->validate_timestamp($timestamp);

and

commit d152befa9977b8fc13df9f3ea3f756217751cb0d
Merge: 77b20e2 2ab5fb1
Author: xxx
Date:   Thu Jul 4 14:43:26 2013

    Comment

diff --cc lib/services/MarkingService.class.php
index acf830a,c553876..f5a8bc4
--- a/lib/services/MarkingService.class.php
+++ b/lib/services/MarkingService.class.php
@@@ -131,34 -157,4 +105,34 @@@ class MarkingService 
              throw new InvalidArgumentException("Invalid depth: '$depth'");
          }
      }
 +    
 +    private function validateCoordinates($coordinates)
 +    {
 +        $validatorOptions = array(
 +            'lat' => array(
 +                'field' => 'latitude',
 +                'validator' => FILTER_VALIDATE_FLOAT
 +            ),
 +            'lng' => array(
 +                'field' => 'longitude',
 +                'validator' => FILTER_VALIDATE_FLOAT
 +            ),
 +            'accuracy' => array(
 +                'field' => 'accuracy',
 +                'validator' => FILTER_VALIDATE_INT
 +            )
 +        );
 +        
 +        foreach ($coordinates as $coordinate) {
 +            foreach ($coordinate as $key => $value) {
 +                if (false === filter_var($coordinate[$key], $validatorOptions[$key]['validator'])) {
 +                    throw new InvalidArgumentException("Invalid " . $validatorOptions[$key]['field'] . ": '" . $value . "'");
 +                }
 +            }
 +        }
 +    }
- }
+ }

and for some reason I cannot fathom, the line in the file now reads

public function create_marking($category, $timestamp, $field_id, $lat, $lon, $accuracy, $lat2 = null, $lon2 = null, $accuracy2 = null, $spread = null, $depth = null, $comment = null) {

What am I doing wrong?

EDIT output of git status

$ git status
# On branch master
nothing to commit, working directory clean

output of git log

$ git log lib/services/MarkingService.class.php
commit d152befa9977b8fc13df9f3ea3f756217751cb0d
Merge: 77b20e2 2ab5fb1
Author: xxx
Date:   Thu Jul 4 14:43:26 2013 +0300

    Changed the format of Markings block in Sync API method.
    Send markings coordinates in the API in the new format

commit 2ab5fb14765caa269c027e8c57b11232b0441625
Author: xxx
Date:   Thu Jul 4 13:58:26 2013 +0300

    Changed the format of Markings block in Sync API method.
    Send markings coordinates in the API in the new format

commit 925ec3c11006ccca37cf684443d0fad3e1781dca
Author: xxx
Date:   Tue Jun 4 14:55:52 2013 +0300

    Changed the makrings coordinates structure.
    Allow more points in CreateMarking

git log --oneline --graph

$ git log --oneline --graph lib/services/MarkingService.class.php
*   d152bef Changed the format of Markings block in Sync API method. Send markings coordinates in the API in the new format
|\
| * 2ab5fb1 Changed the format of Markings block in Sync API method. Send markings coordinates in the API in the new format
* | 925ec3c Changed the makrings coordinates structure. Allow more points in CreateMarking
|/
* bacfb14 An intermediate commit. HOTFIX: Fix markings
* 6479513 LETFARM-1863 DB and API support for markings
Community
  • 1
  • 1
Vonsild
  • 587
  • 1
  • 4
  • 7
  • What's the output of `git status` and `git log lib/services/MarkingService.class.php`? – Koraktor Jul 10 '13 at 12:37
  • I have also tried to reset to the commit that introduced the desired change, but that line is STILL the old version. – Vonsild Jul 10 '13 at 12:41
  • If you look at commit 2ab5fb14765caa269, the line in question is the old version. When you are updating your branch are you doing `git merge` or `git rebase`? – Schleis Jul 10 '13 at 12:46
  • @Koraktor - added the info to my question – Vonsild Jul 10 '13 at 12:46
  • @Schleis - I'm using merge – Vonsild Jul 10 '13 at 12:49
  • So there is also a merge commit in the history then? – Schleis Jul 10 '13 at 13:33
  • The last commit mentioned - d152befa9977b8fc13df9f3ea3f756217751cb0d - is a merge commit, yes – Vonsild Jul 10 '13 at 13:37
  • What is the output of `git log --oneline --graph`? – Chronial Jul 10 '13 at 14:11
  • @Chronial - appended to question – Vonsild Jul 10 '13 at 14:22
  • hehe, i think git is hiding something from you :). Do you get a different output for `git log --oneline --graph --full-history`? – Chronial Jul 10 '13 at 14:26
  • waaaaiiit a second: how did you create that merge? – Chronial Jul 10 '13 at 14:31
  • The --full-history thing seems to list a lot of commits in no way related to that file, so I don't know what use it will be. The merge was a regular merge - the guy who made it says there were no conflicts. Solved is such a big word :) I'm still a bit fuzzy on why and how it happened, but I suppose we should start rebasing some stuff, though that seems to bring it's own set of problems – Vonsild Jul 11 '13 at 07:55

1 Answers1

3

When you pulled and did the merge, commit 2ab5fb14765caa269c027e8c57b11232b0441625 removed a large chunk of code near where you made your change and git mistakenly resolved the merge using that commits version of the line. You can see the line in the patch for that commit.

Git doesn't keep track of line changes. It tracks the state of the files at a given point.

Git doesn’t think of or store its data this way. Instead, Git thinks of its data more like a set of snapshots of a mini filesystem. Every time you commit, or save the state of your project in Git, it basically takes a picture of what all your files look like at that moment and stores a reference to that snapshot. To be efficient, if files have not changed, Git doesn’t store the file again—just a link to the previous identical file it has already stored.

http://git-scm.com/book/en/Getting-Started-Git-Basics

So Git is seeing the commit that you pulled in as a change even to the line you modified and "resolved" the merge by choosing the line.

I have seen this happen before and it usually happens due to removing / re-ordering large chunks of code that happen to be near where other changes occur. My recommendation to avoid this is to use git pull --rebase or git rebase when you are updating your local branch.

http://git-scm.com/book/en/Git-Branching-Rebasing

This will move your local commits to after any that are already on the remote and if git has difficulty in applying the commit will ask you to change your commit. This also keeps the history linear and doesn't let git resolve merges incorrectly on you.

Schleis
  • 41,516
  • 7
  • 68
  • 87
  • Thank you, I'll try that. Seems very odd though, as that line is not shown as changed in the diff of that commit. – Vonsild Jul 10 '13 at 14:04
  • The reason that it isn't marked as changed in the commit, is that git is comparing the file state between it's parent commit and itself. In that commit that line wasn't changed. So hence when looking at the patch you don't see a change. – Schleis Jul 10 '13 at 14:10
  • Hmm - so that basically means, you can't trust the output? As it clearly does not reflect the actual changes to the files – Vonsild Jul 10 '13 at 14:15
  • 2
    No, the question is what a "change" is – by default git says "what’s different compared to first parent". If you want to see changes against both parents, you need to add `-c` to your `git log`. – Chronial Jul 10 '13 at 14:16