* Re: [PATCH] Add tests for documented features of "git reset".
From: Junio C Hamano @ 2007-09-14 22:10 UTC (permalink / raw)
To: Carlos Rica; +Cc: git, Johannes Schindelin
In-Reply-To: <1b46aba20709141501l6f0f7440hd22b2bd6c4838a0b@mail.gmail.com>
"Carlos Rica" <jasampler@gmail.com> writes:
> 2007/9/11, Junio C Hamano <gitster@pobox.com>:
>> I also suspect this would not pass on CRLF boxes.
>
> I finally removed the hardcoded object IDs (patch attached,
> to be applied on top of the previous patch).
>
> But I don't know what to fix for making the test to pass
> in CRLF boxes.
The blobs created by the test script would be different on CRLF
boxes, and exact object IDs hardcoded in your test would not
match. That was the only thing I meant.
But now you bring it up, I guess the expected output may not
match as well. Hmmm..
Anyway, thanks for the update.
^ permalink raw reply
* [StGit PATCH 00/13] Eliminate 'top' and 'bottom' files
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
The following series removes the 'bottom' and 'top' files for each
patch, and instead uses the commit objects to keep track of the
patches.
The patches are based on kha/safe
24a81d7a94cd7c9ad2fc741b0179db5b830cce78 and will conflict with the
'conflict' series in at least one place.
The first ten patches are actually only cleanups and refactoring that
could go in regardless of the last ones. Some of the changes are not
really necessary, but I did them while digging into the the code, and
believe they are improvements. They should not change the way stg
behaves in any way.
The eleventh one is just a sanity check before applying the last two
that removes the top and bottom files.
The last two patches do the final cleansing. Obviously, this changes
the format, and the format version should be increased and and update
function be written. So it's not really ready to go in yet.
Also, there are some documentation changes not included in this.
Maybe I should have sent the first ten patches separately, but this
makes it clearer why I did it.
---
David Kågedal (13):
Remove the 'top' field
Remove the 'bottom' field
Check bottom and invariants
Refactor Series.new_patch
Clear up the semantics of Series.new_patch
Add a 'bottom' parameter to Series.refresh_patch and use it
Clean up Series.refresh_patch
Refactor Series.push_patch
Remove dead code from push_empty_patch
Split Series.push_patch in two
Moved that status function to the status command file
Clear up semantics of tree_status
Add some more tests of "stg status" output
stgit/commands/common.py | 2
stgit/commands/status.py | 42 +++++++++-
stgit/commands/sync.py | 1
stgit/commands/uncommit.py | 1
stgit/git.py | 61 +++-----------
stgit/stack.py | 189 ++++++++++++++++++++++----------------------
t/t0002-status.sh | 36 ++++++++
7 files changed, 185 insertions(+), 147 deletions(-)
--
Signature
^ permalink raw reply
* [StGit PATCH 01/13] Add some more tests of "stg status" output
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
t/t0002-status.sh | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index ce3b688..790b9fb 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -85,6 +85,12 @@ test_expect_success 'Status after refresh' '
diff -u expected.txt output.txt
'
+test_expect_success 'Add another file' '
+ echo lajbans > fie &&
+ stg add fie &&
+ stg refresh
+'
+
test_expect_success 'Make a conflicting patch' '
stg pop &&
stg new -m "third patch" &&
@@ -105,6 +111,28 @@ test_expect_success 'Status after conflicting push' '
'
cat > expected.txt <<EOF
+C foo/bar
+EOF
+test_expect_success 'Status of file' '
+ stg status foo/bar > output.txt &&
+ diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
+EOF
+test_expect_success 'Status of dir' '
+ stg status foo > output.txt &&
+ diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
+EOF
+test_expect_success 'Status of other file' '
+ stg status fie > output.txt &&
+ diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
M foo/bar
EOF
test_expect_success 'Status after resolving the push' '
^ permalink raw reply related
* [StGit PATCH 03/13] Moved that status function to the status command file
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
The git.status() was more of a command than a library function, and
was only used in one place.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/commands/status.py | 39 ++++++++++++++++++++++++++++++++++++---
stgit/git.py | 33 ---------------------------------
t/t0002-status.sh | 8 ++++++++
3 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index de88ba0..bbfb5df 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -65,6 +65,39 @@ options = [make_option('-m', '--modified',
action = 'store_true')]
+def status(files = None, modified = False, new = False, deleted = False,
+ conflict = False, unknown = False, noexclude = False,
+ diff_flags = []):
+ """Show the tree status
+ """
+ cache_files = git.tree_status(files,
+ unknown = (files == None),
+ noexclude = noexclude,
+ diff_flags = diff_flags)
+ filtered = (modified or new or deleted or conflict or unknown)
+
+ if filtered:
+ filestat = []
+ if modified:
+ filestat.append('M')
+ if new:
+ filestat.append('A')
+ filestat.append('N')
+ if deleted:
+ filestat.append('D')
+ if conflict:
+ filestat.append('C')
+ if unknown:
+ filestat.append('?')
+ cache_files = [x for x in cache_files if x[0] in filestat]
+
+ for fs in cache_files:
+ assert files == None or fs[1] in files
+ if not filtered:
+ out.stdout('%s %s' % (fs[0], fs[1]))
+ else:
+ out.stdout('%s' % fs[1])
+
def func(parser, options, args):
"""Show the tree status
"""
@@ -85,6 +118,6 @@ def func(parser, options, args):
# No args means all files
if not args:
args = None
- git.status(args, options.modified, options.new, options.deleted,
- options.conflict, options.unknown, options.noexclude,
- diff_flags = diff_flags)
+ status(args, options.modified, options.new, options.deleted,
+ options.conflict, options.unknown, options.noexclude,
+ diff_flags = diff_flags)
diff --git a/stgit/git.py b/stgit/git.py
index 8dda1ed..b112787 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -711,39 +711,6 @@ def merge(base, head1, head2, recursive = False):
if errors:
raise GitException, 'GIT index merging failed (possible conflicts)'
-def status(files = None, modified = False, new = False, deleted = False,
- conflict = False, unknown = False, noexclude = False,
- diff_flags = []):
- """Show the tree status
- """
- cache_files = tree_status(files,
- unknown = (files == None),
- noexclude = noexclude,
- diff_flags = diff_flags)
- filtered = (modified or new or deleted or conflict or unknown)
-
- if filtered:
- filestat = []
- if modified:
- filestat.append('M')
- if new:
- filestat.append('A')
- filestat.append('N')
- if deleted:
- filestat.append('D')
- if conflict:
- filestat.append('C')
- if unknown:
- filestat.append('?')
- cache_files = [x for x in cache_files if x[0] in filestat]
-
- for fs in cache_files:
- assert files == None or fs[1] in files
- if not filtered:
- out.stdout('%s %s' % (fs[0], fs[1]))
- else:
- out.stdout('%s' % fs[1])
-
def diff(files = None, rev1 = 'HEAD', rev2 = None, diff_flags = []):
"""Show the diff between rev1 and rev2
"""
diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index 790b9fb..d0c31b2 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -60,6 +60,14 @@ test_expect_success 'Status with an added file' '
'
cat > expected.txt <<EOF
+foo/bar
+EOF
+test_expect_success 'Status with an added file and -n option' '
+ stg status -n > output.txt &&
+ diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
EOF
test_expect_success 'Status after refresh' '
stg new -m "first patch" &&
^ permalink raw reply related
* [StGit PATCH 02/13] Clear up semantics of tree_status
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
The tree_status() function does a few slightly different things
depending on the arguments. This patch adds checks that the arguments
are consistent and that the returned value looks good.
It also changes the semantics slightly. If the 'files' parameter is
None, it will run status on all files. If 'files' is a list, it will
run status on only those files. This changes two things:
1) If 'files' is the empty list, it will run status on no files.
2) It 'files' is a list, it will never return the status for other files
Clearing this up will make it easier to understand code that is using
this function.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/commands/status.py | 3 +++
stgit/git.py | 44 +++++++++++++++++++++++---------------------
2 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index 156552b..de88ba0 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -82,6 +82,9 @@ def func(parser, options, args):
else:
diff_flags = []
+ # No args means all files
+ if not args:
+ args = None
git.status(args, options.modified, options.new, options.deleted,
options.conflict, options.unknown, options.noexclude,
diff_flags = diff_flags)
diff --git a/stgit/git.py b/stgit/git.py
index 539d699..8dda1ed 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -167,15 +167,20 @@ def exclude_files():
def tree_status(files = None, tree_id = 'HEAD', unknown = False,
noexclude = True, verbose = False, diff_flags = []):
- """Returns a list of pairs - (status, filename)
+ """Get the status of all changed files, or of a selected set of
+ files. Returns a list of pairs - (status, filename).
+
+ If 'files' is None, it will check all files, and optionally all
+ unknown files. If 'files' is a list, it will only check the files
+ in the list.
"""
+ assert files == None or not unknown
+
if verbose:
out.start('Checking for changes in the working directory')
refresh_index()
- if not files:
- files = []
cache_files = []
# unknown files
@@ -197,11 +202,14 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,
conflicts = get_conflicts()
if not conflicts:
conflicts = []
- cache_files += [('C', filename) for filename in conflicts]
+ cache_files += [('C', filename) for filename in conflicts
+ if files == None or filename in files]
# the rest
- for line in GRun('git-diff-index', *(diff_flags + [tree_id, '--'] + files)
- ).output_lines():
+ args = diff_flags + [tree_id]
+ if files != None:
+ args += ['--'] + files
+ for line in GRun('git-diff-index', *args).output_lines():
fs = tuple(line.rstrip().split(' ',4)[-1].split('\t',1))
if fs[1] not in conflicts:
cache_files.append(fs)
@@ -209,6 +217,7 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,
if verbose:
out.done()
+ assert files == None or set(f for s,f in cache_files) <= set(files)
return cache_files
def local_changes(verbose = True):
@@ -538,9 +547,6 @@ def committer():
def update_cache(files = None, force = False):
"""Update the cache information for the given files
"""
- if not files:
- files = []
-
cache_files = tree_status(files, verbose = False)
# everything is up-to-date
@@ -569,8 +575,6 @@ def commit(message, files = None, parents = None, allowempty = False,
committer_name = None, committer_email = None):
"""Commit the current tree to repository
"""
- if not files:
- files = []
if not parents:
parents = []
@@ -712,14 +716,13 @@ def status(files = None, modified = False, new = False, deleted = False,
diff_flags = []):
"""Show the tree status
"""
- if not files:
- files = []
-
- cache_files = tree_status(files, unknown = True, noexclude = noexclude,
- diff_flags = diff_flags)
- all = not (modified or new or deleted or conflict or unknown)
+ cache_files = tree_status(files,
+ unknown = (files == None),
+ noexclude = noexclude,
+ diff_flags = diff_flags)
+ filtered = (modified or new or deleted or conflict or unknown)
- if not all:
+ if filtered:
filestat = []
if modified:
filestat.append('M')
@@ -735,9 +738,8 @@ def status(files = None, modified = False, new = False, deleted = False,
cache_files = [x for x in cache_files if x[0] in filestat]
for fs in cache_files:
- if files and not fs[1] in files:
- continue
- if all:
+ assert files == None or fs[1] in files
+ if not filtered:
out.stdout('%s %s' % (fs[0], fs[1]))
else:
out.stdout('%s' % fs[1])
^ permalink raw reply related
* [StGit PATCH 05/13] Remove dead code from push_empty_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
Since the split from push_patch, the push_empty_patch contains some
code that fills no purpose. Remove it and simplify the code.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 33 +++------------------------------
1 files changed, 3 insertions(+), 30 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index 79d6cf3..f31f308 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -1029,46 +1029,19 @@ class Series(PatchSet):
assert(name in unapplied)
patch = self.get_patch(name)
-
head = git.get_head()
- bottom = patch.get_bottom()
- top = patch.get_top()
-
- ex = None
- modified = False
- # top != bottom always since we have a commit for each patch
- # just make an empty patch (top = bottom = HEAD). This
- # option is useful to allow undoing already merged
- # patches. The top is updated by refresh_patch since we
- # need an empty commit
+ # The top is updated by refresh_patch since we need an empty
+ # commit
patch.set_bottom(head, backup = True)
patch.set_top(head, backup = True)
- modified = True
append_string(self.__applied_file, name)
unapplied.remove(name)
write_strings(self.__unapplied_file, unapplied)
- # head == bottom case doesn't need to refresh the patch
- if not ex:
- # if the merge was OK and no conflicts, just refresh the patch
- # The GIT cache was already updated by the merge operation
- if modified:
- log = 'push(m)'
- else:
- log = 'push'
- self.refresh_patch(cache_update = False, log = log)
- else:
- # we store the correctly merged files only for
- # tracking the conflict history. Note that the
- # git.merge() operations should always leave the index
- # in a valid state (i.e. only stage 0 files)
- self.refresh_patch(cache_update = False, log = 'push(c)')
- raise StackException, str(ex)
-
- return modified
+ self.refresh_patch(cache_update = False, log = 'push(m)')
def push_patch(self, name):
"""Pushes a patch on the stack
^ permalink raw reply related
* [StGit PATCH 04/13] Split Series.push_patch in two
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
The push_patch() function has a complex control flow and actually does
two different things depending on the 'empty' parameter. This patch
splits in in two functions without other code changes.
Later patches will refactor the code to simplify it.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/commands/common.py | 2 +
stgit/stack.py | 62 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index f3fa89d..eaaf5fc 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -184,7 +184,7 @@ def push_patches(patches, check_merged = False):
out.start('Pushing patch "%s"' % p)
if p in merged:
- crt_series.push_patch(p, empty = True)
+ crt_series.push_empty_patch(p)
out.done('merged upstream')
else:
modified = crt_series.push_patch(p)
diff --git a/stgit/stack.py b/stgit/stack.py
index 906e6b1..79d6cf3 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -1022,7 +1022,55 @@ class Series(PatchSet):
return merged
- def push_patch(self, name, empty = False):
+ def push_empty_patch(self, name):
+ """Pushes an empty patch on the stack
+ """
+ unapplied = self.get_unapplied()
+ assert(name in unapplied)
+
+ patch = self.get_patch(name)
+
+ head = git.get_head()
+ bottom = patch.get_bottom()
+ top = patch.get_top()
+
+ ex = None
+ modified = False
+
+ # top != bottom always since we have a commit for each patch
+ # just make an empty patch (top = bottom = HEAD). This
+ # option is useful to allow undoing already merged
+ # patches. The top is updated by refresh_patch since we
+ # need an empty commit
+ patch.set_bottom(head, backup = True)
+ patch.set_top(head, backup = True)
+ modified = True
+
+ append_string(self.__applied_file, name)
+
+ unapplied.remove(name)
+ write_strings(self.__unapplied_file, unapplied)
+
+ # head == bottom case doesn't need to refresh the patch
+ if not ex:
+ # if the merge was OK and no conflicts, just refresh the patch
+ # The GIT cache was already updated by the merge operation
+ if modified:
+ log = 'push(m)'
+ else:
+ log = 'push'
+ self.refresh_patch(cache_update = False, log = log)
+ else:
+ # we store the correctly merged files only for
+ # tracking the conflict history. Note that the
+ # git.merge() operations should always leave the index
+ # in a valid state (i.e. only stage 0 files)
+ self.refresh_patch(cache_update = False, log = 'push(c)')
+ raise StackException, str(ex)
+
+ return modified
+
+ def push_patch(self, name):
"""Pushes a patch on the stack
"""
unapplied = self.get_unapplied()
@@ -1038,15 +1086,7 @@ class Series(PatchSet):
modified = False
# top != bottom always since we have a commit for each patch
- if empty:
- # just make an empty patch (top = bottom = HEAD). This
- # option is useful to allow undoing already merged
- # patches. The top is updated by refresh_patch since we
- # need an empty commit
- patch.set_bottom(head, backup = True)
- patch.set_top(head, backup = True)
- modified = True
- elif head == bottom:
+ if head == bottom:
# reset the backup information. No need for logging
patch.set_bottom(bottom, backup = True)
patch.set_top(top, backup = True)
@@ -1079,7 +1119,7 @@ class Series(PatchSet):
write_strings(self.__unapplied_file, unapplied)
# head == bottom case doesn't need to refresh the patch
- if empty or head != bottom:
+ if head != bottom:
if not ex:
# if the merge was OK and no conflicts, just refresh the patch
# The GIT cache was already updated by the merge operation
^ permalink raw reply related
* [StGit PATCH 06/13] Refactor Series.push_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
Refactor the Series.push_patch function to make the code flow
simpler. It identifies the simple case and handles it early, and
reduces the number of if statements.
Most changes are simply indentation changes.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 87 ++++++++++++++++++++++++++++++--------------------------
1 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index f31f308..9b8ed6e 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -1054,60 +1054,65 @@ class Series(PatchSet):
head = git.get_head()
bottom = patch.get_bottom()
top = patch.get_top()
-
- ex = None
- modified = False
-
# top != bottom always since we have a commit for each patch
+
if head == bottom:
- # reset the backup information. No need for logging
+ # A fast-forward push. Just reset the backup
+ # information. No need for logging
patch.set_bottom(bottom, backup = True)
patch.set_top(top, backup = True)
git.switch(top)
- else:
- # new patch needs to be refreshed.
- # The current patch is empty after merge.
- patch.set_bottom(head, backup = True)
- patch.set_top(head, backup = True)
-
- # Try the fast applying first. If this fails, fall back to the
- # three-way merge
- if not git.apply_diff(bottom, top):
- # if git.apply_diff() fails, the patch requires a diff3
- # merge and can be reported as modified
- modified = True
-
- # merge can fail but the patch needs to be pushed
- try:
- git.merge(bottom, head, top, recursive = True)
- except git.GitException, ex:
- out.error('The merge failed during "push".',
- 'Use "refresh" after fixing the conflicts or'
- ' revert the operation with "push --undo".')
+ append_string(self.__applied_file, name)
+
+ unapplied.remove(name)
+ write_strings(self.__unapplied_file, unapplied)
+ return False
+
+ # Need to create a new commit an merge in the old patch
+ ex = None
+ modified = False
+
+ # new patch needs to be refreshed.
+ # The current patch is empty after merge.
+ patch.set_bottom(head, backup = True)
+ patch.set_top(head, backup = True)
+
+ # Try the fast applying first. If this fails, fall back to the
+ # three-way merge
+ if not git.apply_diff(bottom, top):
+ # if git.apply_diff() fails, the patch requires a diff3
+ # merge and can be reported as modified
+ modified = True
+
+ # merge can fail but the patch needs to be pushed
+ try:
+ git.merge(bottom, head, top, recursive = True)
+ except git.GitException, ex:
+ out.error('The merge failed during "push".',
+ 'Use "refresh" after fixing the conflicts or'
+ ' revert the operation with "push --undo".')
append_string(self.__applied_file, name)
unapplied.remove(name)
write_strings(self.__unapplied_file, unapplied)
- # head == bottom case doesn't need to refresh the patch
- if head != bottom:
- if not ex:
- # if the merge was OK and no conflicts, just refresh the patch
- # The GIT cache was already updated by the merge operation
- if modified:
- log = 'push(m)'
- else:
- log = 'push'
- self.refresh_patch(cache_update = False, log = log)
+ if not ex:
+ # if the merge was OK and no conflicts, just refresh the patch
+ # The GIT cache was already updated by the merge operation
+ if modified:
+ log = 'push(m)'
else:
- # we store the correctly merged files only for
- # tracking the conflict history. Note that the
- # git.merge() operations should always leave the index
- # in a valid state (i.e. only stage 0 files)
- self.refresh_patch(cache_update = False, log = 'push(c)')
- raise StackException, str(ex)
+ log = 'push'
+ self.refresh_patch(cache_update = False, log = log)
+ else:
+ # we store the correctly merged files only for
+ # tracking the conflict history. Note that the
+ # git.merge() operations should always leave the index
+ # in a valid state (i.e. only stage 0 files)
+ self.refresh_patch(cache_update = False, log = 'push(c)')
+ raise StackException, str(ex)
return modified
^ permalink raw reply related
* [StGit PATCH 07/13] Clean up Series.refresh_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
This patch does some minor simplifications of the code and updates the
documentation string of Series.refresh_patch.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index 9b8ed6e..03ce218 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -749,14 +749,12 @@ class Series(PatchSet):
committer_name = None, committer_email = None,
backup = False, sign_str = None, log = 'refresh',
notes = None):
- """Generates a new commit for the given patch
+ """Generates a new commit for the topmost patch
"""
- name = self.get_current()
- if not name:
+ patch = self.get_current_patch()
+ if not patch:
raise StackException, 'No patches applied'
- patch = self.get_patch(name)
-
descr = patch.get_description()
if not (message or descr):
edit = True
@@ -767,7 +765,7 @@ class Series(PatchSet):
if not message and edit:
descr = edit_file(self, descr.rstrip(), \
'Please edit the description for patch "%s" ' \
- 'above.' % name, show_patch)
+ 'above.' % patch.get_name(), show_patch)
if not author_name:
author_name = patch.get_authname()
^ permalink raw reply related
* [StGit PATCH 08/13] Add a 'bottom' parameter to Series.refresh_patch and use it
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
By specifying a bottom for the new patch commit, it is no longer
necessary to update the bottom of the patch before calling
refresh_patch. This ensures that the patch top always correspond to a
commit object, and the bottom to its parent.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 24 ++++++++----------------
1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index 03ce218..fd19a82 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -748,7 +748,7 @@ class Series(PatchSet):
author_date = None,
committer_name = None, committer_email = None,
backup = False, sign_str = None, log = 'refresh',
- notes = None):
+ notes = None, bottom = None):
"""Generates a new commit for the topmost patch
"""
patch = self.get_current_patch()
@@ -780,7 +780,8 @@ class Series(PatchSet):
descr = add_sign_line(descr, sign_str, committer_name, committer_email)
- bottom = patch.get_bottom()
+ if not bottom:
+ bottom = patch.get_bottom()
commit_id = git.commit(files = files,
message = descr, parents = [bottom],
@@ -1026,20 +1027,15 @@ class Series(PatchSet):
unapplied = self.get_unapplied()
assert(name in unapplied)
- patch = self.get_patch(name)
+ # patch = self.get_patch(name)
head = git.get_head()
- # The top is updated by refresh_patch since we need an empty
- # commit
- patch.set_bottom(head, backup = True)
- patch.set_top(head, backup = True)
-
append_string(self.__applied_file, name)
unapplied.remove(name)
write_strings(self.__unapplied_file, unapplied)
- self.refresh_patch(cache_update = False, log = 'push(m)')
+ self.refresh_patch(bottom = head, cache_update = False, log = 'push(m)')
def push_patch(self, name):
"""Pushes a patch on the stack
@@ -1071,11 +1067,6 @@ class Series(PatchSet):
ex = None
modified = False
- # new patch needs to be refreshed.
- # The current patch is empty after merge.
- patch.set_bottom(head, backup = True)
- patch.set_top(head, backup = True)
-
# Try the fast applying first. If this fails, fall back to the
# three-way merge
if not git.apply_diff(bottom, top):
@@ -1103,13 +1094,14 @@ class Series(PatchSet):
log = 'push(m)'
else:
log = 'push'
- self.refresh_patch(cache_update = False, log = log)
+ self.refresh_patch(bottom = head, cache_update = False, log = log)
else:
# we store the correctly merged files only for
# tracking the conflict history. Note that the
# git.merge() operations should always leave the index
# in a valid state (i.e. only stage 0 files)
- self.refresh_patch(cache_update = False, log = 'push(c)')
+ self.refresh_patch(bottom = head, cache_update = False,
+ log = 'push(c)')
raise StackException, str(ex)
return modified
^ permalink raw reply related
* [StGit PATCH 10/13] Refactor Series.new_patch
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
This shuffles some code so that the top and bottom never need to be
set to anything other than a valid commit and its parent.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index 733a241..fdff5a4 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -864,13 +864,6 @@ class Series(PatchSet):
patch = self.get_patch(name)
patch.create()
- if not bottom:
- bottom = head
- if not top:
- top = head
-
- patch.set_bottom(bottom)
- patch.set_top(top)
patch.set_description(descr)
patch.set_authname(author_name)
patch.set_authemail(author_email)
@@ -889,10 +882,15 @@ class Series(PatchSet):
set_head = True
if commit:
+ if top:
+ top_commit = git.get_commit(top)
+ else:
+ bottom = head
+ top_commit = git.get_commit(head)
+
# create a commit for the patch (may be empty if top == bottom);
# only commit on top of the current branch
assert(unapplied or bottom == head)
- top_commit = git.get_commit(top)
commit_id = git.commit(message = descr, parents = [bottom],
cache_update = False,
tree_id = top_commit.get_tree(),
@@ -903,9 +901,12 @@ class Series(PatchSet):
committer_name = committer_name,
committer_email = committer_email)
# set the patch top to the new commit
+ patch.set_bottom(bottom)
patch.set_top(commit_id)
else:
assert top != bottom
+ patch.set_bottom(bottom)
+ patch.set_top(top)
self.log_patch(patch, 'new')
^ permalink raw reply related
* [StGit PATCH 09/13] Clear up the semantics of Series.new_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
This patch adds a number of assertions to document and verify the
complex restrictions of the input parameters to the Series.new_patch
function. It also adds the requirement that 'before_existing' and
'commit' cannot be true at the same time when calling it, instead of
updating 'commit' inside the function.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/commands/uncommit.py | 1 +
stgit/stack.py | 14 ++++++++++----
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py
index 3cf2f0a..0cd0fb0 100644
--- a/stgit/commands/uncommit.py
+++ b/stgit/commands/uncommit.py
@@ -129,6 +129,7 @@ def func(parser, options, args):
name_email_date(commit.get_author())
crt_series.new_patch(patchname,
can_edit = False, before_existing = True,
+ commit = False,
top = commit_id, bottom = parent,
message = commit.get_log(),
author_name = author_name,
diff --git a/stgit/stack.py b/stgit/stack.py
index fd19a82..733a241 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -833,9 +833,16 @@ class Series(PatchSet):
author_name = None, author_email = None, author_date = None,
committer_name = None, committer_email = None,
before_existing = False):
- """Creates a new patch
+ """Creates a new patch, either pointing to an existing commit object,
+ or by creating a new commit object.
"""
+ assert commit or (top and bottom)
+ assert not before_existing or (top and bottom)
+ assert not (commit and before_existing)
+ assert (top and bottom) or (not top and not bottom)
+ assert not top or (bottom == git.get_commit(top).get_parent())
+
if name != None:
self.__patch_name_valid(name)
if self.patch_exists(name):
@@ -873,9 +880,6 @@ class Series(PatchSet):
if before_existing:
insert_string(self.__applied_file, patch.get_name())
- # no need to commit anything as the object is already
- # present (mainly used by 'uncommit')
- commit = False
elif unapplied:
patches = [patch.get_name()] + self.get_unapplied()
write_strings(self.__unapplied_file, patches)
@@ -900,6 +904,8 @@ class Series(PatchSet):
committer_email = committer_email)
# set the patch top to the new commit
patch.set_top(commit_id)
+ else:
+ assert top != bottom
self.log_patch(patch, 'new')
^ permalink raw reply related
* [StGit PATCH 11/13] Check bottom and invariants
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
This code adds some checks that the bottom is actually always the
parent of top.
It also checks that the top is the same as what the patch ref points
to.
This is only do to ensure that the next patches are correct.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index fdff5a4..d9a2e56 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -197,9 +197,15 @@ class Patch(StgitObject):
self.__update_top_ref(top)
def get_old_bottom(self):
- return self._get_field('bottom.old')
+ old_bottom = self._get_field('bottom.old')
+ old_top = self.get_old_top()
+ assert old_bottom == git.get_commit(old_top).get_parent()
+ return old_bottom
def get_bottom(self):
+ bottom = self._get_field('bottom')
+ top = self.get_top()
+ assert bottom == git.get_commit(top).get_parent()
return self._get_field('bottom')
def set_bottom(self, value, backup = False):
@@ -212,7 +218,13 @@ class Patch(StgitObject):
return self._get_field('top.old')
def get_top(self):
- return self._get_field('top')
+ top = self._get_field('top')
+ try:
+ ref = git.rev_parse(self.__top_ref)
+ except:
+ ref = None
+ assert not ref or top == ref
+ return top
def set_top(self, value, backup = False):
if backup:
@@ -220,6 +232,7 @@ class Patch(StgitObject):
self._set_field('top.old', curr)
self._set_field('top', value)
self.__update_top_ref(value)
+ self.get_bottom() # check the assert
def restore_old_boundaries(self):
bottom = self._get_field('bottom.old')
^ permalink raw reply related
* [StGit PATCH 12/13] Remove the 'bottom' field
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
The bottom is instead always calculated from the top by getting its
parent commit.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/commands/sync.py | 1 -
stgit/stack.py | 29 +++--------------------------
2 files changed, 3 insertions(+), 27 deletions(-)
diff --git a/stgit/commands/sync.py b/stgit/commands/sync.py
index 580b5bd..c18d7e0 100644
--- a/stgit/commands/sync.py
+++ b/stgit/commands/sync.py
@@ -148,7 +148,6 @@ def func(parser, options, args):
# reset the patch backup information. That's needed in case we
# undo the sync but there were no changes made
- patch.set_bottom(bottom, backup = True)
patch.set_top(top, backup = True)
# the actual merging (either from a branch or an external file)
diff --git a/stgit/stack.py b/stgit/stack.py
index d9a2e56..00b91c6 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -158,7 +158,6 @@ class Patch(StgitObject):
def create(self):
os.mkdir(self._dir())
- self.create_empty_field('bottom')
self.create_empty_field('top')
def delete(self):
@@ -197,22 +196,10 @@ class Patch(StgitObject):
self.__update_top_ref(top)
def get_old_bottom(self):
- old_bottom = self._get_field('bottom.old')
- old_top = self.get_old_top()
- assert old_bottom == git.get_commit(old_top).get_parent()
- return old_bottom
+ return git.get_commit(self.get_old_top()).get_parent()
def get_bottom(self):
- bottom = self._get_field('bottom')
- top = self.get_top()
- assert bottom == git.get_commit(top).get_parent()
- return self._get_field('bottom')
-
- def set_bottom(self, value, backup = False):
- if backup:
- curr = self._get_field('bottom')
- self._set_field('bottom.old', curr)
- self._set_field('bottom', value)
+ return git.get_commit(self.get_top()).get_parent()
def get_old_top(self):
return self._get_field('top.old')
@@ -232,14 +219,11 @@ class Patch(StgitObject):
self._set_field('top.old', curr)
self._set_field('top', value)
self.__update_top_ref(value)
- self.get_bottom() # check the assert
def restore_old_boundaries(self):
- bottom = self._get_field('bottom.old')
top = self._get_field('top.old')
- if top and bottom:
- self._set_field('bottom', bottom)
+ if top:
self._set_field('top', top)
self.__update_top_ref(top)
return True
@@ -806,7 +790,6 @@ class Series(PatchSet):
committer_name = committer_name,
committer_email = committer_email)
- patch.set_bottom(bottom, backup = backup)
patch.set_top(commit_id, backup = backup)
patch.set_description(descr)
patch.set_authname(author_name)
@@ -914,11 +897,8 @@ class Series(PatchSet):
committer_name = committer_name,
committer_email = committer_email)
# set the patch top to the new commit
- patch.set_bottom(bottom)
patch.set_top(commit_id)
else:
- assert top != bottom
- patch.set_bottom(bottom)
patch.set_top(top)
self.log_patch(patch, 'new')
@@ -972,7 +952,6 @@ class Series(PatchSet):
if head == bottom:
# reset the backup information. No logging since the
# patch hasn't changed
- patch.set_bottom(head, backup = True)
patch.set_top(top, backup = True)
else:
@@ -1000,7 +979,6 @@ class Series(PatchSet):
committer_name = committer_name,
committer_email = committer_email)
- patch.set_bottom(head, backup = True)
patch.set_top(top, backup = True)
self.log_patch(patch, 'push(f)')
@@ -1073,7 +1051,6 @@ class Series(PatchSet):
if head == bottom:
# A fast-forward push. Just reset the backup
# information. No need for logging
- patch.set_bottom(bottom, backup = True)
patch.set_top(top, backup = True)
git.switch(top)
^ permalink raw reply related
* [StGit PATCH 13/13] Remove the 'top' field
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>
The top is instead implicitly defined by the patch ref.
Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
stgit/stack.py | 26 +++++++++-----------------
1 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index 00b91c6..94d3eee 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -158,7 +158,6 @@ class Patch(StgitObject):
def create(self):
os.mkdir(self._dir())
- self.create_empty_field('top')
def delete(self):
for f in os.listdir(self._dir()):
@@ -190,11 +189,6 @@ class Patch(StgitObject):
def __update_log_ref(self, ref):
git.set_ref(self.__log_ref, ref)
- def update_top_ref(self):
- top = self.get_top()
- if top:
- self.__update_top_ref(top)
-
def get_old_bottom(self):
return git.get_commit(self.get_old_top()).get_parent()
@@ -205,26 +199,18 @@ class Patch(StgitObject):
return self._get_field('top.old')
def get_top(self):
- top = self._get_field('top')
- try:
- ref = git.rev_parse(self.__top_ref)
- except:
- ref = None
- assert not ref or top == ref
- return top
+ return git.rev_parse(self.__top_ref)
def set_top(self, value, backup = False):
if backup:
- curr = self._get_field('top')
+ curr = self.get_top()
self._set_field('top.old', curr)
- self._set_field('top', value)
self.__update_top_ref(value)
def restore_old_boundaries(self):
top = self._get_field('top.old')
if top:
- self._set_field('top', top)
self.__update_top_ref(top)
return True
else:
@@ -436,7 +422,13 @@ class Series(PatchSet):
patch = patch.strip()
os.rename(os.path.join(branch_dir, patch),
os.path.join(patch_dir, patch))
- Patch(patch, patch_dir, refs_base).update_top_ref()
+ topfield = os.path.join(patch_dir, patch, 'top')
+ if os.path.isfile(topfield):
+ top = read_string(topfield, False)
+ else:
+ top = None
+ if top:
+ git.set_ref(refs_base + '/' + patch, top)
set_format_version(1)
# Update 1 -> 2.
^ permalink raw reply related
* Re: [StGit PATCH 03/13] Moved that status function to the status command file
From: David Kågedal @ 2007-09-14 22:36 UTC (permalink / raw)
To: git
In-Reply-To: <20070914223124.7001.29374.stgit@morpheus.local>
David Kågedal <davidk@lysator.liu.se> writes:
> diff --git a/t/t0002-status.sh b/t/t0002-status.sh
> index 790b9fb..d0c31b2 100755
> --- a/t/t0002-status.sh
> +++ b/t/t0002-status.sh
> @@ -60,6 +60,14 @@ test_expect_success 'Status with an added file' '
> '
>
> cat > expected.txt <<EOF
> +foo/bar
> +EOF
> +test_expect_success 'Status with an added file and -n option' '
> + stg status -n > output.txt &&
> + diff -u expected.txt output.txt
> +'
> +
> +cat > expected.txt <<EOF
> EOF
> test_expect_success 'Status after refresh' '
> stg new -m "first patch" &&
>
Oops, that should have been in the first patch.
--
David Kågedal
^ permalink raw reply
* [PATCH 1/2] Fix builtin-fetch memory corruption by not overstepping array
From: Shawn Pearce @ 2007-09-14 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
A long time ago Junio added this line to always ensure that the
output array created by remove_duplicates() had a NULL as its
terminating node. Today none of the downstream consumers of this
array care about a NULL terminator; they only pay attention to the
size of the array (as indicated by nr_heads). In (nearly?) all
cases passing a NULL element will cause SIGSEGV failures. So this
NULL terminal is not actually necessary.
Unfortunately we cannot continue to NULL terminate the array at
this point as the array may only have been allocated large enough
to match the input of nr_heads. If there are no duplicates than
we would be trying to store NULL into heads[nr_heads] and that may
be outside of the array.
My recent series to cleanup builtin-fetch changed the allocation of
the heads array from 256 entries to exactly nr_heads thus ensuring
we were always overstepping the array and causing memory corruption.
Signed-off-by: Shawn Pearce <spearce@blink.home.spearce.org>
---
This patch is meant for the top of the db/fetch-pack series
(currently in pu) but it may actually need to be applied back on
prior versions (e.g. maint). The code affected seems to predate
builtin-fetch by quite a bit and we may just be getting lucky that
the array is always allocated large enough for this extra entry.
It almost fixes the corruption Junio saw in t3200. When combined
with the next patch (2/2) that corruption appears to be fixed on
both my Mac OS X and my Linux amd64 system.
builtin-fetch-pack.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index b0936cc..2977a94 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -654,7 +654,6 @@ static int remove_duplicates(int nr_heads, char **heads)
heads[dst] = heads[src];
dst++;
}
- heads[dst] = 0;
return dst;
}
--
1.5.3.1.921.g8c3b
^ permalink raw reply related
* [PATCH 2/2] Backup the array passed to fetch_pack so we can free items
From: Shawn O. Pearce @ 2007-09-14 22:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
fetch_pack() can call remove_duplicates() on its input array and
this will possibly overwrite an earlier entry with a later one if
there are any duplicates in the input array. In such a case the
caller here might then attempt to free an item multiple times as
it goes through its cleanup.
I also forgot to free the heads array we pass down into fetch_pack()
when I introduced the allocation of it in this function during my
builtin-fetch cleanup series. Better free it while we are here
working on related memory management fixes.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
transport.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index 0882edd..0338ed4 100644
--- a/transport.c
+++ b/transport.c
@@ -311,6 +311,7 @@ static int fetch_refs_via_pack(struct transport *transport,
{
struct git_transport_data *data = transport->data;
char **heads = xmalloc(nr_heads * sizeof(*heads));
+ char **origh = xmalloc(nr_heads * sizeof(*origh));
struct ref *refs;
char *dest = xstrdup(transport->url);
struct fetch_pack_args args;
@@ -329,11 +330,13 @@ static int fetch_refs_via_pack(struct transport *transport,
setup_fetch_pack(&args);
for (i = 0; i < nr_heads; i++)
- heads[i] = xstrdup(to_fetch[i]->name);
+ origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
refs = fetch_pack(dest, nr_heads, heads, &transport->pack_lockfile);
for (i = 0; i < nr_heads; i++)
- free(heads[i]);
+ free(origh[i]);
+ free(origh);
+ free(heads);
free_refs(refs);
free(dest);
return 0;
--
1.5.3.1.921.g8c3b
^ permalink raw reply related
* [RESEND PATCH 1/2] Fix builtin-fetch memory corruption by not overstepping array
From: Shawn O. Pearce @ 2007-09-14 22:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
A long time ago Junio added this line to always ensure that the
output array created by remove_duplicates() had a NULL as its
terminating node. Today none of the downstream consumers of this
array care about a NULL terminator; they only pay attention to the
size of the array (as indicated by nr_heads). In (nearly?) all
cases passing a NULL element will cause SIGSEGV failures. So this
NULL terminal is not actually necessary.
Unfortunately we cannot continue to NULL terminate the array at
this point as the array may only have been allocated large enough
to match the input of nr_heads. If there are no duplicates than
we would be trying to store NULL into heads[nr_heads] and that may
be outside of the array.
My recent series to cleanup builtin-fetch changed the allocation of
the heads array from 256 entries to exactly nr_heads thus ensuring
we were always overstepping the array and causing memory corruption.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
*** RESEND TO CORRECT FROM+SBO ***
Sorry Junio. I realized too late that the headers were wrong.
That's what I get for not configuring ~/.gitconfig on my amd64
Linux box. ;-)
builtin-fetch-pack.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index b0936cc..2977a94 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -654,7 +654,6 @@ static int remove_duplicates(int nr_heads, char **heads)
heads[dst] = heads[src];
dst++;
}
- heads[dst] = 0;
return dst;
}
--
1.5.3.1.921.g8c3b
^ permalink raw reply related
* Re: [RESEND PATCH 1/2] Fix builtin-fetch memory corruption by not overstepping array
From: Shawn O. Pearce @ 2007-09-14 23:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20070914225953.GC16512@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Unfortunately we cannot continue to NULL terminate the array at
> this point as the array may only have been allocated large enough
> to match the input of nr_heads. If there are no duplicates than
> we would be trying to store NULL into heads[nr_heads] and that may
> be outside of the array.
...
> diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
> index b0936cc..2977a94 100644
> --- a/builtin-fetch-pack.c
> +++ b/builtin-fetch-pack.c
> @@ -654,7 +654,6 @@ static int remove_duplicates(int nr_heads, char **heads)
> heads[dst] = heads[src];
> dst++;
> }
> - heads[dst] = 0;
> return dst;
> }
>
This fortunately does not appear to be a bug in `maint`, `master`,
or `next`. It appears to be isolated to the builtin-fetch topic.
The builtin-fetch topic changed fetch-pack to be builtin-fetch-pack
and in doing so changed things such that the heads array might not
have room for this final entry.
In maint/master/next the heads array is actually backed by the
argv passed into main by the kernel, and that does have an existing
NULL terminator, which the above "heads[dst] = 0" is simply going
to overwrite if "heads == nr_heads".
--
Shawn.
^ permalink raw reply
* Re: [PATCH] git-commit: Allow partial commit of file removal.
From: Junio C Hamano @ 2007-09-14 23:34 UTC (permalink / raw)
To: Gerrit Pape; +Cc: Martin Koegler, git
In-Reply-To: <7vzlzqieko.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Gerrit Pape <pape@smarden.org> writes:
>
>> On Wed, Sep 12, 2007 at 04:04:22PM -0700, Junio C Hamano wrote:
>>> * In my earlier reply to Gerrit, I hinted that we need to
>>> update the pathspec semantics in ls-tree to properly fix this
>>> issue. I cheated here and have ls-files apply its pathspec
>>> semantics to the entries from HEAD as well.
>>
>> This fixes the problem reported through http://bugs.debian.org/437817
>> just fine, thanks. Is this an interims-fix, or should the new option
>> be documented?
>
> I honestly am not convinced it is the right fix. It has a few
> holes in the logic.
>
> Most notably, I think "git rm --cached A; git commit A" would
> not work.
I managed to convince myself that not committing the removal of
A in that case is a _good_ thing, unless somebody comes up with
a good counterexample this will most likely go to 'master' over
the weekend and then to 'maint'.
Any partial commit "git commit <paths>..." is saying:
I might have changed stuff in the index and also have
changes in the working tree. But I do not care about
the changes between HEAD and the index. Honestly, I do
not understand the index at all, and I do not care about
what I staged earlier to the named paths either. Take
the current state of these paths from my work tree and
make a commit relative to the HEAD.
So, if you do:
$ edit new-file old-file
$ rm gone-file
$ git rm missing-file
$ git rm --cached disappeared-file
$ git add new-file ;# was not in HEAD
$ edit new-file old-file
Then:
$ git commit new-file old-file
honors what is in the work tree and picks up the later edits,
largely ignoring the changes to the index.
Removal should work the same way to be consistent.
$ git commit gone-file
$ git commit missing-file
$ git commit disappeared-file
should remove the former two but leave the last one alone, as
you _do_ still have disappeared-file in the work tree.
As it happens, the version I sent will error out on the last one
(disappeared-file case). I think the following patch on top of
it "fixes" it, by adding it back to the index and including its
contents in the resulting commit.
---
diff --git a/git-commit.sh b/git-commit.sh
index 5ea3fd0..bb113e8 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -404,7 +404,7 @@ t,)
(
GIT_INDEX_FILE="$NEXT_INDEX"
export GIT_INDEX_FILE
- git update-index --remove --stdin
+ git update-index --add --remove --stdin
) || exit
;;
esac
^ permalink raw reply related
* Re: [RESEND PATCH 1/2] Fix builtin-fetch memory corruption by not overstepping array
From: Junio C Hamano @ 2007-09-14 23:43 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070914225953.GC16512@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> *** RESEND TO CORRECT FROM+SBO ***
>
> Sorry Junio. I realized too late that the headers were wrong.
> That's what I get for not configuring ~/.gitconfig on my amd64
> Linux box. ;-)
Actually this might have been a good thing, according to Linus.
It is an evidence that you built and tested that change on the
"blink" box not your usual box.
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Johannes Schindelin @ 2007-09-14 23:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wd1d0le.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 14 Sep 2007, Junio C Hamano wrote:
> * cr/reset (Fri Sep 14 01:19:30 2007 -0700) 5 commits
> + Simplify cache API
> + An additional test for "git-reset -- path"
> + Make "git reset" a builtin.
> + Move make_cache_entry() from merge-recursive.c into read-cache.c
> + Add tests for documented features of "git reset".
>
> I found "git reset commit paths..." had problem in this series,
> which was why jc/cachetree is merged into this topic to fix it.
> Hopefully we can put this in 'master' soon, after giving it
> another and final round of eyeballing.
I think this is my bug. The initial reset-with-paths functionality in the
builtin reset came out of my feather... Sorry!
Ciao,
Dscho
^ permalink raw reply
* Re: Data Integrity & un-Commited Branches
From: Brian Scott Dobrovodsky @ 2007-09-15 0:40 UTC (permalink / raw)
To: git
In-Reply-To: <7vk5qtd3le.fsf@gitster.siamese.dyndns.org>
It was a misunderstanding of Git's work flow. By switching from 'an
un-committed demo' to a previously committed master: I was expecting
Git to give me the content last commited to master while at the same
time preserving(without having to commit) the changes made in demo.
Intuitively, this is how I expected Git to function.
Indeed, I read through the Crash Courses: 'Git for everyone' & 'Git
for SVN users'.
--
Brian Scott Dobrovodsky
^ permalink raw reply
* Mailing patch series'
From: Russ Brown @ 2007-09-15 1:06 UTC (permalink / raw)
To: git
Hi,
I've noticed the method of mailing series' of patches on this mailing
list, and I'd like to know a little more about how it's done, as I'm
considering how well it might work for us as a workflow and review process.
Particularly, where does the series of patches come from? Is this a
usage scenario for stacked git (something else I don't fully understand
the rationale behind as yet)? I'm imagining the developer has a local
branch to which he commits his changes, and then rebases resulting in
his branches being at the end of the local branch. How are they then
extracted and mailed out?
My next question, is supposing that the patches are reviewed and changes
suggested. How does the developer then go about amending, say the second
patch and getting the subsequent ones rebased off that? I'm assuming
there's a nice clever way of doing it that doesn't involve manually
messing with individual patch files etc.
Please excuse my ignorance: I'm still getting my head around this. Once
I do that I'm usually away and happy, but it takes a while for that
'click' moment to hit me sometimes :)
Thanks.
--
Russ
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox