Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-sh-setup: Fix scripts whose PWD is a symlink into a git work-dir
From: Junio C Hamano @ 2008-12-03  7:20 UTC (permalink / raw)
  To: Marcel M. Cary; +Cc: git, jnareb, ae, j.sixt
In-Reply-To: <1228282020-2294-1-git-send-email-marcel@oak.homeunix.org>

"Marcel M. Cary" <marcel@oak.homeunix.org> writes:

> If cd_to_toplevel had concatenated $(/bin/pwd) with $cdup to
> avoid the separate "cd", it would require checking for $cdup
> being an absolute path.  I wasn't sure how to check that in
> a way that is both portable and clearly faster than "cd",

    case "$v" in
    /*) : handle absolute path ;;
    *) : everything else ;;
    esac

In all shells that support "case..esac", it is built-in.

Having said that, I think it would probably be better to bite the bullet
and start using "cd -P" soon after 1.6.1 goes final, and at the same time
existing places that use "cd `pwd`" as a workaround if there are some.

^ permalink raw reply

* What's cooking in git.git (Dec 2008, #01; Tue, 02)
From: Junio C Hamano @ 2008-12-03  6:59 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed
with '-' are only in 'pu' while commits prefixed with '+' are
in 'next'.

The topics list the commits in reverse chronological order.  The topics
meant to be merged to the maintenance series have "maint-" in their names.

As we have already passed -rc1, things queued in 'next' let alone 'pu' are
unlikely to be merged to 'master' by the end of year unless otherwise
noted.

----------------------------------------------------------------
[New Topics]

* gb/gitweb-patch (Sat Nov 29 14:41:11 2008 +0100) 2 commits
 - [DONTMERGE: wait for signoff] gitweb: links to patch action in
   commitdiff and shortlog view
 - gitweb: add patch view

* lt/reset-merge (Mon Dec 1 09:30:31 2008 -0800) 1 commit
 + Add 'merge' mode to 'git reset'

Unfortunately, I cannot write down a good use case in what circumstances
this is needed, even though I do recall that I occasionally (perhaps once
every two months) needed to do "read-tree -m -u" myself in the past, and
this patch brings that feature close to Porcelain.  On the other hand, the
existing "reset --mixed" is very easily explained ("after starting to add
changes to the staging area, you realized that you screwed up, and you
want to redo it from scratch").  Therefore, I cannot justify what the log
message of this change claims myself.

* jc/rm-i-t-a (Fri Nov 28 19:56:34 2008 -0800) 5 commits
 + git add --intent-to-add: do not let an empty blob be committed by
   accident
 + git add --intent-to-add: fix removal of cached emptiness
 + builtin-rm.c: explain and clarify the "local change" logic
 + Merge branch 'nd/narrow' (early part) into jc/add-i-t-a
 + Extend index to save more flags

As intent-to-add index entry is a new feature for 1.6.1, it probably is a
good idea to merge this to 'master'.  Nitpicks and bugfixes are very much
appreciated.

* wp/add-patch-find (Thu Nov 27 04:08:03 2008 +0000) 3 commits
 - In add --patch, Handle K,k,J,j slightly more gracefully.
 - Add / command in add --patch
 - git-add -i/-p: Change prompt separater from slash to comma

There are some other useful bits and pieces around this area exchanged on
the list with the author of these patches.  I think it would become a
useful series after reassembling their pieces.  Waiting for v2.

* jn/gitweb-utf8 (Mon Dec 1 19:01:42 2008 +0100) 1 commit
 - gitweb: Fix handling of non-ASCII characters in inserted HTML
   files.

Possibly a bugfix worthy to have in 1.6.1.

* jc/clone-symref-2 (Sat Nov 29 23:38:21 2008 -0800) 7 commits
 - clone: test the new HEAD detection logic
 - Merge commit 'HEAD@{2}' into HEAD
 - upload-pack: send the HEAD information
 - clone: find the current branch more explicitly
 - connect.c::read_extra_info(): find where HEAD points at
 - connect.c::read_extra_info(): prepare to receive more than server
   capabilities
 - get_remote_heads(): refactor code to read "server capabilities"

This is no way meant for 1.6.1, let alone next, yet.

----------------------------------------------------------------
[Graduated to "master"]

* cr/remote-update-v (Tue Nov 18 19:04:02 2008 +0800) 1 commit
 + git-remote: add verbose mode to git remote update

* rs/strbuf-expand (Sun Nov 23 00:16:59 2008 +0100) 6 commits
 + remove the unused files interpolate.c and interpolate.h
 + daemon: deglobalize variable 'directory'
 + daemon: inline fill_in_extra_table_entries()
 + daemon: use strbuf_expand() instead of interpolate()
 + merge-recursive: use strbuf_expand() instead of interpolate()
 + add strbuf_expand_dict_cb(), a helper for simple cases

* mv/fast-export (Sun Nov 23 12:55:54 2008 +0100) 2 commits
 + fast-export: use an unsorted string list for extra_refs
 + Add new testcase to show fast-export does not always exports all
   tags

* st/levenshtein (Thu Nov 20 14:27:27 2008 +0100) 2 commits
 + Document levenshtein.c
 + Fix deletion of last character in levenshtein distance

* js/mingw-rename-fix (Wed Nov 19 17:25:27 2008 +0100) 1 commit
 + compat/mingw.c: Teach mingw_rename() to replace read-only files

* mv/clone-strbuf (Fri Nov 21 01:45:01 2008 +0100) 3 commits
 + builtin_clone: use strbuf in cmd_clone()
 + builtin-clone: use strbuf in clone_local() and
   copy_or_link_directory()
 + builtin-clone: use strbuf in guess_dir_name()

* pw/maint-p4 (Wed Nov 26 13:52:15 2008 -0500) 1 commit
 - git-p4: fix keyword-expansion regex

* cc/bisect-skip (Sun Nov 23 22:02:49 2008 +0100) 1 commit
 - bisect: teach "skip" to accept special arguments like "A..B"

Should be in 1.6.1-rc1.

* lt/preload-lstat (Mon Nov 17 09:01:20 2008 -0800) 2 commits
 + Fix index preloading for racy dirty case
 + Add cache preload facility

* ta/quiet-pull (Mon Nov 17 23:09:30 2008 +0100) 2 commits
 + Retain multiple -q/-v occurrences in git pull
 + Teach/Fix pull/fetch -q/-v options

* ph/send-email (Tue Nov 11 00:54:02 2008 +0100) 4 commits
 + git send-email: ask less questions when --compose is used.
 + git send-email: add --annotate option
 + git send-email: interpret unknown files as revision lists
 + git send-email: make the message file name more specific.

After merging these to 'master' I found a breakage which I hopefully
fixed.

----------------------------------------------------------------
[Will merge to "master" soon]

What are you looking for?  We are in -rc ;-)

----------------------------------------------------------------
[On Hold]

* cb/mergetool (Thu Nov 13 12:41:15 2008 +0000) 3 commits
 - [DONTMERGE] Add -k/--keep-going option to mergetool
 - Add -y/--no-prompt option to mergetool
 - Fix some tab/space inconsistencies in git-mergetool.sh

Jeff had good comments on the last one; the discussion needs concluded,
and also waiting for comments from the original author (Ted).

* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits
 + blame: show "previous" information in --porcelain/--incremental
   format
 + git-blame: refactor code to emit "porcelain format" output

* ds/uintmax-config (Mon Nov 3 09:14:28 2008 -0900) 1 commit
 - autoconf: Enable threaded delta search when pthreads are supported

Rebased to 'master', that introduced NO_PTHREADS.

* cc/bisect-replace (Mon Nov 24 22:20:30 2008 +0100) 9 commits
 - bisect: add "--no-replace" option to bisect without using replace
   refs
 - rev-list: make it possible to disable replacing using "--no-
   bisect-replace"
 - bisect: use "--bisect-replace" options when checking merge bases
 - merge-base: add "--bisect-replace" option to use fixed up revs
 - commit: add "bisect_replace_all" prototype to "commit.h"
 - rev-list: add "--bisect-replace" to list revisions with fixed up
   history
 - Documentation: add "git bisect replace" documentation
 - bisect: add test cases for "git bisect replace"
 - bisect: add "git bisect replace" subcommand

I really hate the idea of introducing a potentially much more useful
replacement of the existing graft mechanism and tie it very tightly to
bisect, making it unusable from outside.

 (1) I do not think "bisect replace" workflow is a practical and usable
     one;

 (2) The underlying mechanism to express "this object replaces that other
     object" is much easier to work with than what the graft does which is
     "the parents of this commit are these", and idea to use the normal
     ref to point at them means this can potentially be used for
     transferring the graft information across repositories, which the
     current graft mechanism cannot do.

 (3) Because I like the aspect (2) of this series so much, it deeply
     disappoints and troubles me that this is implemented minimally near
     the surface, and that it is controlled by the "bisect" Porcelain
     alone, by explicitly passing command line arguments.

I think a mechanism like this should be added to replace grafts, but it
should always be enabled for normal revision traversal operation, while
always disabled for object enumeration and transfer operation (iow, fsck,
fetch and push should use the real ancestry information recorded in the
underlying objects, while rev-list, log, etc. should always use the
replaced objects).  I have a suspicion that even cat-file could honor it.

* nd/narrow (Sun Nov 30 17:54:38 2008 +0700) 18 commits
 - wt-status: show sparse checkout info
 - Introduce default sparse patterns (core.defaultsparse)
 - checkout: add new options to support sparse checkout
 - clone: support sparse checkout with --sparse-checkout option
 - unpack_trees(): add support for sparse checkout
 - unpack_trees(): keep track of unmerged entries
 - Introduce "sparse patterns"
 - Merge branch 'master' into nd/narrow
 + t2104: touch portability fix
 + grep: skip files outside sparse checkout area
 + checkout_entry(): CE_NO_CHECKOUT on checked out entries.
 + Prevent diff machinery from examining worktree outside sparse
   checkout
 + ls-files: Add tests for --sparse and friends
 + update-index: add --checkout/--no-checkout to update
   CE_NO_CHECKOUT bit
 + update-index: refactor mark_valid() in preparation for new options
 + ls-files: add options to support sparse checkout
 + Introduce CE_NO_CHECKOUT bit
 + Extend index to save more flags

Kicked back to 'on hold' until 1.6.1 final by popular(?) demand.

* jc/send-pack-tell-me-more (Thu Mar 20 00:44:11 2008 -0700) 1 commit
 - "git push": tellme-more protocol extension

This seems to have a deadlock during communication between the peers.
Someone needs to pick up this topic and resolve the deadlock before it can
continue.

* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit
 - diff: enable "too large a rename" warning when -M/-C is explicitly
   asked for

This would be the right thing to do for command line use,
but gitk will be hit due to tcl/tk's limitation, so I am holding
this back for now.

* jc/stripspace (Sun Mar 9 00:30:35 2008 -0800) 6 commits
 - git-am --forge: add Signed-off-by: line for the author
 - git-am: clean-up Signed-off-by: lines
 - stripspace: add --log-clean option to clean up signed-off-by:
   lines
 - stripspace: use parse_options()
 - Add "git am -s" test
 - git-am: refactor code to add signed-off-by line for the committer

^ permalink raw reply

* What's in git.git (Dec 2008, #01; Tue, 02)
From: Junio C Hamano @ 2008-12-03  6:23 UTC (permalink / raw)
  To: git

We'll probably have 1.6.0.5 by the end of this week.

The tip of the 'master' branch is at 1.6.1-rc1 with a few more fixes.
Hopefully we will have the final 1.6.1 by the end of the year.

* The 'maint' branch has these fixes since the last announcement.

Joey Hess (1):
  sha1_file: avoid bogus "file exists" error message

Johannes Schindelin (1):
  fast-export: use an unsorted string list for extra_refs

Junio C Hamano (1):
  Update draft release notes to 1.6.0.5

Martin Koegler (1):
  git push: Interpret $GIT_DIR/branches in a Cogito compatible way

Matt McCutchen (1):
  git checkout: don't warn about unborn branch if -f is already passed

Miklos Vajna (2):
  Add new testcase to show fast-export does not always exports all tags
  User's Manual: remove duplicated url at the end of Appendix B

Nguyễn Thái Ngọc Duy (1):
  generate-cmdlist.sh: avoid selecting synopsis at wrong place

Pete Wyckoff (1):
  git-p4: fix keyword-expansion regex

Ralf Wildenhues (1):
  Fix typos in the documentation.

SZEDER Gábor (2):
  bash: remove dashed command leftovers
  bash: offer refs instead of filenames for 'git revert'

Sam Vilain (1):
  sha1_file.c: resolve confusion EACCES vs EPERM

Samuel Tardieu (2):
  tag: Check that options are only allowed in the appropriate mode
  tag: Add more tests about mixing incompatible modes and options


* The 'master' branch has these since the last announcement
  in addition to the above.

Alexander Gavrilov (3):
  gitk: Avoid handling the Return key twice in Add Branch
  gitk: Make line origin search update the busy status
  gitk: Add a menu option to start git gui

Cheng Renquan (1):
  git-remote: add verbose mode to git remote update

Christian Couder (4):
  git-gui: french translation update
  bisect: teach "skip" to accept special arguments like "A..B"
  bisect: fix "git bisect skip <commit>" and add tests cases
  Documentation: describe how to "bisect skip" a range of commits

Christian Stimming (1):
  gitk: Update German translation

Giuseppe Bilotta (2):
  gitweb: make gitweb_check_feature a boolean wrapper
  Update comment on gitweb_check/get_feature

Johannes Schindelin (1):
  Document levenshtein.c

Johannes Sixt (2):
  compat/mingw.c: Teach mingw_rename() to replace read-only files
  t4030-diff-textconv: Make octal escape sequence more portable

Junio C Hamano (6):
  gitweb: fix 'ctags' feature check and others
  gitweb: rename gitweb_check_feature to gitweb_get_feature
  send-email: do not reverse the command line arguments
  Include git-gui--askpass in git-gui RPM package
  GIT 1.6.1-rc1
  Makefile: introduce NO_PTHREADS

Linus Torvalds (3):
  Add cache preload facility
  Fix index preloading for racy dirty case
  Add backslash to list of 'crud' characters in real name

Mark Burton (1):
  git-gui: Teach start_push_anywhere_action{} to notice when remote is a
    mirror.

Michele Ballabio (1):
  git gui: update Italian translation

Miklos Vajna (5):
  Update Hungarian translation. 100% completed.
  builtin-clone: use strbuf in guess_dir_name()
  builtin-clone: use strbuf in clone_local() and copy_or_link_directory()
  builtin_clone: use strbuf in cmd_clone()
  git-stash: use git rev-parse -q

Nanako Shiraishi (1):
  git-gui: update Japanese translation

Paul Mackerras (6):
  gitk: Index line[hnd]tag arrays by id rather than row number
  gitk: Fix switch statement in parseviewargs
  gitk: Show local changes properly when we have a path limit
  gitk: Fix context menu items for generating diffs when in tree mode
  gitk: Highlight only when search type is "containing:".
  gitk: Fix bug in accessing undefined "notflag" variable

Peter Krefting (1):
  Updated Swedish translation (514t0f0u).

Pierre Habouzit (4):
  git send-email: make the message file name more specific.
  git send-email: interpret unknown files as revision lists
  git send-email: add --annotate option
  git send-email: ask less questions when --compose is used.

Ralf Wildenhues (1):
  Fix typos in the documentation.

René Scharfe (6):
  add strbuf_expand_dict_cb(), a helper for simple cases
  merge-recursive: use strbuf_expand() instead of interpolate()
  daemon: use strbuf_expand() instead of interpolate()
  daemon: inline fill_in_extra_table_entries()
  daemon: deglobalize variable 'directory'
  remove the unused files interpolate.c and interpolate.h

SZEDER Gábor (1):
  bash: complete full refs

Samuel Tardieu (1):
  Fix deletion of last character in levenshtein distance

Scott Chacon (1):
  Add a built-in alias for 'stage' to the 'add' command

Tuncer Ayaz (2):
  Teach/Fix pull/fetch -q/-v options
  Retain multiple -q/-v occurrences in git pull

^ permalink raw reply

* Re: git-p4 submit, Can't clobber writable file
From: Gary Yang @ 2008-12-03  6:04 UTC (permalink / raw)
  To: Jing Xue; +Cc: git
In-Reply-To: <20081203030729.GB5624@jabba.hq.digizenstudio.com>

So, I have to keep two copies of source tree at my home directory. One is for Perforce build_scripts workspace, another is for Git build_scripts.git. I normally work at build_scripts.git. But, when I need to submit changes to Perforce, I have to copy changed code from build_scripts.git to build_scripts. Then, p4 submit code into Perforce. Is this the only way of using git-p4?
Note: I cannot use git-p4 submit at build_scripts. It claims "Cannot clobber writable file". Is it a bug of git-p4 or the instruction is not correct?


--- On Tue, 12/2/08, Jing Xue <jingxue@digizenstudio.com> wrote:

> From: Jing Xue <jingxue@digizenstudio.com>
> Subject: Re: git-p4 submit, Can't clobber writable file
> To: "Gary Yang" <garyyang6@yahoo.com>
> Cc: git@vger.kernel.org
> Date: Tuesday, December 2, 2008, 7:07 PM
> On Tue, Dec 02, 2008 at 02:30:51PM -0800, Gary Yang wrote:
> > 
> > I followed the instructions at
> http://modular.math.washington.edu/home/mhansen/git-1.5.5.1/contrib/fast-import/git-p4.txt
> > 
> > But, I am not able to git-p4 submit. Any idea?
> > 
> > git-p4 clone //build/scripts build_scripts
> > cd build_scripts
> > vi foo.h
> > git commit foo.h
> > git-p4 rebase
> > git-p4 submit
> > 
> >   from sets import Set;
> > Perforce checkout for depot path //build/scripts/
> located at /home/gyang/workspace/build_scripts/
> > Syncronizing p4 checkout...
> > //build/scripts/foo.h#1 - added as
> /home/gyang/workspace/build_scripts/foo.h
> > Can't clobber writable file
> /home/gyang/workspace/build_scripts/foo.h
> > //build/scripts/foo.c#1 - added as
> /home/gyang/workspace/build_scripts/foo.c
> > Can't clobber writable file
> /user/home/gyang/workspace/build_scripts/foo.c
> > ......
> > command failed: p4 sync ...
> 
> You might want to clone to a git working dir different than
> the p4
> working dir.
> 
> For instance, if your p4 workspace has the working dir set
> to
> build_scripts/, try 'git p4 clone //build/scripts
> build_scripts.git'.
> 
> You would then normally work under build_scripts.git/.
> build_scripts/
> would only be used by git-p4 at submission time.
> 
> Cheers.
> -- 
> Jing Xue


      

^ permalink raw reply

* [PATCH] git-sh-setup: Fix scripts whose PWD is a symlink into a git work-dir
From: Marcel M. Cary @ 2008-12-03  5:27 UTC (permalink / raw)
  To: gitster, git; +Cc: jnareb, ae, j.sixt, Marcel M. Cary
In-Reply-To: <7vtz9vk6uj.fsf@gitster.siamese.dyndns.org>

* Before interpretting an upward path (../) in cd_to_toplevel,
  cd to a path without symlinks given by /bin/pwd
* Add tests for cd_to_toplevel and "git pull" in a symlinked
  directory that failed before this fix, plus constrasting
  scenarios that already worked

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---

I hope this patch will address concerns both about changes
to existing APIs and speed of the new behavior.

A few notes on implementation choices:

I used /bin/pwd because of this precedent for choosing it over
"cd -P" for compatibility.
http://article.gmane.org/gmane.comp.version-control.git/46918

If cd_to_toplevel had concatenated $(/bin/pwd) with $cdup to
avoid the separate "cd", it would require checking for $cdup
being an absolute path.  I wasn't sure how to check that in
a way that is both portable and clearly faster than "cd",
so cd_to_toplevel runs "cd" twice.  I'm assuming that
running an external command like expr or grep is slower than
just doing the "cd".

cd_to_toplevel doesn't check $PWD to see whether to do the
first cd, because some shells allegedly don't update it
reliably.

Since cd_to_toplevel doesn't know whether it's at a
symlinked PWD or not, I wrote it to treat the 
"cd $(/bin/pwd)" as mandatory, even when it might not
actually be.  So on systems without /bin/pwd, it will fail
even when there are no symlinks.  I thought that was better
than inconsistent behavior depending on whether /bin/pwd is
available.

The extra "cd" will be skipped when the script is already at
the top of the working tree.


 git-sh-setup.sh           |   11 +++++++
 t/t2300-cd-to-toplevel.sh |   37 +++++++++++++++++++++++++
 t/t5521-pull-symlink.sh   |   67 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 0 deletions(-)
 create mode 100755 t/t2300-cd-to-toplevel.sh
 create mode 100755 t/t5521-pull-symlink.sh

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index dbdf209..377700b 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -85,6 +85,17 @@ cd_to_toplevel () {
 	cdup=$(git rev-parse --show-cdup)
 	if test ! -z "$cdup"
 	then
+		# Interpret $cdup relative to the physical, not logical, cwd.
+		# Probably /bin/pwd is more portable than passing -P to cd or pwd.
+		phys="$(/bin/pwd)" || {
+			echo >&2 "Cannot determine the physical path to the current dir"
+			exit 1
+		}
+		cd "$phys" || {
+			echo >&2 "Cannot chdir to the physical path to current dir: $phys"
+			exit 1
+		}
+
 		cd "$cdup" || {
 			echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
 			exit 1
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
new file mode 100755
index 0000000..293dc35
--- /dev/null
+++ b/t/t2300-cd-to-toplevel.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='cd_to_toplevel'
+
+. ./test-lib.sh
+
+test_cd_to_toplevel () {
+	test_expect_success "$2" '
+		(
+			cd '"'$1'"' &&
+			. git-sh-setup &&
+			cd_to_toplevel &&
+			[ "$(pwd -P)" = "$TOPLEVEL" ]
+		)
+	'
+}
+
+TOPLEVEL="$(pwd -P)/repo"
+mkdir -p repo/sub/dir
+mv .git repo/
+SUBDIRECTORY_OK=1
+
+test_cd_to_toplevel repo 'at physical root'
+
+test_cd_to_toplevel repo/sub/dir 'at physical subdir'
+
+ln -s repo symrepo
+test_cd_to_toplevel symrepo 'at symbolic root'
+
+ln -s repo/sub/dir subdir-link
+test_cd_to_toplevel subdir-link 'at symbolic subdir'
+
+cd repo
+ln -s sub/dir internal-link
+test_cd_to_toplevel internal-link 'at internal symbolic subdir'
+
+test_done
diff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh
new file mode 100755
index 0000000..f18fec7
--- /dev/null
+++ b/t/t5521-pull-symlink.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='pulling from symlinked subdir'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+# The scenario we are building:
+#
+#   trash\ directory/
+#     clone-repo/
+#       subdir/
+#         bar
+#     subdir-link -> clone-repo/subdir/
+#
+# The working directory is subdir-link.
+#
+test_expect_success setup '
+
+    mkdir subdir &&
+    touch subdir/bar &&
+    git add subdir/bar &&
+    git commit -m empty &&
+    git clone . clone-repo &&
+    # demonstrate that things work without the symlink
+    test_debug "cd clone-repo/subdir/ && git pull; cd ../.." &&
+    ln -s clone-repo/subdir/ subdir-link &&
+    cd subdir-link/ &&
+    test_debug "set +x"
+'
+
+# From subdir-link, pulling should work as it does from
+# clone-repo/subdir/.
+#
+# Instead, the error pull gave was:
+#
+#   fatal: 'origin': unable to chdir or not a git archive
+#   fatal: The remote end hung up unexpectedly
+#
+# because git would find the .git/config for the "trash directory"
+# repo, not for the clone-repo repo.  The "trash directory" repo
+# had no entry for origin.  Git found the wrong .git because
+# git rev-parse --show-cdup printed a path relative to
+# clone-repo/subdir/, not subdir-link/.  Git rev-parse --show-cdup
+# used the correct .git, but when the git pull shell script did
+# "cd `git rev-parse --show-cdup`", it ended up in the wrong
+# directory.  Shell "cd" works a little different from chdir() in C.
+# Bash's "cd -P" works like chdir() in C.
+#
+test_expect_success 'pulling from symlinked subdir' '
+
+    git pull
+'
+
+# Prove that the remote end really is a repo, and other commands
+# work fine in this context.
+#
+test_debug "
+    test_expect_success 'pushing from symlinked subdir' '
+
+        git push
+    '
+"
+cd "$D"
+
+test_done
-- 
1.6.0.3

^ permalink raw reply related

* Re: [PATCH 5/5] support writing uncompressed loose object
From: Liu Yubao @ 2008-12-03  4:22 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202160706.GN23984@spearce.org>

Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> Signed-off-by: Liu Yubao <yubao.liu@gmail.com>
> 
> IMHO, this needs more description in the commit message.
> 
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 05a9fa3..053b564 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -2328,7 +2328,7 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
>>  }
>>  
>>  static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
>> -			      void *buf, unsigned long len, time_t mtime)
>> +			      void *buf, unsigned long len, time_t mtime, int dont_deflate)
> 
> Passing this as an argument is pointless.  It should be a repository
> wide configuration option in core, so you can declare it a static and
> allow git_config to populate it.  Defaulting to 1 (no compression)
> like you do elsewhere in the patch isn't good.
> 
Aha, sorry again, I sent the patch series as separate topics by mistake.

I considered adding a configuration variable, the patch series are sent
just to see whether the idea is worth.

> I'm still against this file format change.  The series itself isn't
> that bad, and the buffer overflow catch in parse_sha1_header()
> may be something worthwhile fixing.  But I'm still not sold that
> introducing a new loose object format is worth it.
> 
> I'd rather use a binary header encoding like the new-style/in-pack
> format rather than the older style text headers.  Its faster to
> parse for one thing.
> 
The key point I suggest is to use *uncompressed* loose object, I didn't
change the format of uncompressed loose object because I don't want
to distract your attention and keep the patches small.

> Your changes in the reading code cause a copy of the buffer we
> mmap()'d.  That sort of ruins your argument that this change is
> worthwhile because concurrent processes on the same host can mmap the
> same buffer and save memory.  We're still copying the buffer anyway.
> I probably should have commented on that in patch 4/5, but I just
> realized it, so I'm saying it here.
> 
Yes, I mentioned it in the cover letter(sigh, sorry!)

I didn't use the mapped buffer directly because other functions required
a null terminated buffer to parse data part of loose object. It can be
fixed but I don't want to make the patches too big.

The two big pros of uncompressed loose object are:

*) avoid compressing and uncompressing loose objects    (I have implemented it)
*) use memory mapped loose object directly              (I havn't implemented it)


Thank you for reviewing my patches, seems the idea to use uncompressed loose
object isn't attractive enough, I will keep the patches locally.


Best regards,

Liu Yubao

^ permalink raw reply

* Re: [PATCH] Implement rebase -q to fix pull --rebase -q
From: Tuncer Ayaz @ 2008-12-03  4:09 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <1228277212-5917-1-git-send-email-tuncer.ayaz@gmail.com>

On Wed, Dec 3, 2008 at 5:06 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> This is needed on top of the fetch/pull -q/-v changes
> to make
> $ git pull --rebase -q
> as quiet as expected.
>
> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
> ---
>  git-pull.sh   |    2 +-
>  git-rebase.sh |   31 +++++++++++++++++++++++--------
>  2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 1cac898..57fcee9 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -184,6 +184,6 @@ fi
>  merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
>  test true = "$rebase" &&
>        exec git-rebase $strategy_args --onto $merge_head \
> -       ${oldremoteref:-$merge_head}
> +       $verbosity ${oldremoteref:-$merge_head}
>  exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
>        "$merge_name" HEAD $merge_head $verbosity
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 023a6dc..bbfdc2e 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -3,7 +3,7 @@
>  # Copyright (c) 2005 Junio C Hamano.
>  #
>
> -USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
> +USAGE='[--interactive | -i] [-q] [-v] [--onto <newbase>] <upstream> [<branch>]'
>  LONG_USAGE='git-rebase replaces <branch> with a new branch of the
>  same name.  When the --onto option is provided the new branch starts
>  out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
> @@ -45,7 +45,7 @@ strategy=recursive
>  do_merge=
>  dotest="$GIT_DIR"/rebase-merge
>  prec=4
> -verbose=
> +verbosity=1
>  git_am_opt=
>
>  continue_merge () {
> @@ -135,7 +135,10 @@ move_to_original_branch () {
>  finish_rb_merge () {
>        move_to_original_branch
>        rm -r "$dotest"
> -       echo "All done."
> +       if test $verbosity -gt 0
> +       then
> +               echo "All done."
> +       fi
>  }
>
>  is_interactive () {
> @@ -288,8 +291,11 @@ do
>                esac
>                do_merge=t
>                ;;
> +       -q|--quiet)
> +               verbosity=0
> +               ;;
>        -v|--verbose)
> -               verbose=t
> +               verbosity=2
>                ;;
>        --whitespace=*)
>                git_am_opt="$git_am_opt $1"
> @@ -401,11 +407,14 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
>  then
>        # Lazily switch to the target branch if needed...
>        test -z "$switch_to" || git checkout "$switch_to"
> -       echo >&2 "Current branch $branch_name is up to date."
> +       if test $verbosity -gt 0
> +       then
> +               echo >&2 "Current branch $branch_name is up to date."
> +       fi

If anyone dislikes the additional three lines I could combine
the test with the action on one line. I'm just not sure that would
make it better, especially depending on log message length.

>        exit 0
>  fi
>
> -if test -n "$verbose"
> +if test $verbosity -gt 1
>  then
>        echo "Changes from $mb to $onto:"
>        # We want color (if set), but no pager
> @@ -413,7 +422,10 @@ then
>  fi
>
>  # Detach HEAD and reset the tree
> -echo "First, rewinding head to replay your work on top of it..."
> +if test $verbosity -gt 0
> +then
> +       echo "First, rewinding head to replay your work on top of it..."
> +fi
>  git checkout -q "$onto^0" || die "could not detach HEAD"
>  git update-ref ORIG_HEAD $branch
>
> @@ -421,7 +433,10 @@ git update-ref ORIG_HEAD $branch
>  # we just fast forwarded.
>  if test "$mb" = "$branch"
>  then
> -       echo >&2 "Fast-forwarded $branch_name to $onto_name."
> +       if test $verbosity -gt 0
> +       then
> +               echo >&2 "Fast-forwarded $branch_name to $onto_name."
> +       fi
>        move_to_original_branch
>        exit 0
>  fi
> --
> 1.6.0.2.GIT
>
>

^ permalink raw reply

* Re: [PATCH 4/5] support reading uncompressed loose object
From: Liu Yubao @ 2008-12-03  4:09 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202155806.GM23984@spearce.org>

Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> Signed-off-by: Liu Yubao <yubao.liu@gmail.com>
> 
> I'd like to see a bit more of an explanation of the new loose
> object format you are reading in the commit message.  We have a
> long history of explaining *why* the code behaves the way it does
> in our commits, so we can look at it in blame/log and understand
> what the heck went on.
>  
I mentioned in the cover letter sent as patch 0/5, indeed I should
have mentioned it in the commit message again.

An uncompressed loose object is just an uncompressed loose object:

loose object = deflate(typename + <space> + size + '\0' + data)
uncompressed object = inflate(loose object)

^ permalink raw reply

* [PATCH] Implement rebase -q to fix pull --rebase -q
From: Tuncer Ayaz @ 2008-12-03  4:06 UTC (permalink / raw)
  To: gitster; +Cc: git

This is needed on top of the fetch/pull -q/-v changes
to make
$ git pull --rebase -q
as quiet as expected.

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 git-pull.sh   |    2 +-
 git-rebase.sh |   31 +++++++++++++++++++++++--------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 1cac898..57fcee9 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -184,6 +184,6 @@ fi
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
-	${oldremoteref:-$merge_head}
+	$verbosity ${oldremoteref:-$merge_head}
 exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
 	"$merge_name" HEAD $merge_head $verbosity
diff --git a/git-rebase.sh b/git-rebase.sh
index 023a6dc..bbfdc2e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Junio C Hamano.
 #
 
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
+USAGE='[--interactive | -i] [-q] [-v] [--onto <newbase>] <upstream> [<branch>]'
 LONG_USAGE='git-rebase replaces <branch> with a new branch of the
 same name.  When the --onto option is provided the new branch starts
 out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
@@ -45,7 +45,7 @@ strategy=recursive
 do_merge=
 dotest="$GIT_DIR"/rebase-merge
 prec=4
-verbose=
+verbosity=1
 git_am_opt=
 
 continue_merge () {
@@ -135,7 +135,10 @@ move_to_original_branch () {
 finish_rb_merge () {
 	move_to_original_branch
 	rm -r "$dotest"
-	echo "All done."
+	if test $verbosity -gt 0
+	then
+		echo "All done."
+	fi
 }
 
 is_interactive () {
@@ -288,8 +291,11 @@ do
 		esac
 		do_merge=t
 		;;
+	-q|--quiet)
+		verbosity=0
+		;;
 	-v|--verbose)
-		verbose=t
+		verbosity=2
 		;;
 	--whitespace=*)
 		git_am_opt="$git_am_opt $1"
@@ -401,11 +407,14 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 then
 	# Lazily switch to the target branch if needed...
 	test -z "$switch_to" || git checkout "$switch_to"
-	echo >&2 "Current branch $branch_name is up to date."
+	if test $verbosity -gt 0
+	then
+		echo >&2 "Current branch $branch_name is up to date."
+	fi
 	exit 0
 fi
 
-if test -n "$verbose"
+if test $verbosity -gt 1
 then
 	echo "Changes from $mb to $onto:"
 	# We want color (if set), but no pager
@@ -413,7 +422,10 @@ then
 fi
 
 # Detach HEAD and reset the tree
-echo "First, rewinding head to replay your work on top of it..."
+if test $verbosity -gt 0
+then
+	echo "First, rewinding head to replay your work on top of it..."
+fi
 git checkout -q "$onto^0" || die "could not detach HEAD"
 git update-ref ORIG_HEAD $branch
 
@@ -421,7 +433,10 @@ git update-ref ORIG_HEAD $branch
 # we just fast forwarded.
 if test "$mb" = "$branch"
 then
-	echo >&2 "Fast-forwarded $branch_name to $onto_name."
+	if test $verbosity -gt 0
+	then
+		echo >&2 "Fast-forwarded $branch_name to $onto_name."
+	fi
 	move_to_original_branch
 	exit 0
 fi
-- 
1.6.0.2.GIT

^ permalink raw reply related

* Re: [PATCH 3/5] optimize parse_sha1_header() a little by detecting object type
From: Liu Yubao @ 2008-12-03  4:06 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202155300.GL23984@spearce.org>

Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> diff --git a/sha1_file.c b/sha1_file.c
>> index dccc455..79062f0 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -1099,7 +1099,8 @@ static void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
>>  
>>  		if (!fstat(fd, &st)) {
>>  			*size = xsize_t(st.st_size);
>> -			map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
>> +			if (*size > 0)
>> +				map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
>>  		}
>>  		close(fd);
>>  	}
> 
> This has nothing to do with this change description.  Why are we
> returning NULL from map_sha1_file when the file length is 0 bytes?
> No loose object should ever be an empty file, there must always be
> some sort of type header present.  So it probably is an error to
> have a 0 length file here.  But that bug is a different change.
> 

Also a defensive programming for uncompressed loose object because the mapped memory
will be passed to parse_sha1_header() directly without being checked by inflateInit().

In fact unpack_sha1_header() in current code calls legacy_loose_object() without checking
mapsize first. If it encounters (very very unlikely) a corrupted empty loose object, it
will crash.

>> @@ -1257,6 +1258,8 @@ static int parse_sha1_header(const char *hdr, unsigned long length, unsigned lon
>>  	 * terminating '\0' that we add), and is followed by
>>  	 * a space, at least one byte for size, and a '\0'.
>>  	 */
>> +	if ('b' != *hdr && 'c' != *hdr && 't' != *hdr)	/* blob/commit/tag/tree */
>> +		return -1;
>>  	i = 0;
>>  	while (hdr < hdr_end - 2) {
>>  		char c = *hdr++;
> 
> Oh.  I wouldn't do that.  Its a cute trick and it works to quickly
> determine if the header is an uncompressed header vs. a zlib header
> vs. a new-style loose object header (which git cannot write anymore,
> but it still can read).  But its just asking for trouble when/if a
> new object type was ever added to the type table.
> 
I can't agree any more, it's just a trick. I considered adding
a function seems_likely_uncompressed_loose_object(), but I didn't
because this patch series are just my first try, I don't know whether
the idea to support uncompressed loose object is attractive enough.

> Given that we know that no type name can be more than 10 bytes and
> if you use my patch from earlier today you can be certain hdr has a
> '\0' terminator, so you could write a function to test for the type
> against the hdr, stopping on either ' ' or '\0'.  Or find the first
> ' ' in the first 10 bytes (which is what this loop does anyway) and
> then test that against the type name table.
> 

^ permalink raw reply

* Re: [PATCH] gitweb: Fix handling of non-ASCII characters in inserted HTML files
From: Junio C Hamano @ 2008-12-03  3:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Tatsuki Sugiura, Gerrit Pape, Recai Oktas
In-Reply-To: <20081201175741.11133.82393.stgit@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Use new insert_file() subroutine to insert HTML chunks from external
> files: $site_header, $home_text (by default indextext.html),
> $site_footer, and $projectroot/$project/REAME.html.
>
> All non-ASCII chars of those files will be broken by Perl IO layer
> without decoding to utf8, so insert_file() does to_utf8() on each
> printed line; alternate solution would be to open those files with
> "binmode $fh, ':utf8'", or even all files with "use open qw(:std :utf8)".
>
> Note that inserting README.html lost one of checks for simplicity.
>
> Noticed-by: Tatsuki Sugiura <sugi@nemui.org>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This is more complete solution that the one provided by Tatsuki Sugiura
> in original patch
>
>   [PATCH] gitweb: fix encode handling for site_{header,footer}
>   Msg-Id: <87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp>
>   http://thread.gmane.org/gmane.comp.version-control.git/101199

It may be more complete but it is obviously untested.  Please help me
trust you better with your future patches.  Because I personally do not
run gitweb myself, I really need a trustworthy lieutenant(s) in the area.

[Wed Dec  3 01:52:07 2008] gitweb.perl: Global symbol "$fd" requires explicit package name at /git.git/t/../gitweb/gitweb.perl line 4500.
[Wed Dec  3 01:52:07 2008] gitweb.perl: Execution of /git.git/t/../gitweb/gitweb.perl aborted due to compilation errors.

> but it is in principle the same solution.
>
> I think this one as it is a bugfix should go in git 1.6.1

Trading a gitweb with a small bug with a gitweb that does not even pass
its test script does not feel like a good change to me.

I think the breakage is the "close $fd" at the end of this hunk:

> @@ -4472,13 +4475,11 @@ sub git_summary {
>  	print "</table>\n";
>  
>  	if (-s "$projectroot/$project/README.html") {
> -		if (open my $fd, "$projectroot/$project/README.html") {
> -			print "<div class=\"title\">readme</div>\n" .
> -			      "<div class=\"readme\">\n";
> -			print $_ while (<$fd>);
> -			print "\n</div>\n"; # class="readme"
> -			close $fd;
> -		}
> +		print "<div class=\"title\">readme</div>\n" .
> +		      "<div class=\"readme\">\n";
> +		insert_file("$projectroot/$project/README.html");
> +		print "\n</div>\n"; # class="readme"
> +		close $fd;
>  	}
>  
>  	# we need to request one more than 16 (0..15) to check if

I'll queue it to 'pu', with the "close $fd" removed, for now.

^ permalink raw reply

* Re: [PATCH] git-svn: Make branch use correct svn-remote
From: Junio C Hamano @ 2008-12-03  3:55 UTC (permalink / raw)
  To: Eric Wong; +Cc: Deskin Miller, git, gitster
In-Reply-To: <20081202215157.GB9650@hand.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> Deskin Miller <deskinm@umich.edu> wrote:
>> The 'branch' subcommand incorrectly had the svn-remote to use hardcoded
>> as 'svn', the default remote name.  This meant that branches derived
>> from other svn-remotes would try to use the branch and tag configuration
>> for the 'svn' remote, potentially copying would-be branches to the wrong
>> place in SVN, into the branch namespace for another project.
>> 
>> Fix this by using the remote name extracted from the svn info for the
>> specified git ref.  Add a testcase for this behaviour.
>> 
>> Signed-off-by: Deskin Miller <deskinm@umich.edu>
>
> Looks alright to me, thanks Deskin.
>
> Acked-by: Eric Wong <normalperson@yhbt.net>

Does not work for me X-<.

* expecting success:
        (svn co "$svnrepo" svn &&
        cd svn &&
        mkdir mirror &&
        svn add mirror &&
        svn copy trunk tags branches mirror/ &&
        svn ci -m "made mirror" ) &&
        rm -rf svn &&
        git svn init -s -R mirror --prefix=mirror/ "$svnrepo"/mirror &&
        git svn fetch -R mirror &&
        git checkout mirror/trunk &&
        base=$(git rev-parse HEAD:) &&
        git svn branch -m "branch in mirror" d &&
        test $base = $(git rev-parse remotes/mirror/d:) &&
        test_must_fail git rev-parse remotes/d

A    svn/trunk
A    svn/trunk/foo
A    svn/branches
A    svn/branches/a
A    svn/branches/a/foo
A    svn/branches/b
A    svn/branches/b/foo
A    svn/tags
A    svn/tags/tag4
A    svn/tags/tag4/foo
A    svn/tags/tag1
A    svn/tags/tag1/foo
A    svn/tags/tag2
A    svn/tags/tag2/foo
A    svn/tags/tag3
A    svn/tags/tag3/foo
Checked out revision 8.
A         mirror
svn: Client error in parsing arguments
* FAIL 4: branch uses correct svn-remote

                (svn co "$svnrepo" svn &&
                cd svn &&
                mkdir mirror &&
                svn add mirror &&
                svn copy trunk tags branches mirror/ &&
                svn ci -m "made mirror" ) &&
                rm -rf svn &&
                git svn init -s -R mirror --prefix=mirror/ "$svnrepo"/mirror &&
                git svn fetch -R mirror &&
                git checkout mirror/trunk &&
                base=$(git rev-parse HEAD:) &&
                git svn branch -m "branch in mirror" d &&
                test $base = $(git rev-parse remotes/mirror/d:) &&
                test_must_fail git rev-parse remotes/d

^ permalink raw reply

* Re: [PATCH 1/5] avoid parse_sha1_header() accessing memory out of bound
From: Liu Yubao @ 2008-12-03  3:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202154257.GK23984@spearce.org>

Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 6c0e251..efe6967 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -1254,10 +1255,10 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
>>  	/*
>>  	 * The type can be at most ten bytes (including the
>>  	 * terminating '\0' that we add), and is followed by
>> -	 * a space.
>> +	 * a space, at least one byte for size, and a '\0'.
>>  	 */
>>  	i = 0;
>> -	for (;;) {
>> +	while (hdr < hdr_end - 2) {
>>  		char c = *hdr++;
>>  		if (c == ' ')
>>  			break;
>> @@ -1265,6 +1266,8 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
>>  		if (i >= sizeof(type))
>>  			return -1;
> 
> That first hunk I am citing is unnecessary, because of the lines
> right above.  All of the callers of this function pass in a buffer
> that is at least 32 bytes in size; this loop aborts if it does not
> find a ' ' within the first 10 bytes of the buffer.  We'll never
> access memory outside of the buffer during this loop.
> 
> So IMHO your first three hunks here aren't necessary.
> 

Seems you missed the cover letter sent as patch 0/5, all patches are explained
in the cover letter, sorry I sent them as separate topics by mistake.

This bound check is mainly for uncompressed loose object, a loose object
that just are uncompressed:

uncompressed loose object = inflate(loose object)
loose object = deflate(typename + <space> + size + '\0' + data)

I'm doing a defensive programming, for uncompressed loose object the mmapped
memory is passed to parse_sha1_header without being checked by inflateInit() first,
so there may be a SIGSEGV crash for a corrupted uncompressed loose object.


>> @@ -1275,7 +1278,7 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
>>  	if (size > 9)
>>  		return -1;
>>  	if (size) {
>> -		for (;;) {
>> +		while (hdr < hdr_end - 1) {
>>  			unsigned long c = *hdr - '0';
>>  			if (c > 9)
>>  				break;
> 
> OK, there's no promise here that we don't roll off the buffer.
> 
> This can be fixed in the caller, ensuring we always have the '\0'
> at some point in the initial header buffer we were asked to parse:
> 
Isn't it easier to solve this problem in one place and maintain it? Maybe someday
someone forgets parse_sha1_header requires a null terminated buffer, and a corrupted
uncompressed loose object even doesn't have to be null terminated (if there will be
this kind of loose object).

^ permalink raw reply

* Re: git-p4 submit, Can't clobber writable file
From: Jing Xue @ 2008-12-03  3:07 UTC (permalink / raw)
  To: Gary Yang; +Cc: git
In-Reply-To: <40586.80065.qm@web37902.mail.mud.yahoo.com>

On Tue, Dec 02, 2008 at 02:30:51PM -0800, Gary Yang wrote:
> 
> I followed the instructions at http://modular.math.washington.edu/home/mhansen/git-1.5.5.1/contrib/fast-import/git-p4.txt
> 
> But, I am not able to git-p4 submit. Any idea?
> 
> git-p4 clone //build/scripts build_scripts
> cd build_scripts
> vi foo.h
> git commit foo.h
> git-p4 rebase
> git-p4 submit
> 
>   from sets import Set;
> Perforce checkout for depot path //build/scripts/ located at /home/gyang/workspace/build_scripts/
> Syncronizing p4 checkout...
> //build/scripts/foo.h#1 - added as /home/gyang/workspace/build_scripts/foo.h
> Can't clobber writable file /home/gyang/workspace/build_scripts/foo.h
> //build/scripts/foo.c#1 - added as /home/gyang/workspace/build_scripts/foo.c
> Can't clobber writable file /user/home/gyang/workspace/build_scripts/foo.c
> ......
> command failed: p4 sync ...

You might want to clone to a git working dir different than the p4
working dir.

For instance, if your p4 workspace has the working dir set to
build_scripts/, try 'git p4 clone //build/scripts build_scripts.git'.

You would then normally work under build_scripts.git/. build_scripts/
would only be used by git-p4 at submission time.

Cheers.
-- 
Jing Xue

^ permalink raw reply

* Re: [PATCH] t4030-diff-textconv: Make octal escape sequence more portable
From: Junio C Hamano @ 2008-12-03  2:18 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jeff King, Git Mailing List
In-Reply-To: <4934F245.9020908@viscovery.net>

Thanks.

^ permalink raw reply

* Re: [PATCH] Add a built-in alias for 'stage' to the 'add' command
From: Junio C Hamano @ 2008-12-03  2:18 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Scott Chacon, git, peff
In-Reply-To: <fcaeb9bf0812020436n69a21c96g4c13976e493372d9@mail.gmail.com>

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> On 12/2/08, Scott Chacon <schacon@gmail.com> wrote:
>>  diff --git a/git.c b/git.c
>>  index 89feb0b..9e5813c 100644
>>  --- a/git.c
>>  +++ b/git.c
>>  @@ -266,6 +266,7 @@ static void handle_internal_command(int argc, const char **argv)
>>         const char *cmd = argv[0];
>>         static struct cmd_struct commands[] = {
>>                 { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>>  +               { "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>>                 { "annotate", cmd_annotate, RUN_SETUP },
>>                 { "apply", cmd_apply },
>>                 { "archive", cmd_archive },
>
> Nit-picking. There are some references to "git add" in builtin-add.c,
> like help usage or error message ("Maybe you wanted to say 'git add
> .'?\n"). Should it refer to "git stage" instead as well?

I do not think that is necessary.  As git-stage(1) makes it clear, it is
an alias to "add", not the other way around.

^ permalink raw reply

* Re: [PATCH] Add a built-in alias for 'stage' to the 'add' command
From: Junio C Hamano @ 2008-12-03  2:18 UTC (permalink / raw)
  To: Scott Chacon; +Cc: git, peff
In-Reply-To: <20081202061455.GA48848@agadorsparticus>

Thanks.

^ permalink raw reply

* Re: [PATCHv3bis] gitweb: fixes to gitweb feature check code
From: Junio C Hamano @ 2008-12-03  2:18 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <1228214598-26557-1-git-send-email-giuseppe.bilotta@gmail.com>

Thanks, but the ones based on the previous round were already queued in
'next', so I took the change to the check/get comment (the first hunk in
this squashed patch) not to lose the remaining improvements from this
patch.

^ permalink raw reply

* Re: [PATCH] Makefile: introduce NO_PTHREADS
From: Junio C Hamano @ 2008-12-03  2:18 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Mike Ralphson
In-Reply-To: <4934E6BC.9040203@viscovery.net>

Thanks, both.

^ permalink raw reply

* Re: [PATCH] User's Manual: remove duplicated url at the end of Appendix B
From: Junio C Hamano @ 2008-12-03  2:17 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: J. Bruce Fields, git
In-Reply-To: <1228157402-10961-1-git-send-email-vmiklos@frugalware.org>

Thanks.

^ permalink raw reply

* Re: [PATCH 2/2 v2] Documentation: describe how to "bisect skip" a range of commits
From: Junio C Hamano @ 2008-12-03  2:17 UTC (permalink / raw)
  To: Christian Couder; +Cc: Johannes Schindelin, Johannes Sixt, git, H. Peter Anvin
In-Reply-To: <20081202145351.24d1a1fb.chriscool@tuxfamily.org>

Thanks.

^ permalink raw reply

* Re: summaries in git add --patch
From: Junio C Hamano @ 2008-12-03  2:15 UTC (permalink / raw)
  To: William Pursell; +Cc: git
In-Reply-To: <49308B4B.3070703@gmail.com>

William Pursell <bill.pursell@gmail.com> writes:

> Junio C Hamano wrote:
>> William Pursell <bill.pursell@gmail.com> writes:
>>
>>> Here's a new patch.  Instead of displaying the summary and then
>>> the current hunk, it implements a 'goto' command.
>>
>> I take it that this is for discussion not for immediate inclusion.
>
> Yes.  I tend to think of all of my patches as being merely
> for discussion since I'm not terribly familiar with the code
> base and expect to miss many things.  I'm flattered that
> you would even consider them for inclusion.  For that matter,
> I'm flattered that you have even responded to my submissions!

Thanks.  I value contributions from people who are enthused and can make a
good case for the change they propose.  I utter comments and sometimes
even send out an alternative implementation to illustrate what might be a
better approach.  IOW, I try to help people make progress.

One thing I will not do after such a discussion, unless I am really really
interested in having the new feature personally myself, is to go back to
the discussion thread and assemble the pieces together to make the final
series of patches for inclusion.  The responsibility for doing that lies
on the original contributor.

^ permalink raw reply

* X-Debbugs-Cc didn't make it to git@vger.kernel.org
From: jidanni @ 2008-12-03  1:32 UTC (permalink / raw)
  To: git

Bummer, on
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507475
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507476
I used X-Debbugs-Cc, and it says Report forwarded to git@vger.kernel.org
but I don't see them here on nntp:gmane.comp.version-control.git .
Perhaps they got filtered out?

^ permalink raw reply

* Re: [PATCH] gitweb: fixes to gitweb feature check code
From: Jakub Narebski @ 2008-12-03  1:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <7v4p1mp7hx.fsf@gitster.siamese.dyndns.org>

Dnia wtorek 2. grudnia 2008 22:55, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > What I like about having all this, i.e. fix, futureproof and style
> > correction in one single patch is the fact that fix doesn't introduce
> > strange looking (gitweb_check_feature('bool_feat'))[0]... well, except
> > encapsulated in a subroutine.
> >
> > From all possible splits of this feature into series of up to three
> > patches I think I like the one with pure subroutine rename from *check*
> > to *get* least...
> 
> Well, I have to say that you have a strange taste, sense of priorities,
> and perhaps aversion to logical progression.  Let's explain one more
> time.
> 
> The case we had at hand was that a callee has a less-than-ideal calling
> convention that has caused a few bugs by callers because they did not
> understand the calling convention.  You can argue it is not entirely
> caller's fault that they failed to follow the calling convention, but the
> fact remains that there are bugs taken as a whole.
> 
> First we fix the callers, because existing bugs get highest priority.
> This is a pure bugfix patch that could even go to maintenance "bugfix
> only" branch.

I agree here.

> Then we fix the calling convention because we all know that the calling
> convention was less-than-ideal.  A large part of the reason the calling
> convention was confusing was because the wording "check" implied it was a
> boolean function.  Logically, s/check/get/ would be a major part of fixing
> that.

And here I slightly disagree. The lone change s/check/get/ doesn't
actually do anything. Splitting wrongly named 'check' into checking
if feature is enabled and actual getting config does. And it is IMVHO
quote obvious which calls are to be to new semantic of 'check':
  my ($var) = gitweb_check_feature('feature');
and
  (gitweb_check_feature('feature'))[0]

and which are to be renamed to 'get':
  my ($a, $b) = gitweb_check_feature('feature');
and
  my @a  = gitweb_check_feature('feature');

Perhaps it is more clear from the point of view of reviewing individual
pieces...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: more merge strategies : feature request
From: Leo Razoumov @ 2008-12-03  1:07 UTC (permalink / raw)
  To: Caleb Cushing; +Cc: git
In-Reply-To: <81bfc67a0812020546o79906a20jcd04bd42d18dd803@mail.gmail.com>

On 12/2/08, Caleb Cushing <xenoterracide@gmail.com> wrote:
> > I guess that "no-overwrite" can be achieved by
>  >
>  >  git merge -s ours --no-commit
>
>
> no it doesn't. which is why I called it a bad name. no-overwrite would
>  still add new lines to the file not in ours (and no-commit isn't
>  needed in that case) it just wouldn't overwrite conflicting lines, my
>  understanding of ours is that it will keep the files as is.
>
> Caleb Cushing
>

>From your original email in this thread

"no-overwrite: if a change from the branch being merged in would
overwrite something in the current branch don't merge it. (I think it
needs a better name)"

I got the impression that you would like to preserve "ours" branch
whenever other branch tries to overwrite something? Is it
"no-override-conflicting-lines" that you are really after?

--Leo--

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox