Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Junio C Hamano
Junio C Hamano  writes:

>> I agree that a new value "mergeInstead" or something should be
>> invented when/if different workflows want a looser semantics.
>> People would rely not only on "being able to push when clean" but
>> also on "being safely prevented from pushing when not" (and that is
>> where my earlier comment to test both sides comes from).  Loosening
>> the check to an existing "updateInstead" would break users who have
>> been using updateInstead.
>
> And thinking about the names again, I have a feeling that
> updateInstead and mergeInstead are both probably misnomer.

Let me take this part back.  After all, I do not think I would
design the mechanism to implement an alternative logic that decides
when it is safe to allow the update of the ref and to reflect the
changes to the working tree, and that actually does the checkout to
the working tree by using a new value like mergeInstead.  So we
would only need a single name, and updateInstead is not too bad.

The word "update" is so heavily loaded in the context of accepting a
push (i.e. it is unclear what update we are talking about---updating
the ref that we normally refuse to update?  updating the index?
updating the working tree?  Some combination of them?), so as a
single word, "checkoutInstead" may probably be a better one, though.
Upon hearing "checkout", by definition anybody would know that we
are involving the working tree.

The mechanism I would employ when doing an alternative logic,
possibly looser one but does not necessarily so, would be to have a
hook script "push-to-checkout".  When denyCurrentBranch is set to
updateInstead, if there is no such hook, the "working tree has to be
absolutely clean and we would do a 'read-tree -m -u $old $new'
(which is the same as 'reset --hard $new' under the precondition)"
you implemented will be used as the "logic that decides when it is
safe, and that does the checkout to the working tree".  When the
"push-to-checkout" hook exists, however, we just invoke that hook
with $new as argument, and it can decide when it is safe in whatever
way it chooses to, and it can checkout the $new to the working tree
in whatever way it wants.  The users of "mergeInstead" (now a dead
and unnecessary name) mode would just have a single-liner

#!/bin/sh
git reset --keep "$1" --

in there, as this single command would both decide when it is safe
and does the safe (according to its own definition) updating of the
working tree.

In your other example (not the "deploy to live website" one) of
unidirectional SSH connection, you would be able to connect from
machine A to machine B but not the other way, so while sitting on
machine A you would typically have one SSH session to have an
interactive shell session running on machine B.  You may have local
modification on machine B but your changes to history on machine A
cannot be pulled, so you would emulate it by pushing from A into B.
In such a case, unlike the "live website" example, it would be
useful to loosen the condition even more than "reset --keep" (which
is an equivalent of "checkout -B $current_branch $new_commit").

In such a case, what you want to do is to simulate "git pull" that
could conflict and give you a chance to resolve with a push in the
reverse direction.  You want to run an equivalent of the same
"checkout -B" command but with the "-m" option when accepting such a
push.

There are other definitions of what is safe and how update should
happen depending on the user, and such a logic can be placed in the
push-to-checkout hook without harming other users.



--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread David Aguilar
On Sun, Nov 09, 2014 at 09:21:49AM -0800, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > So just to be clear, the behavior we want is that:
> >
> >   echo foo >some-new-path
> >   git add some-new-path
> >   git checkout HEAD -- .
> >
> > will delete some-new-path (whereas the current code turns it into an
> > untracked file).
> 
> With the updated semantics proposed in the old thread, yes, that is
> what should happen.
> 
> >   git checkout HEAD -- some-new-path
> >
> > do in that case?
> 
> Likewise.  And if some-new-path were a directory, with existing path
> O and new path N both in the index but only the former in HEAD, the
> operation would revert some-new-path/O to that of HEAD and remove
> some-new-path/N.  That is the only logical thing we could do if we
> were to take the updated sematics.
> 
> That is one of the reasons why I am not 100% convinced that the
> proposed updated semantics is better, even though I was fairly
> positive in the old discussion and also I kept the topic in the
> "leftover bits" list.  The above command is a fairly common way to
> say "I started refactoring the existing path some-path/O and
> sprinkled its original contents spread into new files A, B and C in
> the same directory.  Now I no longer have O in the working tree, but
> let me double check by grabbing it out of the state recoded in the
> commit".  You expect that "git checkout HEAD -- some-path" would not
> lose A, B or C, knowing "some-path" only had O.  That expectation
> would even be stronger if you are used to the current semantics, but
> that is something we could fix, if we decide that the proposed
> updated semantics is better, with a careful transition plan.
> 
> It might be less risky if the updated semantics were to make the
> paths that are originally in the index but not in $tree untracked
> (as opposed to "reset --hard" emulation where they will be lost)
> unless they need to be removed to make room for D/F conflict issues,
> but I haven't thought it through.


Git has always been really careful to not lose data.

One way to avoid the problem of changing existing semantics is
to make the new semantics accessible behind a flag, e.g.
"git checkout --hard HEAD -- some-new-path".
-- 
David
--
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: [PATCH] allow TTY tests to run under recent Mac OS

2014-11-13 Thread Torsten Bögershausen

On 11/13/2014 11:01 PM, Mike Blume wrote:

listed bug doesn't reproduce on Mac OS Yosemite. For now, just enable
TTY on Yosemite and higher

Signed-off-by: Mike Blume 
---
  t/lib-terminal.sh | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 5184549..1311ce0 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -29,7 +29,10 @@ test_lazy_prereq TTY '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
-   test "$(uname -s)" != Darwin &&
+   # Under Mac OS X 10.10.1 and Perl 5.18.2, this problem
+   # appears to be gone.
+   #
+   [[ test "$(uname -s)" != Darwin || test "$(uname -r | cut -d. -f1") -ge 14 ]] 
&&

This seems to be bash syntax (the "[[" can (and should) be easily avoided)

Another question:
Is this related to perl or Mac OS ?
The commit message is indicating that the combination of Mac OS 10.10.1 
and perl 5.18.2

is working, but the code does not reflect this.
Does it make sense to test for the perl version in the code?
Or is it OK to mention

 Under Mac OS X 10.10.1 which ships with Perl 5.18.2, this problem



  
  	perl "$TEST_DIRECTORY"/test-terminal.perl \

sh -c "test -t 1 && test -t 2"


--
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


[PATCH] gc: support temporarily preserving garbage

2014-11-13 Thread Brodie Rao
This patch adds a gc.garbageexpire setting that, when not set to "now",
makes gc (and prune, prune-packed, and repack) move garbage into a
temporary garbage directory instead of deleting it immediately. The
garbage directory is then cleared out based on gc.garbageexpire.

The motivation for this setting is to work around various NFS servers
not supporting delete-on-last-close semantics between NFS clients.
Without proper support for that, gc could potentially delete objects
and packs that are in use by git processes on other NFS clients. If
another git process has a deleted pack file mmap()ed, it could crash
with a SIGBUS error on Linux.

Signed-off-by: Brodie Rao 
---
 .gitignore |  1 +
 Documentation/config.txt   | 20 +
 Documentation/git-gc.txt   |  7 
 Documentation/git-prune-garbage.txt| 55 
 Documentation/git-prune-packed.txt |  9 
 Documentation/git-prune.txt|  9 
 Documentation/git-repack.txt   |  6 +++
 Documentation/git.txt  |  6 +++
 Makefile   |  2 +
 builtin.h  |  1 +
 builtin/gc.c   | 20 +
 builtin/prune-garbage.c| 77 ++
 builtin/prune-packed.c |  3 +-
 builtin/prune.c|  5 ++-
 builtin/repack.c   |  7 ++--
 cache.h|  2 +
 command-list.txt   |  1 +
 contrib/completion/git-completion.bash |  2 +
 environment.c  | 12 +-
 gc.c   | 60 ++
 gc.h   | 16 +++
 git.c  |  1 +
 t/t6502-gc-garbage-expire.sh   | 60 ++
 23 files changed, 375 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/git-prune-garbage.txt
 create mode 100644 builtin/prune-garbage.c
 create mode 100644 gc.c
 create mode 100644 gc.h
 create mode 100755 t/t6502-gc-garbage-expire.sh

diff --git a/.gitignore b/.gitignore
index a052419..a9a4e30 100644
--- a/.gitignore
+++ b/.gitignore
@@ -107,6 +107,7 @@
 /git-parse-remote
 /git-patch-id
 /git-prune
+/git-prune-garbage
 /git-prune-packed
 /git-pull
 /git-push
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9220725..0106d8f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1213,6 +1213,26 @@ gc.autodetach::
Make `git gc --auto` return immediately and run in background
if the system supports it. Default is true.
 
+gc.garbageexpire::
+   When 'git gc' is run, objects and packs that are pruned are
+   immediately deleted from the file system. This setting can be
+   overridden to move pruned objects and packs to the garbage
+   directory. That leftover garbage will be deleted after the
+   specified grace period. The default value is "now", meaning
+   garbage is deleted immediately.
++
+Setting this to something other than "now" (e.g., "1.day.ago") can help
+work around issues with NFS servers that don't support
+delete-on-last-close semantics between NFS clients. 'git gc' will not
+unlink files immediately, so a git process on another NFS client that
+might be reading a garbage collected file will not crash.
++
+Note that this setting can cause the repository's size to increase as
+garbage collection passes are made. Care should be taken to make sure
+the grace period isn't too long. A grace period of one day might be
+reasonable if you make the assumption that your git processes over NFS
+won't run longer or have files open longer than one day.
+
 gc.packrefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 273c466..f90dc0a 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -131,6 +131,12 @@ The optional configuration variable 'gc.pruneExpire' 
controls how old
 the unreferenced loose objects have to be before they are pruned.  The
 default is "2 weeks ago".
 
+The optional configurable variable 'gc.garbageexpire' controls how
+pruned objects and packs are deleted. This can be overridden to move
+pruned objects and packs to the garbage directory. That leftover garbage
+will be deleted after the specified grace period. The default value is
+"now", meaning garbage is deleted immediately.
+
 
 Notes
 -
@@ -156,6 +162,7 @@ linkgit:githooks[5] for more information.
 SEE ALSO
 
 linkgit:git-prune[1]
+linkgit:git-prune-garbage[1]
 linkgit:git-reflog[1]
 linkgit:git-repack[1]
 linkgit:git-rerere[1]
diff --git a/Documentation/git-prune-garbage.txt 
b/Documentation/git-prune-garbage.txt
new file mode 100644
index 000..ff130e0
--- /dev/null
+++ b/Documentation/git

What's cooking in git.git (Nov 2014, #03; Thu, 13)

2014-11-13 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Will tag v2.2.0-rc2 tomorrow and v2.2.0 final hopefully late next
week.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* mh/doc-remote-helper-xref (2014-11-11) 1 commit
 - doc: add some crossrefs between manual pages

 Will merge to 'next'.


* ta/tutorial-modernize (2014-11-11) 1 commit
 - gittutorial.txt: remove reference to ancient Git version

 Will merge to 'next'.


* jk/approxidate-avoid-y-d-m-over-future-dates (2014-11-13) 2 commits
 - approxidate: allow ISO-like dates far in the future
 - pass TIME_DATE_NOW to approxidate future-check


* jk/checkout-from-tree (2014-11-13) 1 commit
 - checkout $tree: do not throw away unchanged index entries

 Will merge to 'next'.


* mb/enable-lib-terminal-test-on-newer-darwin (2014-11-13) 1 commit
 - allow TTY tests to run under recent Mac OS

 Will merge to 'next'.


* sn/tutorial-status-output-example (2014-11-13) 1 commit
 - gittutorial: fix output of 'git status'

 Will merge to 'next'.


* sv/get-builtin (2014-11-13) 1 commit
 - builtin: move builtin retrieval to get_builtin()

 Will merge to 'next'.


* sv/submitting-final-patch (2014-11-13) 1 commit
 - SubmittingPatches: final submission is To: maintainer and CC: list

 Will merge to 'next'.


* tb/no-relative-file-url (2014-11-13) 1 commit
 - t5705: the file:// URL should be absolute

 Will merge to 'next'.

--
[Stalled]

* je/quiltimport-no-fuzz (2014-10-21) 2 commits
 - git-quiltimport: flip the default not to allow fuzz
 - git-quiltimport.sh: allow declining fuzz with --exact option

 "quiltimport" drove "git apply" always with -C1 option to reduce
 context of the patch in order to give more chance to somewhat stale
 patches to apply.  Add an "--exact" option to disable, and also
 "-C$n" option to customize this behaviour.  The top patch
 optionally flips the default to "--exact".

 Waiting for an Ack.


* jc/push-cert-hmac-optim (2014-09-25) 2 commits
 - receive-pack: truncate hmac early and convert only necessary bytes
 - sha1_to_hex: split out "hex-format n bytes" helper and use it

 This is "we could do this if we wanted to", not "we measured and it
 improves performance critical codepath".

 Will perhaps drop.


* nd/multiple-work-trees (2014-09-27) 32 commits
 . t2025: add a test to make sure grafts is working from a linked checkout
 . checkout: don't require a work tree when checking out into a new one
 . git_path(): keep "info/sparse-checkout" per work-tree
 . count-objects: report unused files in $GIT_DIR/worktrees/...
 . gc: support prune --worktrees
 . gc: factor out gc.pruneexpire parsing code
 . gc: style change -- no SP before closing parenthesis
 . checkout: clean up half-prepared directories in --to mode
 . checkout: reject if the branch is already checked out elsewhere
 . prune: strategies for linked checkouts
 . checkout: support checking out into a new working directory
 . use new wrapper write_file() for simple file writing
 . wrapper.c: wrapper to open a file, fprintf then close
 . setup.c: support multi-checkout repo setup
 . setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()
 . setup.c: convert check_repository_format_gently to use strbuf
 . setup.c: detect $GIT_COMMON_DIR in is_git_directory()
 . setup.c: convert is_git_directory() to use strbuf
 . git-stash: avoid hardcoding $GIT_DIR/logs/
 . *.sh: avoid hardcoding $GIT_DIR/hooks/...
 . git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects
 . $GIT_COMMON_DIR: a new environment variable
 . commit: use SEQ_DIR instead of hardcoding "sequencer"
 . fast-import: use git_path() for accessing .git dir instead of get_git_dir()
 . reflog: avoid constructing .lock path with git_path
 . *.sh: respect $GIT_INDEX_FILE
 . git_path(): be aware of file relocation in $GIT_DIR
 . path.c: group git_path(), git_pathdup() and strbuf_git_path() together
 . path.c: rename vsnpath() to do_git_path()
 . git_snpath(): retire and replace with strbuf_git_path()
 . path.c: make get_pathname() call sites return const char *
 . path.c: make get_pathname() return strbuf instead of static buffer

 A replacement for contrib/workdir/git-new-workdir that does not
 rely on symbolic links and make sharing of objects and refs safer
 by making the borrowee and borrowers aware of each other.

 A few tests need some tweaks for MinGW ($gmane/{257756,257757}).
 Conflicts with rs/ref-transaction so ejected for now, waiting for a
 reroll.


* mt/patch-id-stable (2014-06-10) 1 commit
 - patch-id: change default to stable

 Teaches "git patch-id" to compute the patch ID that does not change
 when the files in a single patch is reordered. As this new algorithm
 is 

[PATCH 1/1] gitk: Add support for --author-date-order

2014-11-13 Thread lennart spitzner
gitk previously allowed sort-by-committer-date.
Now also allow sort-by-author-date.

Separate the three options for ordering in a new line in the
view-config-dialogue, using radio buttons.

Signed-off-by: Lennart Spitzner 
---
Found this useful for displaying a history with a rebase-heavy workflow,
where the committer-timestamps were largely different from
author-timestamps; sort-by-author-date produces a much simpler graph.

I do not really know much tcl, so review closely :p

 gitk | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/gitk b/gitk
index 78358a7..c176b79 100755
--- a/gitk
+++ b/gitk
@@ -155,11 +155,12 @@ proc unmerged_files {files} {
 }
  proc parseviewargs {n arglist} {
-global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
+global vdatemode vauthordatemode vmergeonly vflags vdflags vrevs vfiltered 
vorigargs env
 global vinlinediff
 global worddiff git_version
  set vdatemode($n) 0
+set vauthordatemode($n) 0
 set vmergeonly($n) 0
 set vinlinediff($n) 0
 set glflags {}
@@ -185,6 +186,12 @@ proc parseviewargs {n arglist} {
set origargs [lreplace $origargs $i $i]
incr i -1
}
+   "--author-date-order" {
+   set vauthordatemode($n) 1
+   # remove from origargs in case we hit an unknown option
+   set origargs [lreplace $origargs $i $i]
+   incr i -1
+   }
"-[puabwcrRBMC]" -
"--no-renames" - "--full-index" - "--binary" - "--abbrev=*" -
"--find-copies-harder" - "-l*" - "--ext-diff" - "--no-ext-diff" -
@@ -690,17 +697,21 @@ proc seeds {v} {
 }
  proc newvarc {view id} {
-global varcid varctok parents children vdatemode
+global varcid varctok parents children vdatemode vauthordatemode
 global vupptr vdownptr vleftptr vbackptr varcrow varcix varcstart
 global commitdata commitinfo vseedcount varccommits vlastins
  set a [llength $varctok($view)]
 set vid $view,$id
-if {[llength $children($vid)] == 0 || $vdatemode($view)} {
+if {[llength $children($vid)] == 0 || $vdatemode($view) || 
$vauthordatemode($view)} {
if {![info exists commitinfo($id)]} {
parsecommit $id $commitdata($id) 1
}
-   set cdate [lindex [lindex $commitinfo($id) 4] 0]
+   if {$vauthordatemode($view)} {
+   set cdate [lindex [lindex $commitinfo($id) 2] 0]
+   } else {
+   set cdate [lindex [lindex $commitinfo($id) 4] 0]
+   }
if {![string is integer -strict $cdate]} {
set cdate 0
}
@@ -800,7 +811,7 @@ proc splitvarc {p v} {
  proc renumbervarc {a v} {
 global parents children varctok varcstart varccommits
-global vupptr vdownptr vleftptr vbackptr vlastins varcid vtokmod vdatemode
+global vupptr vdownptr vleftptr vbackptr vlastins varcid vtokmod vdatemode 
vauthordatemode
  set t1 [clock clicks -milliseconds]
 set todo {}
@@ -836,7 +847,7 @@ proc renumbervarc {a v} {
  $children($v,$id)]
}
set oldtok [lindex $varctok($v) $a]
-   if {!$vdatemode($v)} {
+   if {!($vdatemode($v) || $vauthordatemode($v))} {
set tok {}
} else {
set tok $oldtok
@@ -1411,7 +1422,7 @@ proc check_interest {id scripts} {
  proc getcommitlines {fd inst view updating}  {
 global cmitlisted leftover
-global commitidx commitdata vdatemode
+global commitidx commitdata vdatemode vauthordatemode
 global parents children curview hlview
 global idpending ordertok
 global varccommits varcid varctok vtokmod vfilelimit vshortids
@@ -1553,7 +1564,7 @@ proc getcommitlines {fd inst view updating}  {
} elseif {$a == 0 && [llength $children($vid)] == 1} {
set k [lindex $children($vid) 0]
if {[llength $parents($view,$k)] == 1 &&
-   (!$vdatemode($view) ||
+   (!($vdatemode($view) || $vauthordatemode($view)) ||
 $varcid($view,$k) == [llength $varctok($view)] - 1)} {
set a $varcid($view,$k)
}
@@ -3987,8 +3998,11 @@ set known_view_options {
 {limit_lbl l+  {}   {mc "Limit and/or skip a number of 
revisions (positive integer):"}}
 {limit t10  *. "--max-count=*"  {mc "Number to show:"}}
 {skip  t10  .  "--skip=*"   {mc "Number to skip:"}}
+{order_l   l+  {}   {mc "Ordering:"}}
+{order_s   r0   .  {}   {mc "Default order"}}
+{order_c   r1   .  {"--date-order" "-d"}  {mc "Strictly sort by 
(committer) date"}}
+{order_a   r2   .  "--author-date-order"  {mc "Strictly sort by author 
date"}}
 {misc_lbl  l+  {}   {mc "Miscellaneous options:"}}
-{dorderb*. {"--date-order" "-d"}  {mc "Strictly sort by date"}}
 {lrightb.  "--left-right"   {mc "Mark branch sides

Re: [PATCH] allow TTY tests to run under recent Mac OS

2014-11-13 Thread Michael Blume
ah, sorry, didn't realize those were bash-only

On Thu, Nov 13, 2014 at 2:21 PM, Junio C Hamano  wrote:
> Mike Blume  writes:
>
>> listed bug doesn't reproduce on Mac OS Yosemite. For now, just enable
>> TTY on Yosemite and higher
>>
>> Signed-off-by: Mike Blume 
>> ---
>>  t/lib-terminal.sh | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
>> index 5184549..1311ce0 100644
>> --- a/t/lib-terminal.sh
>> +++ b/t/lib-terminal.sh
>> @@ -29,7 +29,10 @@ test_lazy_prereq TTY '
>>   # After 2000 iterations or so it hangs.
>>   # https://rt.cpan.org/Ticket/Display.html?id=65692
>>   #
>> - test "$(uname -s)" != Darwin &&
>> + # Under Mac OS X 10.10.1 and Perl 5.18.2, this problem
>> + # appears to be gone.
>> + #
>> + [[ test "$(uname -s)" != Darwin || test "$(uname -r | cut -d. -f1") 
>> -ge 14 ]] &&
>
> This is designed to be a plain vanilla POSIX shell script.  Please
> avoid these double brackets.
>
>>
>>   perl "$TEST_DIRECTORY"/test-terminal.perl \
>>   sh -c "test -t 1 && test -t 2"
--
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


[PATCH v2] allow TTY tests to run under recent Mac OS

2014-11-13 Thread Mike Blume
listed bug doesn't reproduce on Mac OS Yosemite. For now, just enable
TTY on Yosemite and higher

Signed-off-by: Mike Blume 
Improved-by: Junio C Hamano 
---
 t/lib-terminal.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 5184549..6395a34 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -29,7 +29,10 @@ test_lazy_prereq TTY '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
-   test "$(uname -s)" != Darwin &&
+   # Under Mac OS X 10.10.1 and Perl 5.18.2, this problem
+   # appears to be gone.
+   #
+   test "$(uname -s)" != Darwin || test "$(uname -r | cut -d. -f1)" -ge 14 
&&
 
perl "$TEST_DIRECTORY"/test-terminal.perl \
sh -c "test -t 1 && test -t 2"
-- 
2.2.0.rc1.197.g60bf093

--
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: [PATCH 2/2] approxidate: allow ISO-like dates far in the future

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Nov 13, 2014 at 01:11:46PM -0800, Junio C Hamano wrote:
>
>> Jeff King  writes:
>> 
>> >if (c != '.' &&
>> > -  is_date(num3, num, num2, refuse_future, now, tm))
>> > +  is_date(num3, num, num2, refuse_future, now, tm, 0))
>> >break;
>> 
>> Doesn't the new argument '0', which is "allow-future", look somewhat
>> strange when we are already passing refuse_future?
>
> To be honest, I had trouble figuring out what the name "refuse_future"
> really meant. We do skip the future check, but it also means that
> is_date will munge the "struct tm" directly, even if we do not find a
> valid date. That worried me a bit.

Ah, OK.  That worries me, too, now you mention it.  I just didn't
see it myself ;-)
>
> But yeah, in theory, the callers I wanted to tweak can just pass in a
> NULL refuse_future.
>
> -Peff
--
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: [PATCH] allow TTY tests to run under recent Mac OS

2014-11-13 Thread Junio C Hamano
Mike Blume  writes:

> listed bug doesn't reproduce on Mac OS Yosemite. For now, just enable
> TTY on Yosemite and higher
>
> Signed-off-by: Mike Blume 
> ---
>  t/lib-terminal.sh | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
> index 5184549..1311ce0 100644
> --- a/t/lib-terminal.sh
> +++ b/t/lib-terminal.sh
> @@ -29,7 +29,10 @@ test_lazy_prereq TTY '
>   # After 2000 iterations or so it hangs.
>   # https://rt.cpan.org/Ticket/Display.html?id=65692
>   #
> - test "$(uname -s)" != Darwin &&
> + # Under Mac OS X 10.10.1 and Perl 5.18.2, this problem
> + # appears to be gone.
> + #
> + [[ test "$(uname -s)" != Darwin || test "$(uname -r | cut -d. -f1") -ge 
> 14 ]] &&

This is designed to be a plain vanilla POSIX shell script.  Please
avoid these double brackets.

>  
>   perl "$TEST_DIRECTORY"/test-terminal.perl \
>   sh -c "test -t 1 && test -t 2"
--
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


[PATCH] allow TTY tests to run under recent Mac OS

2014-11-13 Thread Mike Blume
listed bug doesn't reproduce on Mac OS Yosemite. For now, just enable
TTY on Yosemite and higher

Signed-off-by: Mike Blume 
---
 t/lib-terminal.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 5184549..1311ce0 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -29,7 +29,10 @@ test_lazy_prereq TTY '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
-   test "$(uname -s)" != Darwin &&
+   # Under Mac OS X 10.10.1 and Perl 5.18.2, this problem
+   # appears to be gone.
+   #
+   [[ test "$(uname -s)" != Darwin || test "$(uname -r | cut -d. -f1") -ge 
14 ]] &&
 
perl "$TEST_DIRECTORY"/test-terminal.perl \
sh -c "test -t 1 && test -t 2"
-- 
2.2.0.rc1.197.g60bf093

--
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 archiving only branch work

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> I agree they are technically orthogonal, but I cannot think of a case
> where I have ever generated actual _pathspecs_, which might have
> wildcards, and needed to use "-z". The point of using "-z" is that you
> do not know what crap you are feeding.

You do not have to generate, i.e. you should be allowed to do this:

$ git cmd --stdin -z http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] approxidate: allow ISO-like dates far in the future

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 04:36:47PM -0500, Jeff King wrote:

> On Thu, Nov 13, 2014 at 01:11:46PM -0800, Junio C Hamano wrote:
> 
> > Jeff King  writes:
> > 
> > >   if (c != '.' &&
> > > - is_date(num3, num, num2, refuse_future, now, tm))
> > > + is_date(num3, num, num2, refuse_future, now, tm, 0))
> > >   break;
> > 
> > Doesn't the new argument '0', which is "allow-future", look somewhat
> > strange when we are already passing refuse_future?
> 
> To be honest, I had trouble figuring out what the name "refuse_future"
> really meant. We do skip the future check, but it also means that
> is_date will munge the "struct tm" directly, even if we do not find a
> valid date. That worried me a bit.
> 
> But yeah, in theory, the callers I wanted to tweak can just pass in a
> NULL refuse_future.

So here's what the patch looks like just using refuse_future.

It's definitely nicer to read, and it passes the tests. But I am still
concerned there is some unknown case that is impacted by us half-filling
out the tm_mon and tm_mday fields of the "struct tm" in the first half
of is_date.

-- >8 --
Subject: approxidate: allow ISO-like dates far in the future

When we are parsing approxidate strings and we find three
numbers separate by one of ":/-.", we guess that it may be a
date. We feed the numbers to match_multi_number, which
checks whether it makes sense as a date in various orderings
(e.g., dd/mm/yy or mm/dd/yy, etc).

One of the checks we do is to see whether it is a date more
than 10 days in the future. This was added in 38035cf (date
parsing: be friendlier to our European friends.,
2006-04-05), and lets us guess that if it is currently April
2014, then "10/03/2014" is probably March 10th, not October
3rd.

This has a downside, though; if you want to be overly
generous with your "--until" date specification, we may
wrongly parse "2014-12-01" as "2014-01-12" (because the
latter is an in-the-past date). If the year is a future year
(i.e., both are future dates), it gets even weirder. Due to
the vagaries of approxidate, months _after_ the current date
(no matter the year) get flipped, but ones before do not.

This patch drops the "in the future" check for dates of this
form, letting us treat them always as -mm-dd, even if
they are in the future. This does not affect the normal
dd/mm/ versus mm/dd/ lookup, because this code path
only kicks in when the first number is greater than 70
(i.e., it must be a year, and cannot be either a date or a
month).

The one possible casualty is that "-dd-mm" is less
likely to be chosen over "-mm-dd". That's probably OK,
though because:

  1. The difference happens only when the date is in the
 future. Already we prefer -mm-dd for dates in the
 past.

  2. It's unclear whether anybody even uses -dd-mm
 regularly. It does not appear in lists of common date
 formats in Wikipedia[1,2].

  3. Even if (2) is wrong, it is better to prefer ISO-like
 dates, as that is consistent with what we use elsewhere
 in git.

[1] http://en.wikipedia.org/wiki/Date_and_time_representation_by_country
[2] http://en.wikipedia.org/wiki/Calendar_date

Signed-off-by: Jeff King 
---
 date.c  | 4 ++--
 t/t0006-date.sh | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/date.c b/date.c
index e1a4d49..3eba2df 100644
--- a/date.c
+++ b/date.c
@@ -441,10 +441,10 @@ static int match_multi_number(unsigned long num, char c, 
const char *date,
 
if (num > 70) {
/* -mm-dd? */
-   if (is_date(num, num2, num3, refuse_future, now, tm))
+   if (is_date(num, num2, num3, NULL, now, tm))
break;
/* -dd-mm? */
-   if (is_date(num, num3, num2, refuse_future, now, tm))
+   if (is_date(num, num3, num2, NULL, now, tm))
break;
}
/* Our eastern European friends say dd.mm.yy[yy]
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index e53cf6d..fac0986 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -82,4 +82,7 @@ check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
 check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
 check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
 
+check_approxidate '2008-12-01' '2008-12-01 19:20:00'
+check_approxidate '2009-12-01' '2009-12-01 19:20:00'
+
 test_done
-- 
2.1.2.596.g7379948

--
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 archiving only branch work

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 01:36:48PM -0800, Junio C Hamano wrote:

> > I agree it is probably OK in practice and for the OP's question, but it
> > is nice to have "-z" variants so you do not have to worry about quoting
> > at all. I'd argue that a "--stdin -z" should probably also accept raw
> > filenames, not pathspecs, too (so you do not have to use
> > "--literal-pathspecs" elsewhere).
> 
> I agree "--stdin -z" is a good thing but what makes you think that
> the producer of the data is _always_ walking the directory hierarchy
> and showing the pathnames it sees?  I think use of literal-pathspecs
> should not be tied to the use of either --stdin or -z.

I agree they are technically orthogonal, but I cannot think of a case
where I have ever generated actual _pathspecs_, which might have
wildcards, and needed to use "-z". The point of using "-z" is that you
do not know what crap you are feeding.

Normally I'm in favor of keeping things as flexible as possible, but it
seems very likely that somebody would forget pathspecs in such a case
(the OP did in his example, and I know I have many times in the past).
I don't feel too strongly about it, though.

-Peff
--
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: [PATCH] SubmittingPatches: fix an inconsistency

2014-11-13 Thread slavomir vlcek
On 11/13/2014 07:30 PM, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Junio C Hamano  writes:
>>
 Signed-off-by: slavomir vlcek 
>>
>>> The same comment applies to the log message part.
>>
>> I said:
>>
>>> Will queue; no need to resend.
>>>
>>> Thanks.
>>
>> But one thing to make sure.  Do you really mean to have your
>> sign-off with all lowercase?  I can amend the patch to read
>>
>> Signed-off-by: Slavomir Vlcek 
>>
>> while applying, so that your name does not stand out like a sore
>> thumb in "git shortlog -20 -s" output, if you want.
> 
> ... by the above, I mean something like what appears after the
> scissors "-- >8 --" line below.
> 

Yes, agreed. Thanks for the corrections.

> -- >8 --
> From: Slavomir Vlcek 
> Date: Thu, 13 Nov 2014 00:18:39 +0100
> Subject: [PATCH] SubmittingPatches: final submission is To: maintainer and 
> CC: list
> 
> In an earlier part there is:
> 
>   "re-send it with "To:" set to the maintainer [*1*] and "cc:" the list [*2*]"
> 
> for the final submission, but later we see
> 
>   "Send it to the list and cc the maintainer."
> 
> Fix the later one to match the previous.
> 
> Signed-off-by: Slavomir Vlcek 
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/SubmittingPatches | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index e6d46ed..fa71b5f 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -337,7 +337,7 @@ suggests to the contributors:
>   spend their time to improve your patch.  Go back to step (2).
>  
>   (4) The list forms consensus that the last round of your patch is
> - good.  Send it to the list and cc the maintainer.
> + good.  Send it to the maintainer and cc the list.
>  
>   (5) A topic branch is created with the patch and is merged to 'next',
>   and cooked further and eventually graduates to 'master'.
> 
--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 01:18:43PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Thu, Nov 13, 2014 at 02:26:55PM -0500, Jeff King wrote:
> >
> >> > Makes sense, including the use of strbuf (otherwise you would
> >> > allocate ce and then discard when it turns out that it is not
> >> > needed, which is probably with the same allocation pressure, but
> >> > looks uglier).
> >> 
> >> Exactly. Constructing it in ce->name does save you an allocation/memcpy
> >> in the case that we actually use the new entry, but I thought it would
> >> look weirder. It probably doesn't matter much either way, so I tried to
> >> write the most obvious thing.
> >
> > Actually, it is not that bad:
> 
> Yeah, actually it does look better; want me to squash it into the
> patch before queuing?

Yeah, if you like it, too, then let's go with it. Thanks.

-Peff
--
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 archiving only branch work

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Nov 13, 2014 at 01:10:17PM -0800, Junio C Hamano wrote:
>
>> > How about just adding --stdin, which matches other git commands?
>> 
>> How about doing nothing and use the correct $IFS instead?
>
> Can you cover all cases with $IFS, including filenames with newlines?

You didn't say "--stdin -z", so I presume --stdin is not solving
anything ;-)

> I agree it is probably OK in practice and for the OP's question, but it
> is nice to have "-z" variants so you do not have to worry about quoting
> at all. I'd argue that a "--stdin -z" should probably also accept raw
> filenames, not pathspecs, too (so you do not have to use
> "--literal-pathspecs" elsewhere).

I agree "--stdin -z" is a good thing but what makes you think that
the producer of the data is _always_ walking the directory hierarchy
and showing the pathnames it sees?  I think use of literal-pathspecs
should not be tied to the use of either --stdin or -z.
--
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: [PATCH 2/2] approxidate: allow ISO-like dates far in the future

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 01:11:46PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > if (c != '.' &&
> > -   is_date(num3, num, num2, refuse_future, now, tm))
> > +   is_date(num3, num, num2, refuse_future, now, tm, 0))
> > break;
> 
> Doesn't the new argument '0', which is "allow-future", look somewhat
> strange when we are already passing refuse_future?

To be honest, I had trouble figuring out what the name "refuse_future"
really meant. We do skip the future check, but it also means that
is_date will munge the "struct tm" directly, even if we do not find a
valid date. That worried me a bit.

But yeah, in theory, the callers I wanted to tweak can just pass in a
NULL refuse_future.

-Peff
--
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 archiving only branch work

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 01:10:17PM -0800, Junio C Hamano wrote:

> > How about just adding --stdin, which matches other git commands?
> 
> How about doing nothing and use the correct $IFS instead?

Can you cover all cases with $IFS, including filenames with newlines?

I agree it is probably OK in practice and for the OP's question, but it
is nice to have "-z" variants so you do not have to worry about quoting
at all. I'd argue that a "--stdin -z" should probably also accept raw
filenames, not pathspecs, too (so you do not have to use
"--literal-pathspecs" elsewhere).

-Peff
--
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: appropriate mailing list for gitk patches

2014-11-13 Thread Junio C Hamano
lennart spitzner  writes:

> I have a patch for gitk. Should i send it to this list, or is there some
> other?  git/Documentation/SubmittingPatches mentions
> git://ozlabs.org/~paulus/gitk (the master of which i based my commit
> on), but never mentions any list(s) by name (only "the Git mailing
> list", which i presume is this one).

You are in the right place ;-)  Make sure you Cc: paulus as the area
maintainer on your patches, though.

> Also, a minor question regarding patches: I wondered why patches do not
> mention the commit id that the patch(es) are based on. My current guess
> is that the diffs containing the sha1's is considered sufficient. Is
> this the case? Are there more/other aspects?

A new development is done based on the tip of 'master' by convention
around here, so people know it is meant to apply there unless the
patch submitter says otherwise explicitly.

People send patches on a random commit of their own that is not
available to the public (e.g. you may be working on some big topic,
discover a simple bug that is not related to the topic, and create a
fix for that simple bug right there on top and send that out as a
patch).  Base commit object name would be useless in such a case
anyway.
--
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: [PATCH] git-new-workdir: Add -f to force new-workdir in existing directory.

2014-11-13 Thread Junio C Hamano
Paul Smith  writes:

> From: Paul Smith 
> Date: Thu, 13 Nov 2014 14:01:34 -0500
> Subject: [PATCH] git-new-workdir: Add -f to force new-workdir in existing 
> directory.
>
> Signed-off-by: Paul Smith 
> ---
>
> I have an environment I want to use new-workdir for, where the directory
> I need to use is pre-created for me and I'm dropped into that directory
> and I have no control over this (it's an automated build system).  The
> directory is empty but git-new-workdir still is unhappy about it.  I
> added a "-f" flag to allow the user to force git-new-workdir to continue
> even if the directory exists.  It still bails if there's a .git
> directory already, however.

Is there an easy way to check if the existing directory is really
empty?  For one thing, with your patched version, you may be by
mistake overwriting things when "git checkout -f" happens at the
end, even if there weren't any existing ".git/" directory there.
And if you can check that the existing directory is empty, you
perhaps may not even have to protect this behind a "-f" option.

>
>  contrib/workdir/git-new-workdir | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
> index 75e8b25..a4079c1 100755
> --- a/contrib/workdir/git-new-workdir
> +++ b/contrib/workdir/git-new-workdir
> @@ -10,11 +10,17 @@ die () {
>   exit 128
>  }
>  
> -if test $# -lt 2 || test $# -gt 3
> +if test $# -lt 2 || test $# -gt 4
>  then
> - usage "$0   []"
> + usage "$0 [-f]   []"
>  fi
>  
> +force=false
> +if [ x"$1" = x-f ]
> +then
> +force=true
> +shift
> +fi
>  orig_git=$1
>  new_workdir=$2
>  branch=$3
> @@ -51,7 +57,11 @@ fi
>  # don't recreate a workdir over an existing repository
>  if test -e "$new_workdir"
>  then
> - die "destination directory '$new_workdir' already exists."
> + $force || die "destination directory '$new_workdir' already exists."
> + if test -e "$new_workdir/.git"
> + then
> + die "destination directory '$new_workdir/.git' already exists."
> + fi
>  fi
>  
>  # make sure the links use full paths
--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Nov 13, 2014 at 02:26:55PM -0500, Jeff King wrote:
>
>> > Makes sense, including the use of strbuf (otherwise you would
>> > allocate ce and then discard when it turns out that it is not
>> > needed, which is probably with the same allocation pressure, but
>> > looks uglier).
>> 
>> Exactly. Constructing it in ce->name does save you an allocation/memcpy
>> in the case that we actually use the new entry, but I thought it would
>> look weirder. It probably doesn't matter much either way, so I tried to
>> write the most obvious thing.
>
> Actually, it is not that bad:

Yeah, actually it does look better; want me to squash it into the
patch before queuing?

>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 5410dac..5a78758 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -67,6 +67,7 @@ static int update_some(const unsigned char *sha1, const 
> char *base, int baselen,
>  {
>   int len;
>   struct cache_entry *ce;
> + int pos;
>  
>   if (S_ISDIR(mode))
>   return READ_TREE_RECURSIVE;
> @@ -79,6 +80,23 @@ static int update_some(const unsigned char *sha1, const 
> char *base, int baselen,
>   ce->ce_flags = create_ce_flags(0) | CE_UPDATE;
>   ce->ce_namelen = len;
>   ce->ce_mode = create_ce_mode(mode);
> +
> + /*
> +  * If the entry is the same as the current index, we can leave the old
> +  * entry in place. Whether it is UPTODATE or not, checkout_entry will
> +  * do the right thing.
> +  */
> + pos = cache_name_pos(ce->name, ce->ce_namelen);
> + if (pos >= 0) {
> + struct cache_entry *old = active_cache[pos];
> + if (ce->ce_mode == old->ce_mode &&
> + !hashcmp(ce->sha1, old->sha1)) {
> + old->ce_flags |= CE_UPDATE;
> + free(ce);
> + return 0;
> + }
> + }
> +
>   add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
>   return 0;
>  }
>
> and in some ways more readable, as you form the whole thing, and then as
> the final step either add it, or realize that what is there is fine (I'd
> almost wonder if it could be a flag to add_cache_entry).
>
> -Peff
--
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


appropriate mailing list for gitk patches

2014-11-13 Thread lennart spitzner
Hello folks,

I have a patch for gitk. Should i send it to this list, or is there some
other?  git/Documentation/SubmittingPatches mentions
git://ozlabs.org/~paulus/gitk (the master of which i based my commit
on), but never mentions any list(s) by name (only "the Git mailing
list", which i presume is this one).

Also, a minor question regarding patches: I wondered why patches do not
mention the commit id that the patch(es) are based on. My current guess
is that the diffs containing the sha1's is considered sufficient. Is
this the case? Are there more/other aspects?

Lennart
--
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: [PATCH 2/2] approxidate: allow ISO-like dates far in the future

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

>   if (c != '.' &&
> - is_date(num3, num, num2, refuse_future, now, tm))
> + is_date(num3, num, num2, refuse_future, now, tm, 0))
>   break;

Doesn't the new argument '0', which is "allow-future", look somewhat
strange when we are already passing refuse_future?

--
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: TTY tests are unnecessarily suppressed under Mac OS

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 09:06:51PM +, Michael Blume wrote:

> Can do =)
> 
> uname -r spits out "14.0.0". Is there anything in git's testing library for
> comparing version strings or should I roll my own?

No, there's nothing standard for comparing version numbers. Probably

test "$(uname -r | cut -d. -f1") -ge 14

sufficient.

-Peff
--
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 archiving only branch work

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Nov 13, 2014 at 08:36:16PM +0700, Duy Nguyen wrote:
>
>> On Thu, Nov 13, 2014 at 12:32:40PM +, Graeme Geldenhuys wrote:
>> > [alias]
>> > deploy = !sh -c 'git archive --prefix=$1/ -o deploy_$1.zip HEAD 
>> > $(git diff --name-only -D $2)' -
>> > 
>> > This works very well. The only problem we have so far is that if we 
>> > have files with spaces in the name (eg: SQL update scripts), then the 
>> > command breaks.
>> > 
>> > Does anybody have an idea on how this can be resolved?  Any help would 
>> > be much appreciated.
>> 
>> I wonder if it's overkill to do something like this patch ("git
>> archive" may need some more updates for it to work though). With it
>> you can do:
>> 
>>   git diff --name-only ... | git archive ... HEAD -- ":(file)-"
>> 
>> The good thing is it works for other commands as well. But is it
>> really a good thing..
>
> I like the idea of taking paths from stdin (and especially if there is a
> "-z" option). But using a pathspec that reads from stdin seems like it
> creates a lot of corner cases. What would:
>
>   git rev-list --stdin -- ":(file)-"
>
> do? It is kind of neat that you could read from multiple files (besides
> stdin), but I'm not sure it is all that useful in practice (you can
> always cat them to its stdin).
>
> How about just adding --stdin, which matches other git commands?

How about doing nothing and use the correct $IFS instead?
--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Junio C Hamano
Junio C Hamano  writes:

> Thanks for an explanation why the updateInstead mode must require a
> pristine working tree (and the index).  I *now* agree why such a
> mode would be useful, *after* reading it.  I did not understand why
> *after* reading only the patch, the documentation updates and the
> log message.
>
> That tells us something, doesn't it? ;-)

Just to be less cryptic, I meant that the documentation since v2 is
probably insufficient.

> I agree that a new value "mergeInstead" or something should be
> invented when/if different workflows want a looser semantics.
> People would rely not only on "being able to push when clean" but
> also on "being safely prevented from pushing when not" (and that is
> where my earlier comment to test both sides comes from).  Loosening
> the check to an existing "updateInstead" would break users who have
> been using updateInstead.

And thinking about the names again, I have a feeling that
updateInstead and mergeInstead are both probably misnomer.  The
"make sure there is no modification and then do an equivalent of
reset --hard there" option makes sense only in push-to-deploy
scenario (perhaps "resetInstead"?)  Compared to that, "do an
equivalent of checkout as long as it can be done without involving
real merges" sounds more deserving the "updateInstead" name.

But I am not very good at names (and I suspect you feel you yourself
aren't, either), so perhaps somebody listening from the sideline can
chime in.
--
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: TTY tests are unnecessarily suppressed under Mac OS

2014-11-13 Thread Jeff King
On Wed, Nov 12, 2014 at 02:25:52PM -0800, Michael Blume wrote:

> From lib_terminal.sh:
> 
> # Reading from the pty master seems to get stuck _sometimes_
> # on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
> #
> # Reproduction recipe: run
> #
> # i=0
> # while ./test-terminal.perl echo hi $i
> # do
> # : $((i = $i + 1))
> # done
> #
> # After 2000 iterations or so it hangs.
> # https://rt.cpan.org/Ticket/Display.html?id=65692
> #
> test "$(uname -s)" != Darwin &&
> 
> I tried the reproduction recipe on my mac and couldn't reproduce, so
> it may make sense to take this switch out? In any case, I've set my
> automated mac build to include TTY tests

10.5.0 is pretty ancient at this point; I can well believe that the
upstream problem has been fixed. It would be nice if we knew in which
version it was fixed, though. Just dropping the restriction risks people
getting spurious failures if they are on an old enough version.

Do you want to roll a patch that checks $(uname) to see if we're on a
recent-enough version (where we can just be conservative, and assume
whatever version you have is the first one to fix it)?

-Peff
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 05:08:19PM +0100, Johan Herland wrote:

> Can you not do this much simpler with --reference? Like this:
> 
>   $ git clone --bare git://host/repo.git repo-master
>   $ git clone -b branchA --reference repo-master git://host/repo.git
> repo-branchA
>   $ git clone -b branchB --reference repo-master git://host/repo.git
> repo-branchB
> 
> All three repos now push/pull directly to/from git://host/repo.git,
> but repo-branchA and repo-branchB reference objects from within the
> bare repo-master. You have to make use to never delete objects from
> repo-master

I think the "never delete" part is why we usually warn people off of
using alternates. I think at the least you would have to "git config
gc.auto 0" in the bare repository (otherwise your nightly fetches risk
pruning). Of course you'd probably want to repack eventually for
performance reasons. So maybe setting gc.pruneExpire is a better option
(to something like "20.years.ago").

> If you want to prevent the repos growing in size, you must devise a
> way to add new objects into repo-master before repo-branchA|B. (e.g. a
> nightly cron-job in repo-master that fetches from origin), so that
> when repo-branchA|B pulls, they will find most objects are already
> present in repo-master.

You can also fetch from the children into repo-master periodically.
Like:

  cd repo-master &&
  for i in branchA branchB; do
git fetch ../$i +refs/*:refs/remotes/$i/*
  done

after which it is actually safe to run "git gc" in the master (assuming
there isn't simultaneous activity in the children). This is how we
manage fork networks on GitHub (we take in objects to individual forks
via push, and then migrate them to the master repo via fetch).

-Peff
--
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 archiving only branch work

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 08:36:16PM +0700, Duy Nguyen wrote:

> On Thu, Nov 13, 2014 at 12:32:40PM +, Graeme Geldenhuys wrote:
> > [alias]
> > deploy = !sh -c 'git archive --prefix=$1/ -o deploy_$1.zip HEAD 
> > $(git diff --name-only -D $2)' -
> > 
> > This works very well. The only problem we have so far is that if we 
> > have files with spaces in the name (eg: SQL update scripts), then the 
> > command breaks.
> > 
> > Does anybody have an idea on how this can be resolved?  Any help would 
> > be much appreciated.
> 
> I wonder if it's overkill to do something like this patch ("git
> archive" may need some more updates for it to work though). With it
> you can do:
> 
>   git diff --name-only ... | git archive ... HEAD -- ":(file)-"
> 
> The good thing is it works for other commands as well. But is it
> really a good thing..

I like the idea of taking paths from stdin (and especially if there is a
"-z" option). But using a pathspec that reads from stdin seems like it
creates a lot of corner cases. What would:

  git rev-list --stdin -- ":(file)-"

do? It is kind of neat that you could read from multiple files (besides
stdin), but I'm not sure it is all that useful in practice (you can
always cat them to its stdin).

How about just adding --stdin, which matches other git commands?

-Peff
--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 02:26:55PM -0500, Jeff King wrote:

> > Makes sense, including the use of strbuf (otherwise you would
> > allocate ce and then discard when it turns out that it is not
> > needed, which is probably with the same allocation pressure, but
> > looks uglier).
> 
> Exactly. Constructing it in ce->name does save you an allocation/memcpy
> in the case that we actually use the new entry, but I thought it would
> look weirder. It probably doesn't matter much either way, so I tried to
> write the most obvious thing.

Actually, it is not that bad:

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5410dac..5a78758 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -67,6 +67,7 @@ static int update_some(const unsigned char *sha1, const char 
*base, int baselen,
 {
int len;
struct cache_entry *ce;
+   int pos;
 
if (S_ISDIR(mode))
return READ_TREE_RECURSIVE;
@@ -79,6 +80,23 @@ static int update_some(const unsigned char *sha1, const char 
*base, int baselen,
ce->ce_flags = create_ce_flags(0) | CE_UPDATE;
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
+
+   /*
+* If the entry is the same as the current index, we can leave the old
+* entry in place. Whether it is UPTODATE or not, checkout_entry will
+* do the right thing.
+*/
+   pos = cache_name_pos(ce->name, ce->ce_namelen);
+   if (pos >= 0) {
+   struct cache_entry *old = active_cache[pos];
+   if (ce->ce_mode == old->ce_mode &&
+   !hashcmp(ce->sha1, old->sha1)) {
+   old->ce_flags |= CE_UPDATE;
+   free(ce);
+   return 0;
+   }
+   }
+
add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
return 0;
 }

and in some ways more readable, as you form the whole thing, and then as
the final step either add it, or realize that what is there is fine (I'd
almost wonder if it could be a flag to add_cache_entry).

-Peff
--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Junio C Hamano
Johannes Schindelin  writes:

> Thanks for mentioning this. I would like to ask not to loosen this later.
> Let me try to explain in more detail than before why I think it would make
> *my* life hard if it ever were loosened.
> ...
> And now when I try to push, Git complains that the working directory is
> not clean, which is *just* fine by me, because after further inspection it
> turns out that the uncommitted changes – which are in a different file
> than the changes introducing my new feature – would have borked the
> production system rather thoroughly.
> ...
> In my mind, when (and if!) a less strict version is desired, it should be
> added as another denyCurrentBranch value and 'updateInstead' should be
> unaffected, otherwise, speaking for me personally, all my work in this
> patch series would be for naught.

Thanks for an explanation why the updateInstead mode must require a
pristine working tree (and the index).  I *now* agree why such a
mode would be useful, *after* reading it.  I did not understand why
*after* reading only the patch, the documentation updates and the
log message.

That tells us something, doesn't it? ;-)

Also the failure case test must protect us from making a change you
fear in the future.  The interdiff you sent in a separate message
was to smudge path2 that is modified in the 'fourth' commit, which
should fail with either your "OK only if really clean" requirement
or the other "OK as long as it does not interfere with the switch"
criterion.  Checking that is a good step, but you would want to have
a separate "Smudge a path that is unchanged with the switch and see
the push updateInstead fail" to protect the current semantics.

I agree that a new value "mergeInstead" or something should be
invented when/if different workflows want a looser semantics.
People would rely not only on "being able to push when clean" but
also on "being safely prevented from pushing when not" (and that is
where my earlier comment to test both sides comes from).  Loosening
the check to an existing "updateInstead" would break users who have
been using updateInstead.

Also I suspect that people would want to send a patch to add "-q" to
your "update-index --refresh" invocation, at which time you would
need to add a call to "diff-files" to check that the working tree
and the index match.  You may want to add that "diff-files" now, or
at least have a test that is designed to break when "-q" is added to
"update-index --refresh" without adding the necessary "diff-files".

Thanks.
--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 11:15:27AM -0800, Junio C Hamano wrote:

> > diff --git a/builtin/checkout.c b/builtin/checkout.c
> > index 5410dac..67cab4e 100644
> > --- a/builtin/checkout.c
> > +++ b/builtin/checkout.c
> > @@ -65,21 +65,40 @@ static int post_checkout_hook(struct commit *old, 
> > struct commit *new,
> >  static int update_some(const unsigned char *sha1, const char *base, int 
> > baselen,
> > const char *pathname, unsigned mode, int stage, void *context)
> >  {
> > ...
> >  }
> 
> Makes sense, including the use of strbuf (otherwise you would
> allocate ce and then discard when it turns out that it is not
> needed, which is probably with the same allocation pressure, but
> looks uglier).

Exactly. Constructing it in ce->name does save you an allocation/memcpy
in the case that we actually use the new entry, but I thought it would
look weirder. It probably doesn't matter much either way, so I tried to
write the most obvious thing.

(I also experimented with using make_cache_entry at one point, which
requires the strbuf; some of my thinking on what looks reasonable may be
left over from that approach).

> > +test_expect_success 'do not touch files that are already up-to-date' '
> > +   git reset --hard &&
> > +   echo one >file1 &&
> > +   echo two >file2 &&
> > +   git add file1 file2 &&
> > +   git commit -m base &&
> > +   echo modified >file1 &&
> > +   test-chmtime =10 file2 &&
> 
> Is the idea behind the hardcoded timestamp that this is sufficiently
> old (Sep 2001) that we will not get in trouble comparing with the
> real timestamp we get from the filesystem (which will definitely newer
> than that anyway) no matter when we run this test (unless you have a
> time-machine, that is)?

I didn't actually calculate what the timestamp was. The important thing
is that it is not the timestamp that your system would give to the file
if git-checkout opened and rewrote it. :)

I initially used "123", but was worried that would cause weird
portability problems on systems. So I opted for something closer to
"normal", but in the past. I think it is fine (modulo time machines),
but I'd be happy to put in some more obvious sentinel, too.

And the worst case if you did have a time machine is that we might
accidentally declare a buggy git to be correct (racily!). I can live
with that, but I guess you could use a relative value (like "-1")
instead of a fixed sentinel (but then you'd have to record it for the
"expect" check).

-Peff
--
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: [PATCH v3 1/1] Add another option for receive.denyCurrentBranch

2014-11-13 Thread Johannes Schindelin
Hi Junio,

On Thu, 13 Nov 2014, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> > index f4da20a..ba002a9 100755
> > --- a/t/t5516-fetch-push.sh
> > +++ b/t/t5516-fetch-push.sh
> > @@ -1330,4 +1330,21 @@ test_expect_success 'fetch into bare respects 
> > core.logallrefupdates' '
> > )
> >  '
> >  
> > +test_expect_success 'receive.denyCurrentBranch = updateInstead' '
> > +   git push testrepo master &&
> > +   (cd testrepo &&
> > +   git reset --hard &&
> > +   git config receive.denyCurrentBranch updateInstead
> > +   ) &&
> > +   test_commit third path2 &&
> > +   git push testrepo master &&
> > +   test $(git rev-parse HEAD) = $(cd testrepo && git rev-parse HEAD) &&
> > +   test third = "$(cat testrepo/path2)" &&
> > +   (cd testrepo &&
> > +   git update-index --refresh &&
> > +   git diff-files --quiet &&
> > +   git diff-index --cached HEAD --
> > +   )
> > +'
> > +
> 
> This new feature has two sides.  Update when we can and more
> importantly fail the update safely.  This tests the "success" case,
> but not the "safely fail" one.
> 
> For the latter "test_must_fail git push" on the sending side, and
> "original HEAD stays the same and the working tree changes are
> preserved when there are local changes before the push" on the
> receiving side needs to be tested.

Right.

I have amended this for the upcoming v4 (but I'll wait whether there are
other things I need to change before submitting that):

-- snipsnap --
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ba002a9..b8df39c 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1343,8 +1343,12 @@ test_expect_success 'receive.denyCurrentBranch =
updateInstead' '
(cd testrepo &&
git update-index --refresh &&
git diff-files --quiet &&
-   git diff-index --cached HEAD --
-   )
+   git diff-index --cached HEAD -- &&
+   echo changed > path2 &&
+   git add path2
+   ) &&
+   test_commit fourth path2 &&
+   test_must_fail git push testrepo master
 '
 
 test_done

--
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: [PATCH v3 0/1] Support updating working trees when pushing into non-bare repos

2014-11-13 Thread Johannes Schindelin
Hi Junio,

On Thu, 13 Nov 2014, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > This patch series adds support for a new receive.denyCurrentBranch setting
> > to update the working directory (which must be clean, i.e. there must not be
> > any uncommitted changes) when pushing into the current branch.
> >
> > The scenario in which the 'updateInstead' setting became a boon in this
> > developer's daily work is when trying to get a bug fix from a Windows
> > computer, a virtual machine or a user's machine onto his main machine (in
> > all of those cases it is only possible to connect via ssh in one direction,
> > but not in the reverse direction).
> >
> > Interdiff vs v2 below the diffstat.
> >
> > Johannes Schindelin (1):
> >   Add another option for receive.denyCurrentBranch
> >
> >  Documentation/config.txt |  5 
> >  builtin/receive-pack.c   | 78 
> > ++--
> >  t/t5516-fetch-push.sh| 17 +++
> >  3 files changed, 98 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 4f9fe81..c384515 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -2134,10 +2134,6 @@ Another option is "updateInstead" which will update 
> > the working
> >  directory (must be clean) if pushing into the current branch. This option 
> > is
> >  intended for synchronizing working directories when one side is not easily
> >  accessible via ssh (e.g. inside a VM).
> > -+
> > -Yet another option is "detachInstead" which will detach the HEAD if updates
> > -are pushed into the current branch; That way, the current revision, the
> > -index and the working directory are always left untouched by pushes.
> 
> I think we had an exchange to clarify the workflow in which
> updateInstead is useful and how to help readers, but I do not see
> any change on that in this part of documentation.  Forgot to revise?

I had revised it for v2 already. It now reads:

-- snip --
Another option is "updateInstead" which will update the working directory
(must be clean) if pushing into the current branch. This option is
intended for synchronizing working directories when one side is not easily
accessible via ssh (e.g. inside a VM).
-- snap --

In my mind, this strikes the balance between sketching a scenario where
the setting makes sense on the one hand and abducting config.txt to tell
my life's story on the other.

> > @@ -737,36 +733,66 @@ static int update_shallow_ref(struct command *cmd, 
> > struct shallow_info *si)
> > return 0;
> >  }
> >  
> > -static const char *merge_worktree(unsigned char *sha1)
> > +static const char *update_worktree(unsigned char *sha1)
> >  {
> > const char *update_refresh[] = {
> > "update-index", "--ignore-submodules", "--refresh", NULL
> > };
> > +   const char *diff_index[] = {
> > +   "diff-index", "--quiet", "--cached", "--ignore-submodules",
> > +   "HEAD", "--", NULL
> > +   };
> > const char *read_tree[] = {
> > "read-tree", "-u", "-m", sha1_to_hex(sha1), NULL
> > };
> 
> OK.
> 
> "update-index --refresh && diff-files && diff-index --cached" is how
> we traditionally ensure the working tree is absolutely clean (see
> require_clean_work_tree in git-sh-setup.sh), but I do not think of a
> reason why diff-files step is not redundant.

I fear that my double-negation-fu is still stuck somewhere in dreamland.
Do you mean to say "I could imagine that the diff-files step is
redundant"? If that is what you are telling me, then your explanation of
the exit code of update-index --refresh would suggest so, and so would
https://github.com/git/git/blob/f5709437/read-cache.c#L1201-L1230 *except*
in the case where refresh_cache_ent() returns an updated cache entry:
https://github.com/git/git/blob/f5709437/read-cache.c#L1116-L1131 – but I
could not figure out quickly when this code path is hit.

Ciao,
Johannes

[PATCH] git-new-workdir: Add -f to force new-workdir in existing directory.

2014-11-13 Thread Paul Smith
From: Paul Smith 
Date: Thu, 13 Nov 2014 14:01:34 -0500
Subject: [PATCH] git-new-workdir: Add -f to force new-workdir in existing 
directory.

Signed-off-by: Paul Smith 
---

I have an environment I want to use new-workdir for, where the directory
I need to use is pre-created for me and I'm dropped into that directory
and I have no control over this (it's an automated build system).  The
directory is empty but git-new-workdir still is unhappy about it.  I
added a "-f" flag to allow the user to force git-new-workdir to continue
even if the directory exists.  It still bails if there's a .git
directory already, however.

 contrib/workdir/git-new-workdir | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 75e8b25..a4079c1 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -10,11 +10,17 @@ die () {
exit 128
 }
 
-if test $# -lt 2 || test $# -gt 3
+if test $# -lt 2 || test $# -gt 4
 then
-   usage "$0   []"
+   usage "$0 [-f]   []"
 fi
 
+force=false
+if [ x"$1" = x-f ]
+then
+force=true
+shift
+fi
 orig_git=$1
 new_workdir=$2
 branch=$3
@@ -51,7 +57,11 @@ fi
 # don't recreate a workdir over an existing repository
 if test -e "$new_workdir"
 then
-   die "destination directory '$new_workdir' already exists."
+   $force || die "destination directory '$new_workdir' already exists."
+   if test -e "$new_workdir/.git"
+   then
+   die "destination directory '$new_workdir/.git' already exists."
+   fi
 fi
 
 # make sure the links use full paths
-- 
2.1.3



--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> On Sun, Nov 09, 2014 at 09:21:49AM -0800, Junio C Hamano wrote:
>
>> Jeff King  writes:
>> 
>> > On Fri, Nov 07, 2014 at 11:35:59PM -0800, Junio C Hamano wrote:
>> >
>> >> I think that has direct linkage; what you have in mind I think is
>> >> http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234935
>> >
>> > Thanks for that link.
>> 
>> It was one of the items in the "git blame leftover bits" list
>> (websearch for that exact phrase), so I didn't have to do any
>> digging just for this thread ;-)
>> 
>> But I made a huge typo above.  s/I think/I do not think/;
>
> Oh. That might explain some of my confusion. :)

Yeah, tells me to never type on a tablet X-<.

>> I'd prefer that these two to be treated separately.
>
> Yeah, that makes sense after reading your emails. What I was really
> unclear on was whether the handling of deletion was a bug or a design
> choice, and it is the latter (if it were the former, we would not need a
> transition plan :) ).

Yeah, I think we agree to refrain from saying if that design choice
was a good one or bad one at least for now.

> Subject: checkout $tree: do not throw away unchanged index entries
>
> When we "git checkout $tree", we pull paths from $tree into
> the index, and then check the resulting entries out to the
> worktree. Our method for the first step is rather
> heavy-handed, though; it clobbers the entire existing index
> entry, even if the content is the same. This means we lose
> our stat information, leading checkout_entry to later
> rewrite the entire file with identical content.
>
> Instead, let's see if we have the identical entry already in
> the index, in which case we leave it in place. That lets
> checkout_entry do the right thing. Our tests cover two
> interesting cases:
>
>   1. We make sure that a file which has no changes is not
>  rewritten.
>
>   2. We make sure that we do update a file that is unchanged
>  in the index (versus $tree), but has working tree
>  changes. We keep the old index entry, and
>  checkout_entry is able to realize that our stat
>  information is out of date.
>
> Signed-off-by: Jeff King 
> ---
> Note that the test refreshes the index manually (because we are tweaking
> the timestamp of file2). In normal use this should not be necessary
> (i.e., your entries should generally be uptodate). I did wonder if
> checkout should be refreshing the index itself, but it would a bunch of
> extra lstats in the common case.
>
>  builtin/checkout.c| 31 +--
>  t/t2022-checkout-paths.sh | 17 +
>  2 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 5410dac..67cab4e 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -65,21 +65,40 @@ static int post_checkout_hook(struct commit *old, struct 
> commit *new,
>  static int update_some(const unsigned char *sha1, const char *base, int 
> baselen,
>   const char *pathname, unsigned mode, int stage, void *context)
>  {
> ...
>  }

Makes sense, including the use of strbuf (otherwise you would
allocate ce and then discard when it turns out that it is not
needed, which is probably with the same allocation pressure, but
looks uglier).

> diff --git a/t/t2022-checkout-paths.sh b/t/t2022-checkout-paths.sh
> index 8e3545d..f46d049 100755
> --- a/t/t2022-checkout-paths.sh
> +++ b/t/t2022-checkout-paths.sh
> @@ -61,4 +61,21 @@ test_expect_success 'do not touch unmerged entries 
> matching $path but not in $tr
>   test_cmp expect.next0 actual.next0
>  '
>  
> +test_expect_success 'do not touch files that are already up-to-date' '
> + git reset --hard &&
> + echo one >file1 &&
> + echo two >file2 &&
> + git add file1 file2 &&
> + git commit -m base &&
> + echo modified >file1 &&
> + test-chmtime =10 file2 &&

Is the idea behind the hardcoded timestamp that this is sufficiently
old (Sep 2001) that we will not get in trouble comparing with the
real timestamp we get from the filesystem (which will definitely newer
than that anyway) no matter when we run this test (unless you have a
time-machine, that is)?

> + git update-index -q --refresh &&
> + git checkout HEAD -- file1 file2 &&
> + echo one >expect &&
> + test_cmp expect file1 &&
> + echo "10file2" >expect &&
> + test-chmtime -v +0 file2 >actual &&
> + test_cmp expect actual
> +'
> +
>  test_done
--
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: [PATCH v3 0/1] Support updating working trees when pushing into non-bare repos

2014-11-13 Thread Junio C Hamano
Junio C Hamano  writes:

>> -static const char *merge_worktree(unsigned char *sha1)
>> +static const char *update_worktree(unsigned char *sha1)
>>  {
>>  const char *update_refresh[] = {
>>  "update-index", "--ignore-submodules", "--refresh", NULL
>>  };
>> +const char *diff_index[] = {
>> +"diff-index", "--quiet", "--cached", "--ignore-submodules",
>> +"HEAD", "--", NULL
>> +};
>>  const char *read_tree[] = {
>>  "read-tree", "-u", "-m", sha1_to_hex(sha1), NULL
>>  };
>
> OK.
>
> "update-index --refresh && diff-files && diff-index --cached" is how
> we traditionally ensure the working tree is absolutely clean (see
> require_clean_work_tree in git-sh-setup.sh), but I do not think of a
> reason why diff-files step is not redundant.  As a totally separate
> topic outside this series, we may want to visit that shell function.

Ahh, scratch that.  Over there we use 'update-index -q --refresh',
so the difference between the index and the working tree is not
noticed and we do need diff-files.  Here, the code runs update-index
without -q, so we do catch such a difference without diff-files.

Sorry for the noise ;-)
--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Johannes Schindelin
Hi Junio,

On Thu, 13 Nov 2014, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > On Thu, 13 Nov 2014, Johannes Schindelin wrote:
> >
> >> Due to that experience, the documentation also states pretty clearly that
> >> `updateInstead` succeeds only in updating the current branch if the
> >> working directory is clean.
> >
> > To clarify why `updateInstead` is stricter than the `merge` scenario: when
> > pushing into a remote repository with attached working directory, we
> > cannot simply clean up merge conflicts or other problems introduced by a
> > merge. Indeed, it is even possible to push without having any option to
> > fix files in the working directory afterwards. Therefore, the
> > `updateInstead` feature really needs to be adamant about the working
> > directory being clean.
> 
> As the stricter check (which I think is unnecessarily strict and
> which you don't) can be loosened later without making something the
> user used to be able to do in an early version unable to do later,
> I am OK to accept the design as-is.

Thanks for mentioning this. I would like to ask not to loosen this later.
Let me try to explain in more detail than before why I think it would make
*my* life hard if it ever were loosened.

Let's start with a hypothetical change to notes.c, a change, say, that
uses part of the diff machinery. So I am doing that in my working
directory, happily testing stuff, but not quite happy with the result just
yet, staging a couple of things after a debug run so I can "git stash -k"
away the debugging statements.

Please note that while the scenario I just illustrated is purely
hypothetical, made up from thin air, the workflow and the technique is
very much my daily bread, though. I do use "git stash -k" quite frequently
to stash away stuff that I do not want, after staging what I want to keep
first.

Now let's continue by saying that I get interrupted with that notes.c work
and only come back to it much, much later, say, a couple of months,
because I have been working on a different computer in the meantime,
say, fixing Git for Windows bugs like mad.

Okay, so now I am back and I "git pull" your 'master'. Now, for the sake
of the argument, let's say that the pull does not introduce any changes to
notes.c, but instead that there has been an improvement in the diff
machinery, one that required a change of the diff machinery's API and all
of a sudden, my code does not compile anymore. It merged cleanly,
Git is completely groovy, but the code just does not compile any more. Not
a big deal: I can inspect the changes via "git diff
junio/master@{1}..junio/master" and find out very easily what went wrong
and fix my code, compile, stage, and maybe even commit because the code
now works.

Splendid.

But it would be different if the working directory was on another computer
than the one I am operating from. I could not easily access the working
directory, let alone the Git history, if all I did was a "git push".

Now let's modify the scenario a little bit further, still. Let's not talk
about git.git, but about a project implementing a web application. And
let's say that I did not dabble with notes.c but I had to work on the
production setup for a really, really urgent hotfix where the speed of a
fix was more important than not touching the production setup at all.

Please note that this is *not* a hypothetical scenario, but a true account
of what I had to go through a couple of times (don't try this at home, it
causes a lot of gray hair).

Now let's assume that I forgot to commit and push, and that nobody noticed
– and let's just draw the curtain of charity over the reasons why. Also,
let's assume that a couple of months later, I am asked to implement a new
feature for the very same web application, this time without frantic "You.
Have. To. Fix. This. ASAP!" stuff going on. So I develop this on a test
machine, test thoroughly, everything's groovy, and then I push, using the
'updateInstead' feature introduced by the patch in question (and which was
applied to that machine's Git at that time, before some joker updated it
with the official Git).

And now when I try to push, Git complains that the working directory is
not clean, which is *just* fine by me, because after further inspection it
turns out that the uncommitted changes – which are in a different file
than the changes introducing my new feature – would have borked the
production system rather thoroughly.

Please note again that I am talking about something that really happened.

So I would really like to have 'updateInstead', but only in the safe
version, i.e. refusing to update anything but a clean working directory,
and it would be a major regression for me, affecting my workflow rather
negatively, if those strict checks were loosened.

In my mind, when (and if!) a less strict version is desired, it should be
added as another denyCurrentBranch value and 'updateInstead' should be
unaffected, otherwise, speaking for me personally, all my 

Re: t9902-completion.sh failed

2014-11-13 Thread Alex Kuleshov

ah, i catched the problem, I launched make test with sudo and now all
tests passed successfully.

Jeff King  @ 2014-11-13 17:24 ALMT:

> On Thu, Nov 13, 2014 at 04:59:12PM +0600, Alex Kuleshov wrote:
>
>> i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
>> built and installed it successfully, now i'm running make test and got
>> following error:
>>
>> *** t9902-completion.sh ***
>> t9902-completion.sh: 118:
>> /home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
>> error: "(" unexpected (expecting "fi")
>> FATAL: Unexpected exit with code 2
>> make[2]: *** [t9902-completion.sh] Error 1
>> make[2]: Leaving directory `/home/shk/dev/git/t'
>> make[1]: *** [test] Error 2
>> make[1]: Leaving directory `/home/shk/dev/git/t'
>> make: *** [test] Error 2
>>
>> $ bash --version
>> 4.3.11(1)-release (x86_64-pc-linux-gnu)
>
> Weird. I can't reproduce here, using the version of bash from Debian
> unstable (4.3.30(1)), nor compiling 4.3.11 from sources. What platform
> are you on (i.e., might it be bash + some other patches installed by the
> distro)?
>
> -Peff

--
Best regards.
0xAX
--
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: [RFC] git checkout $tree -- $path always rewrites files

2014-11-13 Thread Jeff King
On Sun, Nov 09, 2014 at 09:21:49AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Fri, Nov 07, 2014 at 11:35:59PM -0800, Junio C Hamano wrote:
> >
> >> I think that has direct linkage; what you have in mind I think is
> >> http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234935
> >
> > Thanks for that link.
> 
> It was one of the items in the "git blame leftover bits" list
> (websearch for that exact phrase), so I didn't have to do any
> digging just for this thread ;-)
> 
> But I made a huge typo above.  s/I think/I do not think/;

Oh. That might explain some of my confusion. :)

> The original observation you made in this thread is that when "git
> checkout $tree - $pathspec", whose defintion is to "grab paths in
> the $tree that match $pathspec and put them in the index, and then
> overwrite the working tree paths with the contents of these paths
> that are updated in the index with the contents taken from the
> $tree", unnecessarily rewrites the working tree paths even when they
> already had contents that were up to date.  That is what I called an
> "implementation glitch".
> 
> The old thread is a different topic.
> [...]

Right, I do agree that these things don't need to be linked. The reason
I ended up dealing with the deletion thing is that one obvious way to
implement "do not touch entries which have not changed" is by running a
diff between $tree and the index. But doing a diff means we have to
reconsider all of the code surrounding deletions (and handling of
unmerged entries in the pathspec but not in $tree). I tried a few
variants and had trouble making it work (getting caught up either in the
"make sure each pathspec matched" code, or the "treat unmerged entries
specially" behavior).

In the patch below, I ended up retaining the existing
read_tree_recursive code, and just specially handling the replacement of
index entries (which is itself sort of a diff, but it fits nicely into
the existing scheme).

> I'd prefer that these two to be treated separately.

Yeah, that makes sense after reading your emails. What I was really
unclear on was whether the handling of deletion was a bug or a design
choice, and it is the latter (if it were the former, we would not need a
transition plan :) ).

Anyway, here is the patch.

-- >8 --
Subject: checkout $tree: do not throw away unchanged index entries

When we "git checkout $tree", we pull paths from $tree into
the index, and then check the resulting entries out to the
worktree. Our method for the first step is rather
heavy-handed, though; it clobbers the entire existing index
entry, even if the content is the same. This means we lose
our stat information, leading checkout_entry to later
rewrite the entire file with identical content.

Instead, let's see if we have the identical entry already in
the index, in which case we leave it in place. That lets
checkout_entry do the right thing. Our tests cover two
interesting cases:

  1. We make sure that a file which has no changes is not
 rewritten.

  2. We make sure that we do update a file that is unchanged
 in the index (versus $tree), but has working tree
 changes. We keep the old index entry, and
 checkout_entry is able to realize that our stat
 information is out of date.

Signed-off-by: Jeff King 
---
Note that the test refreshes the index manually (because we are tweaking
the timestamp of file2). In normal use this should not be necessary
(i.e., your entries should generally be uptodate). I did wonder if
checkout should be refreshing the index itself, but it would a bunch of
extra lstats in the common case.

 builtin/checkout.c| 31 +--
 t/t2022-checkout-paths.sh | 17 +
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5410dac..67cab4e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -65,21 +65,40 @@ static int post_checkout_hook(struct commit *old, struct 
commit *new,
 static int update_some(const unsigned char *sha1, const char *base, int 
baselen,
const char *pathname, unsigned mode, int stage, void *context)
 {
-   int len;
+   struct strbuf buf = STRBUF_INIT;
struct cache_entry *ce;
+   int pos;
 
if (S_ISDIR(mode))
return READ_TREE_RECURSIVE;
 
-   len = baselen + strlen(pathname);
-   ce = xcalloc(1, cache_entry_size(len));
+   strbuf_add(&buf, base, baselen);
+   strbuf_addstr(&buf, pathname);
+
+   /*
+* If the entry is the same as the current index, we can leave the old
+* entry in place. Whether it is UPTODATE or not, checkout_entry will
+* do the right thing.
+*/
+   pos = cache_name_pos(buf.buf, buf.len);
+   if (pos >= 0) {
+   struct cache_entry *old = active_cache[pos];
+   if (create_ce_mode(mode) == old->ce_mode &&
+   !hashcmp(old->sha1, sha1)) {
+   

Re: [PATCH] SubmittingPatches: fix an inconsistency

2014-11-13 Thread Junio C Hamano
Junio C Hamano  writes:

> Junio C Hamano  writes:
>
>>> Signed-off-by: slavomir vlcek 
>
>> The same comment applies to the log message part.
>
> I said:
>
>> Will queue; no need to resend.
>>
>> Thanks.
>
> But one thing to make sure.  Do you really mean to have your
> sign-off with all lowercase?  I can amend the patch to read
>
> Signed-off-by: Slavomir Vlcek 
>
> while applying, so that your name does not stand out like a sore
> thumb in "git shortlog -20 -s" output, if you want.

... by the above, I mean something like what appears after the
scissors "-- >8 --" line below.

-- >8 --
From: Slavomir Vlcek 
Date: Thu, 13 Nov 2014 00:18:39 +0100
Subject: [PATCH] SubmittingPatches: final submission is To: maintainer and CC: 
list

In an earlier part there is:

  "re-send it with "To:" set to the maintainer [*1*] and "cc:" the list [*2*]"

for the final submission, but later we see

  "Send it to the list and cc the maintainer."

Fix the later one to match the previous.

Signed-off-by: Slavomir Vlcek 
Signed-off-by: Junio C Hamano 
---
 Documentation/SubmittingPatches | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index e6d46ed..fa71b5f 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -337,7 +337,7 @@ suggests to the contributors:
  spend their time to improve your patch.  Go back to step (2).
 
  (4) The list forms consensus that the last round of your patch is
- good.  Send it to the list and cc the maintainer.
+ good.  Send it to the maintainer and cc the list.
 
  (5) A topic branch is created with the patch and is merged to 'next',
  and cooked further and eventually graduates to 'master'.
-- 
2.2.0-rc1-84-gcd6439f

--
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: [PATCH] SubmittingPatches: fix an inconsistency

2014-11-13 Thread Junio C Hamano
Junio C Hamano  writes:

>> Signed-off-by: slavomir vlcek 

> The same comment applies to the log message part.

I said:

> Will queue; no need to resend.
>
> Thanks.

But one thing to make sure.  Do you really mean to have your
sign-off with all lowercase?  I can amend the patch to read

Signed-off-by: Slavomir Vlcek 

while applying, so that your name does not stand out like a sore
thumb in "git shortlog -20 -s" output, if you want.

--
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: [PATCH] SubmittingPatches: fix an inconsistency

2014-11-13 Thread Junio C Hamano
slavomir vlcek  writes:

> The 'SubmittingPatches' document contains a small inconsistency
>  in a patch-email sending procedure.
>
> Not a big thing,
> but a newcomer could get confused.
>
> Please,
> also consider adding the definition/explanation for all the branches available
> at the beginning of this document (and maybe even what their names stand for).
> Thanks.
>
> Signed-off-by: slavomir vlcek 
> ---
>
> From 74859712cf805663e3863686bdc09511c74b207b Mon Sep 17 00:00:00 2001
> From: slavomir vlcek 
> Date: Thu, 13 Nov 2014 00:18:39 +0100
> Subject: [PATCH] SubmittingPatches: fix an inconsistency
>
> At line 213 there was an instruction:
>   "re-send it with "To:" set to the maintainer [*1*] and "cc:" the list [*2*]"
>
> and this instruction got repeated once more in the document (line 340):
>   "Send it to the list and cc the maintainer."
>
> This inconsistency was solved by editing the second occurance.
> ---

The same comment applies to the log message part.

Will queue; no need to resend.

Thanks.

>  Documentation/SubmittingPatches | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index e6d46ed..fa71b5f 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -337,7 +337,7 @@ suggests to the contributors:
>   spend their time to improve your patch.  Go back to step (2).
>  
>   (4) The list forms consensus that the last round of your patch is
> - good.  Send it to the list and cc the maintainer.
> + good.  Send it to the maintainer and cc the list.
>  
>   (5) A topic branch is created with the patch and is merged to 'next',
>   and cooked further and eventually graduates to 'master'.
--
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: [PATCH/RFC] builtin: move builtin retrieval to get_builtin()

2014-11-13 Thread Junio C Hamano
slavomir vlcek  writes:

> Hi,
> found a small code redundancy in a builtin command retrieval ('git.c').
>
> For the "master" branch.
>
> Thanks in advance for any suggestions.
>
> Signed-off-by: slavomir vlcek 
> ---

Thanks.  Please do realize that all of the above before the
three-dash line and nothing else will be made into the commit log
message (together with what you wrote on the Subject: line).

Which means these lines...

> From 78228e3f7c3029d07827f973fa7992777d6e0cb9 Mon Sep 17 00:00:00 2001
> From: slavomir vlcek 
> Date: Wed, 12 Nov 2014 14:10:22 +0100
> Subject: [PATCH] builtin: move builtin retrieval to get_builtin()
>
> There was a redundant code for a builtin command retrieval
> in 'handle_builtin()' and 'is_builtin()'.
>
> This was solved by adding a new function 'get_builtin()'
> and by making a boolean wrapper out of the 'is_builtin()'.
> ---

... will not be part of the log message, which is definitely wrong.

To correct this:

$ git checkout 78228e3f7c3029d0
$ git commit --amend -s --no-edit

to add your sign-off in the log message, then do

$ git format-patch -1 --stdout >patch.mail

Slurp patch.mail into your MUA, move the content on "Subject: " to
where your MUA expects to see the subject line, remove other header
material starting from "From 7822..." so that the message body
begins with "There was a redundant code for...".  And send it out.

>  git.c | 27 +++
>  1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/git.c b/git.c
> index 18fbf79..e32c5b8 100644
> --- a/git.c
> +++ b/git.c
> @@ -487,15 +487,20 @@ static struct cmd_struct commands[] = {
>   { "write-tree", cmd_write_tree, RUN_SETUP },
>  };
>  
> -int is_builtin(const char *s)
> +struct cmd_struct *get_builtin(const char *s)

I do not think this has to be extern.

static struct cmd_struct *get_builtin(const char *s)

perhaps.

> @@ -519,15 +525,12 @@ static void handle_builtin(int argc, const char **argv)
>   argv[0] = cmd = "help";
>   }
>  
> - for (i = 0; i < ARRAY_SIZE(commands); i++) {
> - struct cmd_struct *p = commands+i;
> - if (strcmp(p->cmd, cmd))
> - continue;
> - if (saved_environment && (p->option & NO_SETUP)) {
> + builtin = get_builtin(cmd);

Nice.

> + if (builtin) {
> + if (saved_environment && (builtin->option & NO_SETUP))
>   restore_env();
> - break;
> - }
> - exit(run_builtin(p, argc, argv));
> + else
> + exit(run_builtin(builtin, argc, argv));

This change does not seem to have anything to do with the topic of
the change.  Why is it necessary?

>   }
>  }
--
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: [PATCH v3 1/1] Add another option for receive.denyCurrentBranch

2014-11-13 Thread Junio C Hamano
Johannes Schindelin  writes:

> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index f4da20a..ba002a9 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1330,4 +1330,21 @@ test_expect_success 'fetch into bare respects 
> core.logallrefupdates' '
>   )
>  '
>  
> +test_expect_success 'receive.denyCurrentBranch = updateInstead' '
> + git push testrepo master &&
> + (cd testrepo &&
> + git reset --hard &&
> + git config receive.denyCurrentBranch updateInstead
> + ) &&
> + test_commit third path2 &&
> + git push testrepo master &&
> + test $(git rev-parse HEAD) = $(cd testrepo && git rev-parse HEAD) &&
> + test third = "$(cat testrepo/path2)" &&
> + (cd testrepo &&
> + git update-index --refresh &&
> + git diff-files --quiet &&
> + git diff-index --cached HEAD --
> + )
> +'
> +

This new feature has two sides.  Update when we can and more
importantly fail the update safely.  This tests the "success" case,
but not the "safely fail" one.

For the latter "test_must_fail git push" on the sending side, and
"original HEAD stays the same and the working tree changes are
preserved when there are local changes before the push" on the
receiving side needs to be tested.

Thanks.
--
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: [PATCH v3 0/1] Support updating working trees when pushing into non-bare repos

2014-11-13 Thread Junio C Hamano
Johannes Schindelin  writes:

> This patch series adds support for a new receive.denyCurrentBranch setting
> to update the working directory (which must be clean, i.e. there must not be
> any uncommitted changes) when pushing into the current branch.
>
> The scenario in which the 'updateInstead' setting became a boon in this
> developer's daily work is when trying to get a bug fix from a Windows
> computer, a virtual machine or a user's machine onto his main machine (in
> all of those cases it is only possible to connect via ssh in one direction,
> but not in the reverse direction).
>
> Interdiff vs v2 below the diffstat.
>
> Johannes Schindelin (1):
>   Add another option for receive.denyCurrentBranch
>
>  Documentation/config.txt |  5 
>  builtin/receive-pack.c   | 78 
> ++--
>  t/t5516-fetch-push.sh| 17 +++
>  3 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 4f9fe81..c384515 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2134,10 +2134,6 @@ Another option is "updateInstead" which will update 
> the working
>  directory (must be clean) if pushing into the current branch. This option is
>  intended for synchronizing working directories when one side is not easily
>  accessible via ssh (e.g. inside a VM).
> -+
> -Yet another option is "detachInstead" which will detach the HEAD if updates
> -are pushed into the current branch; That way, the current revision, the
> -index and the working directory are always left untouched by pushes.

I think we had an exchange to clarify the workflow in which
updateInstead is useful and how to help readers, but I do not see
any change on that in this part of documentation.  Forgot to revise?

> @@ -737,36 +733,66 @@ static int update_shallow_ref(struct command *cmd, 
> struct shallow_info *si)
>   return 0;
>  }
>  
> -static const char *merge_worktree(unsigned char *sha1)
> +static const char *update_worktree(unsigned char *sha1)
>  {
>   const char *update_refresh[] = {
>   "update-index", "--ignore-submodules", "--refresh", NULL
>   };
> + const char *diff_index[] = {
> + "diff-index", "--quiet", "--cached", "--ignore-submodules",
> + "HEAD", "--", NULL
> + };
>   const char *read_tree[] = {
>   "read-tree", "-u", "-m", sha1_to_hex(sha1), NULL
>   };

OK.

"update-index --refresh && diff-files && diff-index --cached" is how
we traditionally ensure the working tree is absolutely clean (see
require_clean_work_tree in git-sh-setup.sh), but I do not think of a
reason why diff-files step is not redundant.  As a totally separate
topic outside this series, we may want to visit that shell function.

--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Junio C Hamano
Johannes Schindelin  writes:

> On Thu, 13 Nov 2014, Johannes Schindelin wrote:
>
>> Due to that experience, the documentation also states pretty clearly that
>> `updateInstead` succeeds only in updating the current branch if the
>> working directory is clean.
>
> To clarify why `updateInstead` is stricter than the `merge` scenario: when
> pushing into a remote repository with attached working directory, we
> cannot simply clean up merge conflicts or other problems introduced by a
> merge. Indeed, it is even possible to push without having any option to
> fix files in the working directory afterwards. Therefore, the
> `updateInstead` feature really needs to be adamant about the working
> directory being clean.

As the stricter check (which I think is unnecessarily strict and
which you don't) can be loosened later without making something the
user used to be able to do in an early version unable to do later,
I am OK to accept the design as-is.

But after reading this addendum, I feel the need to double check.

A "reset --keep ", which is the same as "checkout -B
 " (note the lack of "-m"), does not have
any "merge conflict" issues.

To see what I mean, follow along this simple experiment.

# Just make sure we start clean
$ git reset --hard

# Create a playpen
$ git checkout -b throwaway master

# Pick one random path that is different.  We pretend that
# somebody pushed the commit at the tip of 'next' from the side
$ git diff --name-only next | head -n 1
Documentation/git-clone.txt

# Make sure another random path used in the experiment is unchanged
$ git diff next -- COPYING | wc -l
0

# Smudge a path not involved in the branch/HEAD switching
$ echo >>COPYING

# attempt to updateInstead to the other version succeeds.
$ git reset --keep next; echo $?
0
$ git status -suno
 M COPYING
# Notice that we did not get into a funny state.
# You can verify it with "git diff".

# Go back and try smudging what would need a real merge
$ git reset --hard master
$ echo >>Documentation/git-clone.txt

# attempt to updateInstead to the other version
$ git reset --keep next; echo $?
error: Entry 'Documentation/git-clone.txt' not uptodate. Cannot merge.
fatal: Could not reset index file to revision 'next'.
128

# See that HEAD did not move
$ git log HEAD...master | wc -l
0

# See that the working tree change is intact
$ git status -suno
 M Documentation/git-clone.txt
# Notice that we did not get into a funny state.
# You can verify it with "git diff".

The semantics is just as safe as "checkout " without
the "-m" option; if a local change may be clobbered or needs real
file-level merge, the operation fails without touching anything to
preserve the local state.

It is OK for us to agree to disagree, and I am willing to accept the
stricter design for an initial version because loosening it later is
possible without harming users.

But I wanted to first make sure that your disagreement is an
informed one.  Do you still feel the need for "absolutely clean",
even if you take the safety built into "reset --keep" shown in the
above demonstration into account?
--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Junio C Hamano
Johannes Schindelin  writes:

> On Thu, 13 Nov 2014, Johannes Schindelin wrote:
>
>> Due to that experience, the documentation also states pretty clearly that
>> `updateInstead` succeeds only in updating the current branch if the
>> working directory is clean.
>
> To clarify why `updateInstead` is stricter than the `merge` scenario: when
> pushing into a remote repository with attached working directory, we
> cannot simply clean up merge conflicts or other problems introduced by a
> merge. Indeed, it is even possible to push without having any option to
> fix files in the working directory afterwards. Therefore, the
> `updateInstead` feature really needs to be adamant about the working
> directory being clean.

As the stricter check (which I think is unnecessarily strict and
which you don't) can be loosened later without making something the
user used to be able to do in an early version unable to do later,
I am OK to accept the design as-is.

But after reading this addendum, I feel the need to double check.

A "reset --keep ", which is the same as "checkout -B
 " (note the lack of "-m"), does not have
any "merge conflict" issues.

To see what I mean, follow along this simple experiment.

# Just make sure we start clean
$ git reset --hard

# Create a playpen
$ git checkout -b throwaway master

# Pick one random path that is different.  We pretend that
# somebody pushed the commit at the tip of 'next' from the side
$ git diff --name-only next | head -n 1
Documentation/git-clone.txt

# Make sure another random path used in the experiment is unchanged
$ git diff next -- COPYING | wc -l
0

# Smudge a path not involved in the branch/HEAD switching
$ echo >>COPYING

# attempt to updateInstead to the other version succeeds.
$ git reset --keep next; echo $?
0
$ git status -suno
 M COPYING
# Notice that we did not get into a funny state.
# You can verify it with "git diff".

# Go back and try smudging what would need a real merge
$ git reset --hard master
$ echo >>Documentation/git-clone.txt

# attempt to updateInstead to the other version
$ git reset --keep next; echo $?
error: Entry 'Documentation/git-clone.txt' not uptodate. Cannot merge.
fatal: Could not reset index file to revision 'next'.
128

# See that HEAD did not move
$ git log HEAD...master | wc -l
0

# See that the working tree change is intact
$ git status -suno
 M Documentation/git-clone.txt
# Notice that we did not get into a funny state.
# You can verify it with "git diff".

The semantics is just as safe as "checkout " without
the "-m" option; if a local change may be clobbered or needs real
file-level merge, the operation fails without touching anything to
preserve the local state.

It is OK for us to agree to disagree, and I am willing to accept the
stricter design for an initial version because loosening it later is
possible without harming users.

But I wanted to first make sure that your disagreement is an
informed one.  Do you still feel the need for "absolutely clean",
even if you take the safety built into "reset --keep" shown in the
above experiment into account?

--
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 archiving only branch work

2014-11-13 Thread Junio C Hamano
Duy Nguyen  writes:

> On Thu, Nov 13, 2014 at 12:32:40PM +, Graeme Geldenhuys wrote:
>> [alias]
>> deploy = !sh -c 'git archive --prefix=$1/ -o deploy_$1.zip HEAD 
>> $(git diff --name-only -D $2)' -
>> 
>> This works very well. The only problem we have so far is that if we 
>> have files with spaces in the name (eg: SQL update scripts), then the 
>> command breaks.
>> 
>> Does anybody have an idea on how this can be resolved?  Any help would 
>> be much appreciated.

Set $IFS to newline, so that $(git diff --name-only ...) output is
split at record boundaries, not inside pathnames?

A quick experiment you can do to convince yourself may be:

-- >8 --
#!/bin/sh

data () {
echo "a"
echo "b c" ;# SP in between
echo "d e " ;# HT and trailing SP
}

show () {
for i
do
echo "<<$i>>"
done
}

echo ONE
show $(data)

IFS='
'

echo TWO
show $(data)
-- 8< --

On the "git archive" invocation, there may be something that tells
the pathspecs are literal, like '--literal-pathspecs' option, to
avoid metacharacters from being expanded, though.

--
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: [PATCH] gittutorial: fix output of 'git status'

2014-11-13 Thread Junio C Hamano
 writes:

> From: Stefan Naewe 
>
> 'git status' doesn't output leading '#'s these days.
>
> Signed-off-by: Stefan Naewe 
> ---

Thanks, will queue.

>  Documentation/gittutorial-2.txt | 23 ---
>  Documentation/gittutorial.txt   | 17 +
>  2 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/gittutorial-2.txt b/Documentation/gittutorial-2.txt
> index 3109ea8..1901af7 100644
> --- a/Documentation/gittutorial-2.txt
> +++ b/Documentation/gittutorial-2.txt
> @@ -368,17 +368,18 @@ situation:
>  
>  
>  $ git status
> -# On branch master
> -# Changes to be committed:
> -#   (use "git reset HEAD ..." to unstage)
> -#
> -#   new file: closing.txt
> -#
> -# Changes not staged for commit:
> -#   (use "git add ..." to update what will be committed)
> -#
> -#   modified: file.txt
> -#
> +On branch master
> +Changes to be committed:
> +  (use "git reset HEAD ..." to unstage)
> +
> +new file:   closing.txt
> +
> +Changes not staged for commit:
> +  (use "git add ..." to update what will be committed)
> +  (use "git checkout -- ..." to discard changes in working directory)
> +
> +modified:   file.txt
> +
>  
>  
>  Since the current state of closing.txt is cached in the index file,
> diff --git a/Documentation/gittutorial.txt b/Documentation/gittutorial.txt
> index 8262196..8715244 100644
> --- a/Documentation/gittutorial.txt
> +++ b/Documentation/gittutorial.txt
> @@ -107,14 +107,15 @@ summary of the situation with 'git status':
>  
>  
>  $ git status
> -# On branch master
> -# Changes to be committed:
> -#   (use "git reset HEAD ..." to unstage)
> -#
> -#modified:   file1
> -#modified:   file2
> -#modified:   file3
> -#
> +On branch master
> +Changes to be committed:
> +Your branch is up-to-date with 'origin/master'.
> +  (use "git reset HEAD ..." to unstage)
> +
> +modified:   file1
> +modified:   file2
> +modified:   file3
> +
>  
>  
>  If you need to make any further adjustments, do so now, and then add any
--
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: [PATCH] t5705: Use the correct file:// URL

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Nov 13, 2014 at 08:36:07AM +0100, Torsten Bögershausen wrote:
>
>> A URL like file;//. is (no longer) supported by Git:
>> Typically there is no host, and RFC1738 says that file:///
>> should be used.
>> 
>> Update t5705 to use a working URL.
>
> Interesting. This looks like it was unintentionally lost in c59ab2e
> (connect.c: refactor url parsing, 2013-11-28). Given RFC1738, and that
> this is the first notice of it (and that it is not even a real use case,
> but something questionable in the test script), it's probably OK to
> declare the syntax dead and not treat it like a regression.

Yeah, I tend to agree.  I do not think there is any :// that
lets you say things relative to something that is not "root" of the
namespace, and it was a bug the old test depended on that we did not
notice it as an error when "file://." tried to refer to "the current
directory".
--
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: [PATCH] t1410: fix breakage on case-insensitive filesystems

2014-11-13 Thread Junio C Hamano
Jeff King  writes:

> The patch you are responding to is a fix-up for 9233887, which tweaked
> the code and added those tests in the first place (I doubt it would work
> for you, though, as it has a problem on case-insensitive filesystems).
>
>> But the sequence works as expected with a version built
>> in September:
>
> Hmph. So that would mean my theory is not right. Or maybe I am not
> accounting for something else in my analysis.
>
> I guess it is odd that the test right before the failing one passes (it
> is basically that same sequence, with reflogs turned on for both
> operations), which implies that we are properly getting EISDIR. The only
> difference in the failing test is that reflogs are turned off for the
> "git branch one" operation. But I cannot see why that would be broken if
> the other one passes.

Hmph, or perhaps "branch -d one/two" fails to remove the reflog and
does not notice the failure?  But creation of "one" with reflog
disabled shouldn't be affected in such a case, either.  Puzzled...

> I wish it were easy for me to ssh into a Windows VM and run gdb. ;)

likewise.
--
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 archiving only branch work

2014-11-13 Thread Thomas Koch
If your servers run a Unix and you can install Git on the servers than you 
might want to try the install script we use in our company:

https://github.com/comsolit/comsolit_deploy

There's a bare git repository on the server and a post-receive hook that 
exports the content of the git repository in a predefined folder. After that a 
symlink is switched to the new version.

You can run hook scripts after the export (checkout) and after the switch.

The script lacks documentation... (PRs welcome!) But it is unit tested!

Regards, Thomas Koch
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Johan Herland
On Thu, Nov 13, 2014 at 5:03 PM, Fredrik Gustafsson  wrote:
> Thanks for sharing your notes! A few comments:
>
> On Thu, Nov 13, 2014 at 04:44:57PM +0100, Olaf Hering wrote:
>> First clone the remote repository as usual. Then create a local branch for
>> each remote branch that is supposed to be worked on:
>> # git clone git://host/repo.git repo-master
>> # cd repo-master
>> # git checkout -b branchA origin/branchA
>> # git checkout -b branchB origin/branchB
>> # cd -
>>
>> Now clone each work branch into its own directory. The work dir references 
>> the
>> master repo. All changes come from and go into this repo, instead of the
>> remote repo.
>> # git clone -l -b branchA repo-master repo-branchA
>> # git clone -l -b branchB repo-master repo-branchB
>>
>> To make changs in a work dir, commit as usual. The changes will be pushed 
>> from
>> the work copy into the local master repo. Its required to have some other
>> branch than branchA active in repo-master, or push from work copy to
>> repo-master will fail.
>
> That's one of the reason it's not recommended to push into a non-bare
> repository. You should clone your repo-master with the --bare option to
> avoid having a work dir there.
>
>> To publish the outstanding changes its required to do this from the master
>> repo. First checkout the work branch, then pull the local changes and finally
>> push them to the remote repo.
>> # cd repo-master
>> # git checkout branchA
>> # git pull
>> # git push origin branchA
>> # cd -
>
> It's not. You could just add your remote repository as a remote to each
> of your clones of your master repo and push directly from them. It
> would be much simplier and it would allow you to directly fetch changes
> from your remote into your branches as well.
>
> (however, I'm not sure but I think, that this will slowly increase the
> difference between your repositories when you develop. So that they
> won't change any new data since to local clone was made).

Can you not do this much simpler with --reference? Like this:

  $ git clone --bare git://host/repo.git repo-master
  $ git clone -b branchA --reference repo-master git://host/repo.git
repo-branchA
  $ git clone -b branchB --reference repo-master git://host/repo.git
repo-branchB

All three repos now push/pull directly to/from git://host/repo.git,
but repo-branchA and repo-branchB reference objects from within the
bare repo-master. You have to make use to never delete objects from
repo-master (if those objects happen to be referenced from
repo-branchA|B). If you want to prevent the repos growing in size, you
must devise a way to add new objects into repo-master before
repo-branchA|B. (e.g. a nightly cron-job in repo-master that fetches
from origin), so that when repo-branchA|B pulls, they will find most
objects are already present in repo-master.


...Johan

-- 
Johan Herland, 
www.herland.net
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Fredrik Gustafsson
Thanks for sharing your notes! A few comments:

On Thu, Nov 13, 2014 at 04:44:57PM +0100, Olaf Hering wrote:
> First clone the remote repository as usual. Then create a local branch for
> each remote branch that is supposed to be worked on:
> # git clone git://host/repo.git repo-master
> # cd repo-master
> # git checkout -b branchA origin/branchA
> # git checkout -b branchB origin/branchB
> # cd -
>
> Now clone each work branch into its own directory. The work dir references the
> master repo. All changes come from and go into this repo, instead of the
> remote repo.
> # git clone -l -b branchA repo-master repo-branchA
> # git clone -l -b branchB repo-master repo-branchB
>
> To make changs in a work dir, commit as usual. The changes will be pushed from
> the work copy into the local master repo. Its required to have some other
> branch than branchA active in repo-master, or push from work copy to
> repo-master will fail.

That's one of the reason it's not recommended to push into a non-bare
repository. You should clone your repo-master with the --bare option to
avoid having a work dir there.

> To publish the outstanding changes its required to do this from the master
> repo. First checkout the work branch, then pull the local changes and finally
> push them to the remote repo.
> # cd repo-master
> # git checkout branchA
> # git pull
> # git push origin branchA
> # cd -

It's not. You could just add your remote repository as a remote to each
of your clones of your master repo and push directly from them. It
would be much simplier and it would allow you to directly fetch changes
from your remote into your branches as well.

(however, I'm not sure but I think, that this will slowly increase the
difference between your repositories when you develop. So that they
won't change any new data since to local clone was made).

-- 
Med vänlig hälsning
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Olaf Hering
On Thu, Nov 13, Olaf Hering wrote:

> So how can I reduce the disk usage needed for the four .git dirs above?
> I looked around in the docs that came with my git-2.1.3 package, but
> found nothing that answers my question. Maybe we can workout something
> and add it to one of the existing docs.


While playing around with this I made some notes, this is the result:


Manage multiple branches as separate copies


To preserve disk space for each clone, use a master copy of the reop and do
local clones from such copy of a remote repository.

First clone the remote repository as usual. Then create a local branch for
each remote branch that is supposed to be worked on:
# git clone git://host/repo.git repo-master 


   
# cd repo-master


   
# git checkout -b branchA origin/branchA


   
# git checkout -b branchB origin/branchB


   
# cd -  


   

Now clone each work branch into its own directory. The work dir references the
master repo. All changes come from and go into this repo, instead of the
remote repo.
# git clone -l -b branchA repo-master repo-branchA  


   
# git clone -l -b branchB repo-master repo-branchB  


   

To make changs in a work dir, commit as usual. The changes will be pushed from
the work copy into the local master repo. Its required to have some other
branch than branchA active in repo-master, or push from work copy to
repo-master will fail.
# cd repo-master
# git checkout master
# cd -
# cd repo-branchA
# git commit -avs
# git push origin branchA
# cd -

To publish the outstanding changes its required to do this from the master
repo. First checkout the work branch, then pull the local changes and finally
push them to the remote repo.
# cd repo-master
# git checkout branchA
# git pull
# git push origin branchA
# cd -

To receive changes from the remote repo its required to do this from the
master repo. First checkout the work branch, then pull the outstanding remote
changes into the local branch. And finally pull them into the work dir.
# cd repo-master
# git fetch --all (optional)
# git checkout branchB
# git pull
# cd -
# cd repo-branchB
# git pull
# cd -


# vim: set tw=72 et
--
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


[PATCH] replace: fix replacing object with itself

2014-11-13 Thread Manzur Mukhitdinov
When object is replaced with itself git shows unhelpful messages like(git log):
"fatal: replace depth too high for object "

Prevents user from replacing object with itself(with test for checking
this case).

Signed-off-by: Manzur Mukhitdinov 
---
 builtin/replace.c  |  8 +++-
 t/t6050-replace.sh | 11 +--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 294b61b..628377a 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -157,6 +157,9 @@ static int replace_object_sha1(const char *object_ref,
char ref[PATH_MAX];
struct ref_lock *lock;
 
+   if (!hashcmp(object, repl))
+   return error("new object is the same as the old one: '%s'", 
sha1_to_hex(object));
+
obj_type = sha1_object_info(object, NULL);
repl_type = sha1_object_info(repl, NULL);
if (!force && obj_type != repl_type)
@@ -295,9 +298,6 @@ static int edit_and_replace(const char *object_ref, int 
force, int raw)
 
free(tmpfile);
 
-   if (!hashcmp(old, new))
-   return error("new object is the same as the old one: '%s'", 
sha1_to_hex(old));
-
return replace_object_sha1(object_ref, old, "replacement", new, force);
 }
 
@@ -406,8 +406,6 @@ static int create_graft(int argc, const char **argv, int 
force)
 
strbuf_release(&buf);
 
-   if (!hashcmp(old, new))
-   return error("new commit is the same as the old one: '%s'", 
sha1_to_hex(old));
 
return replace_object_sha1(old_ref, old, "replacement", new, force);
 }
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 4d5a25e..5f96374 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -369,9 +369,8 @@ test_expect_success '--edit with and without already 
replaced object' '
git cat-file commit "$PARA3" | grep "A fake Thor"
 '
 
-test_expect_success '--edit and change nothing or command failed' '
+test_expect_success '--edit with failed editor' '
git replace -d "$PARA3" &&
-   test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
test_must_fail env GIT_EDITOR="./fakeeditor;false" git replace --edit 
"$PARA3" &&
GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
git replace -l | grep "$PARA3" &&
@@ -440,4 +439,12 @@ test_expect_success GPG '--graft on a commit with a 
mergetag' '
git replace -d $HASH10
 '
 
+test_expect_success 'replacing object with itself must fail' '
+test_must_fail git replace $HASH1 $HASH1 &&
+HASH8=$(git rev-parse --verify HEAD) &&
+test_must_fail git replace HEAD $HASH8 &&
+test_must_fail git replace --graft HEAD HEAD^ &&
+test_must_fail env GIT_EDITOR=true git replace --edit HEAD
+'
+
 test_done
-- 
1.9.1

--
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: t9902-completion.sh failed

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 05:34:50PM +0600, Alex Kuleshov wrote:

> I'm using ubuntu 14.04 x86_64 and bash GNU bash, version
> 4.3.11(1)-release (x86_64-pc-linux-gnu).
> 
> I didn't applied any patches to bash for all time since i installed
> system. so it reall weird

I tried on a fresh install of Ubuntu 14.04.1, with the same version of
bash, and I still can't reproduce the problem. I'm not sure what else to
check.

-Peff
--
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 archiving only branch work

2014-11-13 Thread Duy Nguyen
On Thu, Nov 13, 2014 at 12:32:40PM +, Graeme Geldenhuys wrote:
> [alias]
> deploy = !sh -c 'git archive --prefix=$1/ -o deploy_$1.zip HEAD 
> $(git diff --name-only -D $2)' -
> 
> This works very well. The only problem we have so far is that if we 
> have files with spaces in the name (eg: SQL update scripts), then the 
> command breaks.
> 
> Does anybody have an idea on how this can be resolved?  Any help would 
> be much appreciated.

I wonder if it's overkill to do something like this patch ("git
archive" may need some more updates for it to work though). With it
you can do:

  git diff --name-only ... | git archive ... HEAD -- ":(file)-"

The good thing is it works for other commands as well. But is it
really a good thing..

-- 8<--
Subject: [PATCH] pathspec: support :(file)

This pathspec magic must be used alone. It reads the actual pathspec
from a given file whose path is specified after :(file). E.g.

  git ls-files :(file)foo

list files specified by pathspec in file "foo". Reading from stdin is
possible to:

  git ls-fiels :(file)-

TODO: specify line terminator..
---
 pathspec.c | 38 ++
 pathspec.h |  4 +++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/pathspec.c b/pathspec.c
index 9304ee3..ba34f9b 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "dir.h"
 #include "pathspec.h"
+#include "argv-array.h"
 
 /*
  * Finds which of the given pathspecs match items in the index.
@@ -72,6 +73,7 @@ static struct pathspec_magic {
{ PATHSPEC_GLOB,   '\0', "glob" },
{ PATHSPEC_ICASE,  '\0', "icase" },
{ PATHSPEC_EXCLUDE, '!', "exclude" },
+   { PATHSPEC_FROMFILE, 0, "file" },
 };
 
 static void prefix_short_magic(struct strbuf *sb, int prefixlen,
@@ -235,6 +237,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
} else if (magic & PATHSPEC_FROMTOP) {
match = xstrdup(copyfrom);
prefixlen = 0;
+   } else if ((magic & PATHSPEC_FROMFILE) && !strcmp(copyfrom, "-")) {
+   match = xstrdup(copyfrom);
+   prefixlen = 0;
} else {
match = prefix_path_gently(prefix, prefixlen, &prefixlen, 
copyfrom);
if (!match)
@@ -354,6 +359,31 @@ static void NORETURN unsupported_magic(const char *pattern,
pattern, sb.buf);
 }
 
+static void pathspec_fromfile(struct pathspec *pathspec,
+ unsigned magic_mask, unsigned flags,
+ const char *prefix, const char *path)
+{
+   struct strbuf buf, nbuf;
+   int line_termination = '\n'; /* FIXME: support :(file:) */
+   struct argv_array av = ARGV_ARRAY_INIT;
+   FILE *fp = !strcmp(path, "-") ? stdin : fopen(path, "r");
+
+   if (!fp)
+   die_errno(_("fail to open %s"), path);
+
+   strbuf_init(&buf, 0);
+   strbuf_init(&nbuf, 0);
+   while (strbuf_getline(&buf, fp, line_termination) != EOF)
+   argv_array_push(&av, buf.buf);
+   strbuf_release(&buf);
+   strbuf_release(&nbuf);
+   if (fp != stdin)
+   fclose(fp);
+   parse_pathspec(pathspec, magic_mask | PATHSPEC_FROMFILE,
+  flags, prefix, av.argv);
+   /* cannot free av because pathspec keeps references to it */
+}
+
 /*
  * Given command line arguments and a prefix, convert the input to
  * pathspec. die() if any magic in magic_mask is used.
@@ -427,6 +457,14 @@ void parse_pathspec(struct pathspec *pathspec,
  item[i].magic & magic_mask,
  short_magic);
 
+   if (item[i].magic & PATHSPEC_FROMFILE) {
+   if (n != 1)
+   die(_(":(file) can only be used alone"));
+   pathspec_fromfile(pathspec,
+ magic_mask | PATHSPEC_FROMFILE,
+ flags, prefix, item[i].match);
+   return;
+   }
if ((flags & PATHSPEC_SYMLINK_LEADING_PATH) &&
has_symlink_leading_path(item[i].match, item[i].len)) {
die(_("pathspec '%s' is beyond a symbolic link"), 
entry);
diff --git a/pathspec.h b/pathspec.h
index 0c11262..84de102 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -8,13 +8,15 @@
 #define PATHSPEC_GLOB  (1<<3)
 #define PATHSPEC_ICASE (1<<4)
 #define PATHSPEC_EXCLUDE   (1<<5)
+#define PATHSPEC_FROMFILE  (1<<6)
 #define PATHSPEC_ALL_MAGIC   \
(PATHSPEC_FROMTOP   | \
 PATHSPEC_MAXDEPTH  | \
 PATHSPEC_LITERAL   | \
 PATHSPEC_GLOB  | \
 PATHSPEC_ICASE | \
-PATHSPEC_EXCLUDE)
+PATHSPEC_EXCLUDE   | \
+PATHSPEC_FROMFILE)
 
 #define PATHSPEC_ONESTAR 1 /* the pathspec pattern satisfies GFNM_ONESTAR 
*/
 
-- 
2.1

Re: Git archiving only branch work

2014-11-13 Thread Peter Krefting

Graeme Geldenhuys:

This works very well. The only problem we have so far is that if we have 
files with spaces in the name (eg: SQL update scripts), then the command 
breaks.


If you add -z to the git diff command-line, it will give you the names 
with nul terminators instead. If you couple that with xargs -0, you 
should be able to do something like this (untested):


deply = !sh -c 'git diff --name-only -z -D $2 | xargs -x -0 git archive --prefix=$1/ -o 
deploy_$1.zip HEAD'


--
\\// Peter - http://www.softwolves.pp.se/
--
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


Git archiving only branch work

2014-11-13 Thread Graeme Geldenhuys

Hi,

I've strung together the following git alias command for our company. 
This command allows us to create a deployment package (archive) for a 
specific feature. The archive will contain only the files that changed 
during the development of that feature. We then deploy that archive to 
our client's web/database server for example.


[alias]
   deploy = !sh -c 'git archive --prefix=$1/ -o deploy_$1.zip HEAD 
$(git diff --name-only -D $2)' -



usage:
  git deploy OPS123 develop..ops-123


'OPS123' is the prefix directory to store the changes in
'ops-123' is the feature branch the work was done in.

This works very well. The only problem we have so far is that if we 
have files with spaces in the name (eg: SQL update scripts), then the 
command breaks.


Does anybody have an idea on how this can be resolved?  Any help would 
be much appreciated.


Not sure if this is useful, but we are working on Windows systems and 
use Git Bash consoles.


Regards,
 Graeme


--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Roger Gammans
On Thu, 2014-11-13 at 12:14 +0100, Olaf Hering wrote:
> How can I reduce the disk usage for multiple copies of the same repo?
> 
> Up to now I just made copies like this, but since .git alone is already
> 2GB it becomes expensive:
> 
>  # git clone git://host/repo.git repo-master
>  # cp -a repo-master repo-branchA
>  # cd repo-branchA
>  # git checkout -b branchA origin/branchA
>  # cd -
>  # cp -a repo-master repo-branchB
>  # cd repo-branchB
>  # git checkout -b branchB origin/branchB
>  # cd -
>  # cp -a repo-master repo-branchB-feature
>  # cd repo-branchB-feature
>  # git checkout -b branchB-feature origin/branchB
>  # cd -
> 
> 
> Since each .git is almost identical I wonder if there is a reliable way
> to "share" it. The "git clone" man page mentions --shared as a dangerous
> way to do things. It does not give an advice how to manage such cloned
> trees.

But you're not using clone you are using cp .

The clone man page also says this:-
  --local, -l
   When the repository to clone from is on a local machine, this flag 
bypasses the normal "Git aware" transport
   mechanism and clones the repository by making a copy of HEAD and 
everything under objects and refs directories. The
   files under .git/objects/ directory are hardlinked to save space 
when possible.

   If the repository is specified as a local path (e.g., 
/path/to/repo), this is the default, and --local is essentially
   a no-op. If the repository is specified as a URL, then this flag is 
ignored (and we never use the local
   optimizations). Specifying --no-local will override the default when 
/path/to/repo is given, using the regular Git
   transport instead.


Note the first sentence of the second paragraph.
 eg:
 # git clone git://host/repo.git repo-master
 # git clone repo-master repo-branchA
 # cd repo-branchA
 # git checkout -b branchA origin/branchA
 # cd -
 # git clone repo-master repo-branchB
 # cd repo-branchB
 # git checkout -b branchB origin/branchB
 # cd -
 # git clone repo-master repo-branchB-feature
 # cd repo-branchB-feature
 # git checkout -b branchB-feature origin/branchB
 # cd -

Should work better for you. And there is probably a way to do it less
commands too.

-- 
Roger Gammans 
--
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: [RFC] On watchman support

2014-11-13 Thread Duy Nguyen
On Thu, Nov 13, 2014 at 12:05 PM, Torsten Bögershausen  wrote:
> From a Git user perspective it could be good to have something like this:
>
> a) git status -u
> b) git status -uno
> c) git status -umtime
> d) git status -uwatchman
>
> We know that a) and b) already exist.
> c) Can be convenient to have, in order to do benchmarking and testing.
>   When the UNTR extension is not found, Git can give an error,
>   saying something like this:
>   No mtime information found, use "git update-index --untracked-cache"
> d) does not yet exist
>
> Of course we may want to configure the default for "git status" in a default 
> variable,
> like status.findUntrackedFiles, which can be empty "", "mtime" or "watchman",
> and we may add other backends later.

While "git status" is in the spotlight, these optimizations have wider
impact. Faster index read/refresh/write helps the majority of
commands. Faster untracked listing hits git-status, git-add,
git-commit -A... This is why I go with environment variable for
temporarily disabling something, or we'll need many config and command
line options, one per command.

> A short test showed that watchman compiles under Mac OS.
> The patch did not compile out of the box (both Git and watchman declare
> there own version of usage(), some C99 complaints from the compiler in 
> watchman,
> nothing that can not be fixed easily)

Yeah it's not perfect. It's mainly to show speeding up refresh with
watchman could be done easily and with low impact

> I will test the mtime patch under networked file systems the next weeks.

Hmm.. you remind me mtime series may have this as an advantage over watchman..
-- 
Duy
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Olaf Hering
On Thu, Nov 13, Roger Gammans wrote:

> Note the first sentence of the second paragraph.
>  eg:
>  # git clone git://host/repo.git repo-master
>  # git clone repo-master repo-branchA
>  # cd repo-branchA
>  # git checkout -b branchA origin/branchA

It fails right here because in this dir only "master" exists, but
branchA is expected.

So far the sequence of commands is:

# git clone git://host/repo.git repo-master
# cd repo-master
# git checkout -b branchA origin/branchA
# git checkout -b branchB origin/branchB
# cd -
# git clone -l -b branchA repo-master repo-branchA
# git clone -l -b branchB repo-master repo-branchB

Next step will be:
# $do_work ; git commit -avs ; git push 

Will that work as expected? Will find out after lunch..


Olaf
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Duy Nguyen
On Thu, Nov 13, 2014 at 6:14 PM, Olaf Hering  wrote:
> Since each .git is almost identical I wonder if there is a reliable way
> to "share" it. The "git clone" man page mentions --shared as a dangerous
> way to do things. It does not give an advice how to manage such cloned
> trees.

If you know what you are doing, you can try git-new-workdir in
contrib/workdir. A safe and reliable version of that is being worked
on, hopefully it'll be released in 2.3.0.
-- 
Duy
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Olaf Hering
On Thu, Nov 13, Fredrik Gustafsson wrote:

> On Thu, Nov 13, 2014 at 12:14:44PM +0100, Olaf Hering wrote:
> > 
> > How can I reduce the disk usage for multiple copies of the same repo?
> 
> You can use --local och --shared. As you say --shared can be dangerous.
> If you don't understand the man page enough to know how you should
> manage your clones you should probably not use it.

Perhaps its more a lack of doc how to use the result.

> --local seems to be what you're looking for.

I will try this.

> However as a side note I'm curious about what your use case is. Why do
> you need this many repos?

Because I do work in each copy, poke around, or do commits and push
them.

> Your setup looks familiar to me for a subversion user switching to git
> and trying to use git as subversion. The common usecase is not to have
> multiple worktrees but to do a checkout to the worktree you need to work
> on. This is possible with git since it's very fast and I recommend you
> to try to use one worktree.

Switching branches will invalidate timestamps, causing a full rebuild.

Olaf
--
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: how to reduce disk usage for large .git dirs?

2014-11-13 Thread Fredrik Gustafsson
On Thu, Nov 13, 2014 at 12:14:44PM +0100, Olaf Hering wrote:
> 
> How can I reduce the disk usage for multiple copies of the same repo?

You can use --local och --shared. As you say --shared can be dangerous.
If you don't understand the man page enough to know how you should
manage your clones you should probably not use it.

--local seems to be what you're looking for.

However as a side note I'm curious about what your use case is. Why do
you need this many repos?

Your setup looks familiar to me for a subversion user switching to git
and trying to use git as subversion. The common usecase is not to have
multiple worktrees but to do a checkout to the worktree you need to work
on. This is possible with git since it's very fast and I recommend you
to try to use one worktree.

-- 
Med vänlig hälsning
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
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: [PATCH] git-send-email.perl: Fix handling of suppresscc option.

2014-11-13 Thread Jens Stimpfle
Junio, thanks for reviewing.

On Wed, Nov 12, 2014 at 10:25:14AM -0800, Junio C Hamano wrote:
> "When I say addresses on Cc: does not matter, it doesn't.  No matter
> what the address in question is" (likewise for S-o-b:) is what the
> updated logic says.  It is easier to explain than the traditional
> "The way to squelch my address is by 'suppress self'; for all other
> addresses on Cc:/S-o-b:, there are separate suppression methods".
> 
> But I have a slight suspicion that this special casing of 'self' was
> done on purpose, and people may be relying on it.

It seems the behaviour is exactly described in the manpage. Sorry for
having missed that.

The behaviour is super-complex and confusing, though. It's easy to
imagine much simpler, intuitive and "transparent" ones.

But I have no experience with distributed git workflows so I'm
absolutely not in the position to call it broken or provide a "fix".

Sorry for the noise.
--
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: t9902-completion.sh failed

2014-11-13 Thread Alex Kuleshov

Hello Jeff,

I'm using ubuntu 14.04 x86_64 and bash GNU bash, version
4.3.11(1)-release (x86_64-pc-linux-gnu).

I didn't applied any patches to bash for all time since i installed
system. so it reall weird


Jeff King  @ 2014-11-13 17:24 ALMT:

> On Thu, Nov 13, 2014 at 04:59:12PM +0600, Alex Kuleshov wrote:
>
>> i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
>> built and installed it successfully, now i'm running make test and got
>> following error:
>>
>> *** t9902-completion.sh ***
>> t9902-completion.sh: 118:
>> /home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
>> error: "(" unexpected (expecting "fi")
>> FATAL: Unexpected exit with code 2
>> make[2]: *** [t9902-completion.sh] Error 1
>> make[2]: Leaving directory `/home/shk/dev/git/t'
>> make[1]: *** [test] Error 2
>> make[1]: Leaving directory `/home/shk/dev/git/t'
>> make: *** [test] Error 2
>>
>> $ bash --version
>> 4.3.11(1)-release (x86_64-pc-linux-gnu)
>
> Weird. I can't reproduce here, using the version of bash from Debian
> unstable (4.3.30(1)), nor compiling 4.3.11 from sources. What platform
> are you on (i.e., might it be bash + some other patches installed by the
> distro)?
>
> -Peff

--
Best regards.
0xAX
--
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: t9902-completion.sh failed

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 04:59:12PM +0600, Alex Kuleshov wrote:

> i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
> built and installed it successfully, now i'm running make test and got
> following error:
> 
> *** t9902-completion.sh ***
> t9902-completion.sh: 118:
> /home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
> error: "(" unexpected (expecting "fi")
> FATAL: Unexpected exit with code 2
> make[2]: *** [t9902-completion.sh] Error 1
> make[2]: Leaving directory `/home/shk/dev/git/t'
> make[1]: *** [test] Error 2
> make[1]: Leaving directory `/home/shk/dev/git/t'
> make: *** [test] Error 2
> 
> $ bash --version
> 4.3.11(1)-release (x86_64-pc-linux-gnu)

Weird. I can't reproduce here, using the version of bash from Debian
unstable (4.3.30(1)), nor compiling 4.3.11 from sources. What platform
are you on (i.e., might it be bash + some other patches installed by the
distro)?

-Peff
--
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


how to reduce disk usage for large .git dirs?

2014-11-13 Thread Olaf Hering

How can I reduce the disk usage for multiple copies of the same repo?

Up to now I just made copies like this, but since .git alone is already
2GB it becomes expensive:

 # git clone git://host/repo.git repo-master
 # cp -a repo-master repo-branchA
 # cd repo-branchA
 # git checkout -b branchA origin/branchA
 # cd -
 # cp -a repo-master repo-branchB
 # cd repo-branchB
 # git checkout -b branchB origin/branchB
 # cd -
 # cp -a repo-master repo-branchB-feature
 # cd repo-branchB-feature
 # git checkout -b branchB-feature origin/branchB
 # cd -


Since each .git is almost identical I wonder if there is a reliable way
to "share" it. The "git clone" man page mentions --shared as a dangerous
way to do things. It does not give an advice how to manage such cloned
trees.

So how can I reduce the disk usage needed for the four .git dirs above?
I looked around in the docs that came with my git-2.1.3 package, but
found nothing that answers my question. Maybe we can workout something
and add it to one of the existing docs.

Thanks!

Olaf
--
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


[PATCH 2/2] approxidate: allow ISO-like dates far in the future

2014-11-13 Thread Jeff King
When we are parsing approxidate strings and we find three
numbers separate by one of ":/-.", we guess that it may be a
date. We feed the numbers to match_multi_number, which
checks whether it makes sense as a date in various orderings
(e.g., dd/mm/yy or mm/dd/yy, etc).

One of the checks we do is to see whether it is a date more
than 10 days in the future. This was added in 38035cf (date
parsing: be friendlier to our European friends.,
2006-04-05), and lets us guess that if it is currently April
2014, then "10/03/2014" is probably March 10th, not October
3rd.

This has a downside, though; if you want to be overly
generous with your "--until" date specification, we may
wrongly parse "2014-12-01" as "2014-01-12" (because the
latter is an in-the-past date). If the year is a future year
(i.e., both are future dates), it gets even weirder. Due to
the vagaries of approxidate, months _after_ the current date
(no matter the year) get flipped, but ones before do not.

This patch drops the "in the future" check for dates of this
form, letting us treat them always as -mm-dd, even if
they are in the future. This does not affect the normal
dd/mm/ versus mm/dd/ lookup, because this code path
only kicks in when the first number is greater than 70
(i.e., it must be a year, and cannot be either a date or a
month).

The one possible casualty is that "-dd-mm" is less
likely to be chosen over "-mm-dd". That's probably OK,
though because:

  1. The difference happens only when the date is in the
 future. Already we prefer -mm-dd for dates in the
 past.

  2. It's unclear whether anybody even uses -dd-mm
 regularly. It does not appear in lists of common date
 formats in Wikipedia[1,2].

  3. Even if (2) is wrong, it is better to prefer ISO-like
 dates, as that is consistent with what we use elsewhere
 in git.

[1] http://en.wikipedia.org/wiki/Date_and_time_representation_by_country
[2] http://en.wikipedia.org/wiki/Calendar_date

Reported-by: Colin Smith 
Signed-off-by: Jeff King 
---
I did not single out "-mm-dd" here versus "/mm/dd"; the change
applies no matter which separator is used. If we wanted to be more
conservative, we could apply it only to the more ISO-looking form with
dashes, but I tend to think the reasoning makes sense no matter which
separate you use (i.e., I do not think any variant with year then day is
in common use).

 date.c  | 29 -
 t/t0006-date.sh |  3 +++
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/date.c b/date.c
index e1a4d49..b9eecfb 100644
--- a/date.c
+++ b/date.c
@@ -363,7 +363,8 @@ static int match_alpha(const char *date, struct tm *tm, int 
*offset)
return skip_alpha(date);
 }
 
-static int is_date(int year, int month, int day, struct tm *now_tm, time_t 
now, struct tm *tm)
+static int is_date(int year, int month, int day, struct tm *now_tm, time_t now,
+  struct tm *tm, int allow_future)
 {
if (month > 0 && month < 13 && day > 0 && day < 32) {
struct tm check = *tm;
@@ -388,14 +389,16 @@ static int is_date(int year, int month, int day, struct 
tm *now_tm, time_t now,
if (!now_tm)
return 1;
 
-   specified = tm_to_time_t(r);
+   if (!allow_future) {
+   specified = tm_to_time_t(r);
 
-   /* Be it commit time or author time, it does not make
-* sense to specify timestamp way into the future.  Make
-* sure it is not later than ten days from now...
-*/
-   if ((specified != -1) && (now + 10*24*3600 < specified))
-   return 0;
+   /* Be it commit time or author time, it does not make
+* sense to specify timestamp way into the future.  Make
+* sure it is not later than ten days from now...
+*/
+   if ((specified != -1) && (now + 10*24*3600 < specified))
+   return 0;
+   }
tm->tm_mon = r->tm_mon;
tm->tm_mday = r->tm_mday;
if (year != -1)
@@ -441,10 +444,10 @@ static int match_multi_number(unsigned long num, char c, 
const char *date,
 
if (num > 70) {
/* -mm-dd? */
-   if (is_date(num, num2, num3, refuse_future, now, tm))
+   if (is_date(num, num2, num3, refuse_future, now, tm, 1))
break;
/* -dd-mm? */
-   if (is_date(num, num3, num2, refuse_future, now, tm))
+   if (is_date(num, num3, num2, refuse_future, now, tm, 1))
break;
}
/* Our eastern European friends say dd.mm.yy[yy]
@@ -452,14 +455,14 @@ static int match_multi_number

[PATCH 1/2] pass TIME_DATE_NOW to approxidate future-check

2014-11-13 Thread Jeff King
The approxidate functions accept an extra "now" parameter to
avoid calling time() themselves. We use this in our test
suite to make sure we have a consistent time for computing
relative dates. However, deep in the bowels of approxidate,
we also call time() to check whether possible dates are far
in the future. Let's make sure that the "now" override makes
it to that spot, too, so we can consistently test that
feature.

Signed-off-by: Jeff King 
---
This code path also gets called from the more-strict parse_date (which
does not know about the current time). This continues to call time()
dynamically, but it is not clear to me if that ever actually matters in
practice.

 date.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/date.c b/date.c
index 59dfe57..e1a4d49 100644
--- a/date.c
+++ b/date.c
@@ -405,9 +405,9 @@ static int is_date(int year, int month, int day, struct tm 
*now_tm, time_t now,
return 0;
 }
 
-static int match_multi_number(unsigned long num, char c, const char *date, 
char *end, struct tm *tm)
+static int match_multi_number(unsigned long num, char c, const char *date,
+ char *end, struct tm *tm, time_t now)
 {
-   time_t now;
struct tm now_tm;
struct tm *refuse_future;
long num2, num3;
@@ -433,7 +433,8 @@ static int match_multi_number(unsigned long num, char c, 
const char *date, char
case '-':
case '/':
case '.':
-   now = time(NULL);
+   if (!now)
+   now = time(NULL);
refuse_future = NULL;
if (gmtime_r(&now, &now_tm))
refuse_future = &now_tm;
@@ -513,7 +514,7 @@ static int match_digit(const char *date, struct tm *tm, int 
*offset, int *tm_gmt
case '/':
case '-':
if (isdigit(end[1])) {
-   int match = match_multi_number(num, *end, date, end, 
tm);
+   int match = match_multi_number(num, *end, date, end, 
tm, 0);
if (match)
return match;
}
@@ -1013,7 +1014,8 @@ static const char *approxidate_alpha(const char *date, 
struct tm *tm, struct tm
return end;
 }
 
-static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
+static const char *approxidate_digit(const char *date, struct tm *tm, int *num,
+time_t now)
 {
char *end;
unsigned long number = strtoul(date, &end, 10);
@@ -1024,7 +1026,8 @@ static const char *approxidate_digit(const char *date, 
struct tm *tm, int *num)
case '/':
case '-':
if (isdigit(end[1])) {
-   int match = match_multi_number(number, *end, date, end, 
tm);
+   int match = match_multi_number(number, *end, date, end,
+  tm, now);
if (match)
return date + match;
}
@@ -1087,7 +1090,7 @@ static unsigned long approxidate_str(const char *date,
date++;
if (isdigit(c)) {
pending_number(&tm, &number);
-   date = approxidate_digit(date-1, &tm, &number);
+   date = approxidate_digit(date-1, &tm, &number, 
time_sec);
touched = 1;
continue;
}
-- 
2.1.2.596.g7379948

--
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


[PATCH v3 0/1] Support updating working trees when pushing into non-bare repos

2014-11-13 Thread Johannes Schindelin
This patch series adds support for a new receive.denyCurrentBranch setting
to update the working directory (which must be clean, i.e. there must not be
any uncommitted changes) when pushing into the current branch.

The scenario in which the 'updateInstead' setting became a boon in this
developer's daily work is when trying to get a bug fix from a Windows
computer, a virtual machine or a user's machine onto his main machine (in
all of those cases it is only possible to connect via ssh in one direction,
but not in the reverse direction).

Interdiff vs v2 below the diffstat.

Johannes Schindelin (1):
  Add another option for receive.denyCurrentBranch

 Documentation/config.txt |  5 
 builtin/receive-pack.c   | 78 ++--
 t/t5516-fetch-push.sh| 17 +++
 3 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4f9fe81..c384515 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2134,10 +2134,6 @@ Another option is "updateInstead" which will update the 
working
 directory (must be clean) if pushing into the current branch. This option is
 intended for synchronizing working directories when one side is not easily
 accessible via ssh (e.g. inside a VM).
-+
-Yet another option is "detachInstead" which will detach the HEAD if updates
-are pushed into the current branch; That way, the current revision, the
-index and the working directory are always left untouched by pushes.
 
 receive.denyNonFastForwards::
If set to true, git-receive-pack will deny a ref update which is
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4534e88..836720f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -27,8 +27,7 @@ enum deny_action {
DENY_IGNORE,
DENY_WARN,
DENY_REFUSE,
-   DENY_UPDATE_INSTEAD,
-   DENY_DETACH_INSTEAD
+   DENY_UPDATE_INSTEAD
 };
 
 static int deny_deletes;
@@ -78,6 +77,8 @@ static enum deny_action parse_deny_action(const char *var, 
const char *value)
return DENY_WARN;
if (!strcasecmp(value, "refuse"))
return DENY_REFUSE;
+   if (!strcasecmp(value, "updateinstead"))
+   return DENY_UPDATE_INSTEAD;
}
if (git_config_bool(var, value))
return DENY_REFUSE;
@@ -122,12 +123,7 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
}
 
if (!strcmp(var, "receive.denycurrentbranch")) {
-   if (value && !strcasecmp(value, "updateinstead"))
-   deny_current_branch = DENY_UPDATE_INSTEAD;
-   else if (value && !strcasecmp(value, "detachinstead"))
-   deny_current_branch = DENY_DETACH_INSTEAD;
-   else
-   deny_current_branch = parse_deny_action(var, value);
+   deny_current_branch = parse_deny_action(var, value);
return 0;
}
 
@@ -737,36 +733,66 @@ static int update_shallow_ref(struct command *cmd, struct 
shallow_info *si)
return 0;
 }
 
-static const char *merge_worktree(unsigned char *sha1)
+static const char *update_worktree(unsigned char *sha1)
 {
const char *update_refresh[] = {
"update-index", "--ignore-submodules", "--refresh", NULL
};
+   const char *diff_index[] = {
+   "diff-index", "--quiet", "--cached", "--ignore-submodules",
+   "HEAD", "--", NULL
+   };
const char *read_tree[] = {
"read-tree", "-u", "-m", sha1_to_hex(sha1), NULL
};
+   const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : "..";
+   struct argv_array env = ARGV_ARRAY_INIT;
struct child_process child = CHILD_PROCESS_INIT;
 
if (is_bare_repository())
return "denyCurrentBranch = updateInstead needs a worktree";
 
-   argv_array_pushf(&child.env_array, "GIT_DIR=%s", 
absolute_path(get_git_dir()));
+   argv_array_pushf(&env, "GIT_DIR=%s", absolute_path(get_git_dir()));
+
child.argv = update_refresh;
-   child.dir = git_work_tree_cfg ? git_work_tree_cfg : "..";
+   child.env = env.argv;
+   child.dir = work_tree;
+   child.no_stdin = 1;
child.stdout_to_stderr = 1;
child.git_cmd = 1;
-   if (run_command(&child))
-   die("Could not refresh the index");
+   if (run_command(&child)) {
+   argv_array_clear(&env);
+   return "Up-to-date check failed";
+   }
 
-   /* finish_command cleared the environment; reinitialize */
-   argv_array_pushf(&child.env_array, "GIT_DIR=%s", 
absolute_path(get_git_dir()));
+   /* run_command() does not clean up completely; reinitialize */
+   child_process_init(&child);
+   child.argv = diff_index;
+   child.env = env.argv;
+   child.no_stdin

[PATCH v3 1/1] Add another option for receive.denyCurrentBranch

2014-11-13 Thread Johannes Schindelin
When synchronizing between working directories, it can be handy to update
the current branch via 'push' rather than 'pull', e.g. when pushing a fix
from inside a VM, or when pushing a fix made on a user's machine (where
the developer is not at liberty to install an ssh daemon let alone know
the user's password).

The common workaround – pushing into a temporary branch and then merging
on the other machine – is no longer necessary with this patch.

The new option is:

'updateInstead':
Update the working tree accordingly, but refuse to do so if there
are any uncommitted changes.

Signed-off-by: Johannes Schindelin 
---
 Documentation/config.txt |  5 
 builtin/receive-pack.c   | 78 ++--
 t/t5516-fetch-push.sh| 17 +++
 3 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9220725..c384515 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2129,6 +2129,11 @@ receive.denyCurrentBranch::
print a warning of such a push to stderr, but allow the push to
proceed. If set to false or "ignore", allow such pushes with no
message. Defaults to "refuse".
++
+Another option is "updateInstead" which will update the working
+directory (must be clean) if pushing into the current branch. This option is
+intended for synchronizing working directories when one side is not easily
+accessible via ssh (e.g. inside a VM).
 
 receive.denyNonFastForwards::
If set to true, git-receive-pack will deny a ref update which is
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 32fc540..836720f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -26,7 +26,8 @@ enum deny_action {
DENY_UNCONFIGURED,
DENY_IGNORE,
DENY_WARN,
-   DENY_REFUSE
+   DENY_REFUSE,
+   DENY_UPDATE_INSTEAD
 };
 
 static int deny_deletes;
@@ -76,6 +77,8 @@ static enum deny_action parse_deny_action(const char *var, 
const char *value)
return DENY_WARN;
if (!strcasecmp(value, "refuse"))
return DENY_REFUSE;
+   if (!strcasecmp(value, "updateinstead"))
+   return DENY_UPDATE_INSTEAD;
}
if (git_config_bool(var, value))
return DENY_REFUSE;
@@ -730,11 +733,74 @@ static int update_shallow_ref(struct command *cmd, struct 
shallow_info *si)
return 0;
 }
 
+static const char *update_worktree(unsigned char *sha1)
+{
+   const char *update_refresh[] = {
+   "update-index", "--ignore-submodules", "--refresh", NULL
+   };
+   const char *diff_index[] = {
+   "diff-index", "--quiet", "--cached", "--ignore-submodules",
+   "HEAD", "--", NULL
+   };
+   const char *read_tree[] = {
+   "read-tree", "-u", "-m", sha1_to_hex(sha1), NULL
+   };
+   const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : "..";
+   struct argv_array env = ARGV_ARRAY_INIT;
+   struct child_process child = CHILD_PROCESS_INIT;
+
+   if (is_bare_repository())
+   return "denyCurrentBranch = updateInstead needs a worktree";
+
+   argv_array_pushf(&env, "GIT_DIR=%s", absolute_path(get_git_dir()));
+
+   child.argv = update_refresh;
+   child.env = env.argv;
+   child.dir = work_tree;
+   child.no_stdin = 1;
+   child.stdout_to_stderr = 1;
+   child.git_cmd = 1;
+   if (run_command(&child)) {
+   argv_array_clear(&env);
+   return "Up-to-date check failed";
+   }
+
+   /* run_command() does not clean up completely; reinitialize */
+   child_process_init(&child);
+   child.argv = diff_index;
+   child.env = env.argv;
+   child.no_stdin = 1;
+   child.no_stdout = 1;
+   child.stdout_to_stderr = 0;
+   child.git_cmd = 1;
+   if (run_command(&child)) {
+   argv_array_clear(&env);
+   return "Working directory not clean";
+   }
+
+   /* run_command() does not clean up completely; reinitialize */
+   child_process_init(&child);
+   child.argv = read_tree;
+   child.env = env.argv;
+   child.dir = work_tree;
+   child.no_stdin = 1;
+   child.no_stdout = 1;
+   child.stdout_to_stderr = 0;
+   child.git_cmd = 1;
+   if (run_command(&child)) {
+   argv_array_clear(&env);
+   return "Could not update working tree to new HEAD";
+   }
+
+   argv_array_clear(&env);
+   return NULL;
+}
+
 static const char *update(struct command *cmd, struct shallow_info *si)
 {
const char *name = cmd->ref_name;
struct strbuf namespaced_name_buf = STRBUF_INIT;
-   const char *namespaced_name;
+   const char *namespaced_name, *ret;
unsigned char *old_sha1 = cmd->old_sha1;
unsigned char *new_sha1 = cmd->new_sha1;
 
@@ -7

[PATCH 0/2] approxidate and future ISO-like times

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 04:36:06AM -0500, Jeff King wrote:

> On Thu, Nov 13, 2014 at 11:27:06AM +1100, Colin Smith wrote:
> 
> > Apologies if this has already been raised or PEBCAK, but I've noticed
> > a bug where git log with certain date ranges breaks things. It appears
> > to be any --since date with a --until date in the future between
> > 2014-12-01 and 2014-12-09. Dates from 2014-12-10 appear to work, and
> > so does the date 2015-12-01.
> 
> Ugh. Approxidate strikes again:
> 
>   for i in 2014-11-01 2013-12-01 2014-12-01; do
> ./test-date approxidate $i
>   done
> 
> produces:
> 
>   2014-11-01 -> 2014-11-01 09:35:19 +
>   2013-12-01 -> 2013-12-01 09:35:19 +
>   2014-12-01 -> 2014-01-12 09:35:19 +
> 
> The first two are right, but the fourth one is not.  It's probably
> something simple and stupid.

Less simple and stupid than I thought, but I think I have a fix. It is
not about December specifically, but about the date being in the future.
The first patch is a cleanup to help us more accurately test the bug;
the interesting bits are in the second one.

  [1/2]: pass TIME_DATE_NOW to approxidate future-check
  [2/2]: approxidate: allow ISO-like dates far in the future

-Peff
--
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


t9902-completion.sh failed

2014-11-13 Thread Alex Kuleshov

Hello all,

i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
built and installed it successfully, now i'm running make test and got
following error:

*** t9902-completion.sh ***
t9902-completion.sh: 118:
/home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
error: "(" unexpected (expecting "fi")
FATAL: Unexpected exit with code 2
make[2]: *** [t9902-completion.sh] Error 1
make[2]: Leaving directory `/home/shk/dev/git/t'
make[1]: *** [test] Error 2
make[1]: Leaving directory `/home/shk/dev/git/t'
make: *** [test] Error 2

$ bash --version
4.3.11(1)-release (x86_64-pc-linux-gnu)

--
Best regards.
0xAX
--
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


[PATCH] gittutorial: fix output of 'git status'

2014-11-13 Thread stefan.naewe
From: Stefan Naewe 

'git status' doesn't output leading '#'s these days.

Signed-off-by: Stefan Naewe 
---
 Documentation/gittutorial-2.txt | 23 ---
 Documentation/gittutorial.txt   | 17 +
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/Documentation/gittutorial-2.txt b/Documentation/gittutorial-2.txt
index 3109ea8..1901af7 100644
--- a/Documentation/gittutorial-2.txt
+++ b/Documentation/gittutorial-2.txt
@@ -368,17 +368,18 @@ situation:
 
 
 $ git status
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD ..." to unstage)
-#
-#   new file: closing.txt
-#
-# Changes not staged for commit:
-#   (use "git add ..." to update what will be committed)
-#
-#   modified: file.txt
-#
+On branch master
+Changes to be committed:
+  (use "git reset HEAD ..." to unstage)
+
+new file:   closing.txt
+
+Changes not staged for commit:
+  (use "git add ..." to update what will be committed)
+  (use "git checkout -- ..." to discard changes in working directory)
+
+modified:   file.txt
+
 
 
 Since the current state of closing.txt is cached in the index file,
diff --git a/Documentation/gittutorial.txt b/Documentation/gittutorial.txt
index 8262196..8715244 100644
--- a/Documentation/gittutorial.txt
+++ b/Documentation/gittutorial.txt
@@ -107,14 +107,15 @@ summary of the situation with 'git status':
 
 
 $ git status
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD ..." to unstage)
-#
-#  modified:   file1
-#  modified:   file2
-#  modified:   file3
-#
+On branch master
+Changes to be committed:
+Your branch is up-to-date with 'origin/master'.
+  (use "git reset HEAD ..." to unstage)
+
+modified:   file1
+modified:   file2
+modified:   file3
+
 
 
 If you need to make any further adjustments, do so now, and then add any
-- 
1.9.2.msysgit.0.2270.g8768113

--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Johannes Schindelin
Hi Junio,

On Thu, 13 Nov 2014, Johannes Schindelin wrote:

> Due to that experience, the documentation also states pretty clearly that
> `updateInstead` succeeds only in updating the current branch if the
> working directory is clean.

To clarify why `updateInstead` is stricter than the `merge` scenario: when
pushing into a remote repository with attached working directory, we
cannot simply clean up merge conflicts or other problems introduced by a
merge. Indeed, it is even possible to push without having any option to
fix files in the working directory afterwards. Therefore, the
`updateInstead` feature really needs to be adamant about the working
directory being clean.

Ciao,
Johannes
--
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: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

2014-11-13 Thread Johannes Schindelin
Hi Junio,

On Wed, 12 Nov 2014, Junio C Hamano wrote:

> Instead of running "update-index --refresh; read-tree -m -u", using
> "reset --keep" may be a better implementation of what you are trying to
> do here.

I do not think that `reset --keep` is what I want. I really want to update
only if the working directory is clean. So I guess I will have to bite the
bullet and test the output of `update-index --refresh`, `diff-index
--quiet --cached HEAD --`.

In my case, the lacking test whether there are staged changes did not
matter, just because I pretty much never leave staged changes around.

What did matter, however, was to make sure that I did not update the
working directory carelessly. In one case, that `update-index --refresh`
test really helped me out because I was about to push  into a working
directory with uncommitted changes inside some web space. That push would
have broken the web application because of the local changes, so I was
really, really happy that I decided to be quite strict in the
implementation of `updateInstead`.

Due to that experience, the documentation also states pretty clearly that
`updateInstead` succeeds only in updating the current branch if the
working directory is clean.

Ciao,
Johannes
--
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: Bug: git log showing nothing when using --since and --until flags with specific dates

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 11:27:06AM +1100, Colin Smith wrote:

> Apologies if this has already been raised or PEBCAK, but I've noticed
> a bug where git log with certain date ranges breaks things. It appears
> to be any --since date with a --until date in the future between
> 2014-12-01 and 2014-12-09. Dates from 2014-12-10 appear to work, and
> so does the date 2015-12-01.

Ugh. Approxidate strikes again:

  for i in 2014-11-01 2013-12-01 2014-12-01; do
./test-date approxidate $i
  done

produces:

  2014-11-01 -> 2014-11-01 09:35:19 +
  2013-12-01 -> 2013-12-01 09:35:19 +
  2014-12-01 -> 2014-01-12 09:35:19 +

The first two are right, but the fourth one is not.  It's probably
something simple and stupid.

-Peff
--
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: [PATCH] t5705: Use the correct file:// URL

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 08:36:07AM +0100, Torsten Bögershausen wrote:

> A URL like file;//. is (no longer) supported by Git:
> Typically there is no host, and RFC1738 says that file:///
> should be used.
> 
> Update t5705 to use a working URL.

Interesting. This looks like it was unintentionally lost in c59ab2e
(connect.c: refactor url parsing, 2013-11-28). Given RFC1738, and that
this is the first notice of it (and that it is not even a real use case,
but something questionable in the test script), it's probably OK to
declare the syntax dead and not treat it like a regression.

-Peff
--
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: [PATCH] t1410: fix breakage on case-insensitive filesystems

2014-11-13 Thread Jeff King
On Thu, Nov 13, 2014 at 09:50:24AM +0100, Johannes Sixt wrote:

> >That looks more like it is failing the actual test (i.e., the creation
> >of branch "one" when there is cruft in the reflog). My guess is that
> >calling open() on a directory is giving us EACCES instead of EISDIR. Can
> >you verify that?
> >
> >If that is the case, then this isn't a new breakage, I think, but just
> >code we weren't previously exercising. It would be interesting to know
> >whether:
> >
> >   git config core.logallrefupdates true
> >   git branch one/two
> >   git branch -d one/two
> >   git branch one
> >
> >works (even without my patch). If so, then there's probably something
> >else going on.
> 
> Don't know what you mean with "my patch" (the one I was responding to
> touches only t1410).

The patch you are responding to is a fix-up for 9233887, which tweaked
the code and added those tests in the first place (I doubt it would work
for you, though, as it has a problem on case-insensitive filesystems).

> But the sequence works as expected with a version built
> in September:

Hmph. So that would mean my theory is not right. Or maybe I am not
accounting for something else in my analysis.

I guess it is odd that the test right before the failing one passes (it
is basically that same sequence, with reflogs turned on for both
operations), which implies that we are properly getting EISDIR. The only
difference in the failing test is that reflogs are turned off for the
"git branch one" operation. But I cannot see why that would be broken if
the other one passes.

I wish it were easy for me to ssh into a Windows VM and run gdb. ;)

-Peff
--
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: [PATCH] t1410: fix breakage on case-insensitive filesystems

2014-11-13 Thread Johannes Sixt

Am 12.11.2014 22:59, schrieb Jeff King:

On Wed, Nov 12, 2014 at 09:20:22PM +0100, Johannes Sixt wrote:


Am 09.11.2014 um 02:59 schrieb Jeff King:

  test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' '
-   test_when_finished "git branch -d a || git branch -d a/b" &&
+   test_when_finished "git branch -d one || git branch -d one/two" &&

-   git branch a/b master &&
-   echo "a/b@{0} branch: Created from master" >expect &&
-   git log -g --format="%gd %gs" a/b >actual &&
+   git branch one/two master &&
+   echo "one/two@{0} branch: Created from master" >expect &&
+   git log -g --format="%gd %gs" one/two >actual &&
test_cmp expect actual &&
-   git branch -d a/b &&
+   git branch -d one/two &&

-   # same as before, but we only create a reflog for "a" if
+   # same as before, but we only create a reflog for "one" if
# it already exists, which it does not
-   git -c core.logallrefupdates=false branch a master &&
+   git -c core.logallrefupdates=false branch one master &&
: >expect &&
-   git log -g --format="%gd %gs" a >actual &&
+   git log -g --format="%gd %gs" one >actual &&
test_cmp expect actual
  '



On Linux I observe

Deleted branch one/two (was b60a214).
warning: unable to unlink .git/logs/refs/heads/one: Is a directory
Deleted branch one (was b60a214).
ok 15 - stale dirs do not cause d/f conflicts (reflogs off)

(notice the warning)


Yes, this is expected. The warning actually comes from the "git branch
-d" run during cleanup. Branch "one" exists but has no reflog. Instead
there is a crufty dir call "logs/refs/heads/one". The deletion process
blindly calls "unlink" on it and complains when it fails for reasons
other than ENOENT.

We could suppress that warning, but I didn't think it was really worth
the bother. This is such a funny setup (reflogs on, then off, then on,
_and_ a d/f conflict in the branches) that I doubt it will come up much.

I seem to recall that ancient versions of SunOS used to do bad things
when you called "unlink" on a directory, but I do not know if that is
still the case (I also would doubt this is the only place in git where
we blindly do an "unlink if you can" without actually checking the file.


but on Windows the test fails with

Deleted branch one/two (was b60a214).
error: Unable to append to .git/logs/refs/heads/one: Permission denied
fatal: Cannot update the ref 'refs/heads/one'.
not ok 15 - stale dirs do not cause d/f conflicts (reflogs off)


That looks more like it is failing the actual test (i.e., the creation
of branch "one" when there is cruft in the reflog). My guess is that
calling open() on a directory is giving us EACCES instead of EISDIR. Can
you verify that?

If that is the case, then this isn't a new breakage, I think, but just
code we weren't previously exercising. It would be interesting to know
whether:

   git config core.logallrefupdates true
   git branch one/two
   git branch -d one/two
   git branch one

works (even without my patch). If so, then there's probably something
else going on.


Don't know what you mean with "my patch" (the one I was responding to 
touches only t1410). But the sequence works as expected with a version 
built in September:


C:\Temp\gittest>git init
Initialized empty Git repository in C:/Temp/gittest/.git/

C:\Temp\gittest>git commit --allow-empty -m init
[master (root-commit) 2e78994] init

C:\Temp\gittest>git branch one/two

C:\Temp\gittest>git branch -d one/two
Deleted branch one/two (was 2e78994).

C:\Temp\gittest>git branch one

C:\Temp\gittest>git version
git version 2.1.0.rc2.1268.g12ef091


The relevant bits are in log_ref_setup. We try to open() once, and
accept EISDIR as a hint that we may need to remove an empty directory
and try again. If Windows gives us EACCES, then we may need to have that
trigger the same code.


Thanks, that is a starting point.

-- Hannes

--
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