Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation
From: Vitor Antunes @ 2012-01-17  0:10 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git
In-Reply-To: <CAOpHH-UkyK-c_AHUOPbQQmW9cQQypDvirMR0Jb7vTGSQF7RZpw@mail.gmail.com>

On Mon, Jan 16, 2012 at 11:41 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:
>> 1.  Move the tempBranch commit outside of the "for blob" loop.
>>    It can have no parent, and the diff-tree will still tell you
>>    if you found the same contents.  Instead of a ref for
>>    each blob inspected for each change, you'll just have one ref
>>    per change.  Only one checkpoint() after the tempBranch
>>    commit should be needed.
>
> You're right. Completely oversaw that. Will improve the code
> accordingly.

Apparently I did not oversee it. Assume you have added a new file to
HEAD of parent branch, but you branched from a previous commit. When the
new branch is committed over HEAD the new file will, incorrectly, be
part of it and diff-tree will not work as expected.

I should avoid taking 6 months to submit a patch to avoid forgetting why
I did what I did :)

Vitor

^ permalink raw reply

* Re: Commit changes to remote repository
From: Junio C Hamano @ 2012-01-17  0:12 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Carlos Martín Nieto, ruperty, git
In-Reply-To: <vpqboq6fjfy.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Carlos Martín Nieto <cmn@elego.de> writes:
>
>> You're trying to push to a non-bare repository and change the
>> currently active branch, which can cause problems, so git isn't
>> letting you. There's an explanation of bare and non-bare at
>> http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
>> you should init the repo you want to use as the central point with
>> --bare and do modifications locally and then push there.
>
> An alternative is to push to a temporary, non-checked-out branch.

Or more generally, treat such a push as if you are pulling in the opposite
direction. So in this example,

> I sometimes do
>
>   laptop$ git push desktop HEAD:incomming
>
> and then
>
>   desktop$ git merge incomming

you pretend as if you are running "git pull" on your desktop in order to
integrate the work done on your laptop. If you did

    desktop$ git pull laptop

you would store where the branches on the laptop are in the remote
tracking branches for "laptop" remote in your desktop's repository.

So a good way to simulate that would be:

    laptop$ git push desktop master:refs/remotes/laptop/master

and then run:

    desktop$ git merge laptop/master

> The push does not disturb the worktree on the desktop, and the merge is
> done manually on the receiving machine.

^ permalink raw reply

* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
From: Nguyen Thai Ngoc Duy @ 2012-01-17  1:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7vaa5nutbp.fsf@alter.siamese.dyndns.org>

2012/1/17 Junio C Hamano <gitster@pobox.com>:
> A new command line option "--no-check-intent-to-add" to "commit" without
> any configuration bit may be a useful addition as a first cut, and in
> order to help users to which "there are situations" is more than 50% of
> the time, a configuration that can be overriden by "--check-intent-to-add"
> may be a usability improvement over that first cut, but if this is really
> about "there are situations", then a configuration that cannot be
> overriden by command line option feels a wrong way to go about it.

"git -c key=value commit" may help.

> Is this really about "there are situations" to begin with? I am suspecting
> that this patch is either:
>
>  (1) making it easier to use a wrong workflow, by promoting a way to
>     bypass a useful safety measure;
>
>  (2) fixing an earlier UI mistake (iow, the interpretation #2 in the old
>     discussion is always the right one and the existing safety measure is
>     misguided) in such a way that allows you to work around an objection
>     from a bonehead maintainer who refuses to admit that earlier mistake;
>     and/or
>
>  (3) splitting the Git userbase into two and making the resulting system
>     harder to teach.

This patch is towards (3). I agree that "add -N" can be confusing to
new users because it does not actually add anything. Those who do not
read manual carefully and commit without checking the result may fall
into that trap. So a user is expected to "level up" a bit and
hopefully by the time he/she feels the need to get around the safety
check and discover this flag, they should be ready to go without the
safety check.

Although I would not oppose deprecating the old behavior if you go with (2) ;)

> In any case, for this change to help people who add more than one paths
> with "add -N" and want to include only a subset of them in the commit, we
> may want to explicitly teach them to add what they want to before
> committing with the new command line option in the documentation.

yeah, keep telling people "this does not add any thing, you need to
git-add again without -N" after running "add -N" using advice
framework when this config is on?
-- 
Duy

^ permalink raw reply

* Re: git grep doesn't follow symbolic link
From: Nguyen Thai Ngoc Duy @ 2012-01-17  1:55 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Pang Yan Han, Thomas Rast, Ramkumar Ramachandra, Bertrand BENOIT,
	git
In-Reply-To: <7vwr8ruv1j.fsf@alter.siamese.dyndns.org>

On Tue, Jan 17, 2012 at 5:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> It's not wrong per se. It's an implication that users have to take
>> when they choose to use it. We may help make it clear that the
>> symlinks point to untracked files by putting some indication in the
>> diff.
>>
>> When I do "git log -Sfoo -- '*.cxx'" I don't really care if bar.cxx is
>> a symlink. Neither does my compiler. It may be a symlink's target
>> change that makes "foo" appear. Git could help me detect that quickly
>> instead of sticking with tracked contents only.
>
> As there is nothing in Git that tells that whatever is pointed at by
> bar.cxx that happens to be in your filesystem today had "foo" in it when
> that historical version of the commit whose bar.cxx symlink was updated to
> point to that file. It is *WRONG* to show the commit as something that
> changes bar.cxx to contain "foo" (or more precisely, changes the count of
> "foo" in it).
>
> Why is it so hard to understand?

OK resolving links to untracked contents is bad and should only be
supported in --no-index case, resolving links to tracked contents
should be ok in principal? (I'm not sure how messed up diff code could
be with these changes)
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
From: Jonathan Nieder @ 2012-01-17  2:47 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8B_qGphVz8PFhPNLsOe-Ve7xb+biNP8Ok7dXiygu3KoSg@mail.gmail.com>

Nguyen Thai Ngoc Duy wrote:

> Although I would not oppose deprecating the old behavior if you go with (2) ;)

My reaction on reading the patch and clarifying followup was similar
to Junio's.  I don't think a configuration item makes sense unless we
are planning to flip the default in the future, and it would probably
be clearer to document it that way.

By the way, thanks very much for working on this.  I wish I had more
time to offer more than comments.

Sincerely,
Jonathan

^ permalink raw reply

* What's cooking in git.git (Jan 2012, #04; Mon, 16)
From: Junio C Hamano @ 2012-01-17  3:27 UTC (permalink / raw)
  To: git

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

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo:

        git://git.kernel.org/pub/scm/git/git.git
        git://repo.or.cz/alt-git.git
        https://code.google.com/p/git-core/
        https://github.com/git/git

With only maint and master:

        git://git.sourceforge.jp/gitroot/git-core/git.git
        git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches:

        https://github.com/gitster/git

The preformatted documentation in HTML and man format are found in:

        git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
        git://repo.or.cz/git-{htmldocs,manpages}.git/
        https://code.google.com/p/git-{htmldocs,manpages}.git/
        https://github.com/gitster/git-{htmldocs,manpages}.git/

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

* mh/maint-show-ref-doc (2012-01-13) 2 commits
  (merged to 'next' on 2012-01-16 at 8573f09)
 + git-show-ref doc: typeset regexp in fixed width font
 + git-show-ref: fix escaping in asciidoc source

Will merge to 'master' by v1.7.9-rc2.

* nd/pathspec-recursion-cleanup (2012-01-16) 2 commits
  (merged to 'next' on 2012-01-16 at 0189264)
 + diff-index: enable recursive pathspec matching in unpack_trees
 + Document limited recursion pathspec matching with wildcards

Will merge to 'master' by v1.7.9-rc2.

* tr/maint-word-diff-incomplete-line (2012-01-12) 1 commit
  (merged to 'next' on 2012-01-16 at 58ddaaf)
 + word-diff: ignore '\ No newline at eof' marker

Will merge to 'master' by v1.7.9-rc2.

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

* jc/request-pull-show-head-4 (2012-01-10) 1 commit
  (merged to 'next' on 2012-01-11 at 8d98a6b)
 + request-pull: use the real fork point when preparing the message

Hopefully the final finishing touch to the request-pull script that was
updated during this cycle.

* jk/maint-upload-archive (2012-01-11) 1 commit
  (merged to 'next' on 2012-01-11 at 5c0bfa9)
 + archive: re-allow HEAD:Documentation on a remote invocation

Running "git archive" remotely and asking for a partial tree of a ref,
e.g. HEAD:$path was forbidden by a recent change to tighten security, but
was found to be overly restrictive.

* jn/maint-gitweb-grep-fix (2012-01-05) 2 commits
  (merged to 'next' on 2012-01-13 at a15e6ab)
 + gitweb: Harden "grep" search against filenames with ':'
 + gitweb: Fix file links in "grep" search

* ss/maint-msys-cvsexportcommit (2012-01-11) 2 commits
  (merged to 'next' on 2012-01-11 at 007aab1)
 + git-cvsexportcommit: Fix calling Perl's rel2abs() on MSYS
 + t9200: On MSYS, do not pass Windows-style paths to CVS

--------------------------------------------------
[Stalled]

* jc/advise-push-default (2011-12-18) 1 commit
 - push: hint to use push.default=upstream when appropriate

Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?

Waiting for a reroll.

* jc/split-blob (2011-12-01) 6 commits
 . WIP (streaming chunked)
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - varint-in-pack: refactor varint encoding/decoding

Not ready.

At least pack-objects and fsck need to learn the new encoding for the
series to be usable locally, and then index-pack/unpack-objects needs to
learn it to be used remotely.

--------------------------------------------------
[Cooking]

* jc/pull-signed-tag (2012-01-11) 1 commit
 - merge: use editor by default in interactive sessions

Per Linus's strong suggestion, sugarcoated (aka "taking blame for the
original UI screw-ups") so that it is easier for me to swallow and accept
a potentially huge backward incompatibility issue, "git merge" is made to
launch an editor to explain the merge in the merge commit by default in
interactive sessions.

May need renaming the backward compatibility "GIT_MERGE_LEGACY"
environment variable, and also will need a smoother migration plan.

Will defer till the next cycle.

* nd/commit-ignore-i-t-a (2012-01-16) 2 commits
 - commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
 - cache-tree: update API to take abitrary flags

May want to consider this as fixing an earlier UI mistake, and not as a
feature that devides the userbase.

Will defer till the next cycle.

* nd/maint-refname-in-hierarchy-check (2012-01-11) 1 commit
 - Fix incorrect ref namespace check

Avoid getting confused by "ref/headxxx" and mistaking it as if it is under
the "refs/heads/" hierarchy.
Not urgent.

* jc/advise-i18n (2011-12-22) 1 commit
 - i18n of multi-line advice messages

Allow localization of advice messages that tend to be longer and
multi-line formatted. For now this is deliberately limited to advise()
interface and not vreportf() in general as touching the latter has
interactions with error() that has plumbing callers whose prefix "error: "
should never be translated.
Not urgent.

* rr/sequencer (2012-01-11) 2 commits
 - sequencer: factor code out of revert builtin
 - revert: prepare to move replay_action to header

Moving large chunk of code out of cherry-pick/revert for later reuse,
primarily to prepare for the next cycle.
Not urgent.

* tr/maint-mailinfo (2012-01-16) 2 commits
 - mailinfo: with -b, keep space after [foo]
 - am: learn passing -b to mailinfo

Looked reasonable.
Not urgent.

* jk/credentials (2012-01-11) 3 commits
  (merged to 'next' on 2012-01-16 at 1c6c94a)
 + unix-socket: do not let close() or chdir() clobber errno during cleanup
 + credential-cache: report more daemon connection errors
 + unix-socket: handle long socket pathnames

Minor fix-ups to the new feature.
Will merge to 'master' by v1.7.9-rc2.

* pw/p4-view-updates (2012-01-11) 5 commits
 - git-p4: add tests demonstrating spec overlay ambiguities
 - git-p4: adjust test to adhere to stricter useClientSpec
 - git-p4: clarify comment
 - git-p4: fix verbose comment typo
 - git-p4: only a single ... wildcard is supported

* rs/diff-postimage-in-context (2012-01-06) 1 commit
  (merged to 'next' on 2012-01-09 at 9635032)
 + xdiff: print post-image for common records instead of pre-image

Looked reasonable.
Not urgent.

* cb/push-quiet (2012-01-08) 3 commits
 - t5541: avoid TAP test miscounting
 - fix push --quiet: add 'quiet' capability to receive-pack
 - server_supports(): parse feature list more carefully

Looked reasonable.
Not urgent.

* nd/clone-detached (2012-01-16) 10 commits
 - clone: print advice on checking out detached HEAD
 - clone: allow --branch to take a tag
 - clone: refuse to clone if --branch points to bogus ref
 - clone: --branch=<branch> always means refs/heads/<branch>
 - clone: delay cloning until after remote HEAD checking
 - clone: factor out remote ref writing
 - clone: factor out HEAD update code
 - clone: factor out checkout code
 - clone: write detached HEAD in bare repositories
 - t5601: add missing && cascade
 (this branch uses nd/clone-single-branch.)

Looking good.
Not urgent.

* nd/clone-single-branch (2012-01-08) 1 commit
  (merged to 'next' on 2012-01-09 at 6c3c759)
 + clone: add --single-branch to fetch only one branch
 (this branch is used by nd/clone-detached.)

Looked reasonable.
Not urgent.

* jn/gitweb-unspecified-action (2012-01-09) 1 commit
 - gitweb: Fix actionless dispatch for non-existent objects

Looked reasonable.
Not urgent.

* nd/index-pack-no-recurse (2012-01-16) 3 commits
 - index-pack: eliminate unlimited recursion in get_base_data()
 - index-pack: eliminate recursion in find_unresolved_deltas
 - Eliminate recursion in setting/clearing marks in commit list

Much better explained than the previous round.
Will defer till the next cycle.

* mh/ref-api-rest (2011-12-12) 35 commits
 - repack_without_ref(): call clear_packed_ref_cache()
 - read_packed_refs(): keep track of the directory being worked in
 - is_refname_available(): query only possibly-conflicting references
 - refs: read loose references lazily
 - read_loose_refs(): take a (ref_entry *) as argument
 - struct ref_dir: store a reference to the enclosing ref_cache
 - sort_ref_dir(): take (ref_entry *) instead of (ref_dir *)
 - do_for_each_ref_in_dir*(): take (ref_entry *) instead of (ref_dir *)
 - add_entry(): take (ref_entry *) instead of (ref_dir *)
 - search_ref_dir(): take (ref_entry *) instead of (ref_dir *)
 - find_containing_direntry(): use (ref_entry *) instead of (ref_dir *)
 - add_ref(): take (ref_entry *) instead of (ref_dir *)
 - read_packed_refs(): take (ref_entry *) instead of (ref_dir *)
 - find_ref(): take (ref_entry *) instead of (ref_dir *)
 - is_refname_available(): take (ref_entry *) instead of (ref_dir *)
 - get_loose_refs(): return (ref_entry *) instead of (ref_dir *)
 - get_packed_refs(): return (ref_entry *) instead of (ref_dir *)
 - refs: wrap top-level ref_dirs in ref_entries
 - get_ref_dir(): keep track of the current ref_dir
 - do_for_each_ref(): only iterate over the subtree that was requested
 - refs: sort ref_dirs lazily
 - sort_ref_dir(): do not sort if already sorted
 - refs: store references hierarchically
 - refs.c: rename ref_array -> ref_dir
 - struct ref_entry: nest the value part in a union
 - check_refname_component(): return 0 for zero-length components
 - free_ref_entry(): new function
 - refs.c: reorder definitions more logically
 - is_refname_available(): reimplement using do_for_each_ref_in_array()
 - names_conflict(): simplify implementation
 - names_conflict(): new function, extracted from is_refname_available()
 - repack_without_ref(): reimplement using do_for_each_ref_in_array()
 - do_for_each_ref_in_arrays(): new function
 - do_for_each_ref_in_array(): new function
 - do_for_each_ref(): correctly terminate while processesing extra_refs

The API for extra anchoring points may require rethought first; that would
hopefully make the "ref" part a lot simpler. And that is happening in
another topic (which has graduated to 'master').

Will defer till the next cycle.

* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
 - fixup! 15eaaf4
 - git-svn, perl/Git.pm: extend Git::prompt helper for querying users
  (merged to 'next' on 2012-01-05 at 954f125)
 + perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS

The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.

Will defer till the next cycle.

* cb/git-daemon-tests (2012-01-08) 5 commits
  (merged to 'next' on 2012-01-08 at 1db8351)
 + git-daemon tests: wait until daemon is ready
 + git-daemon: produce output when ready
 + git-daemon: add tests
 + dashed externals: kill children on exit
 + run-command: optionally kill children on exit

Will defer till the next cycle.

* jk/parse-object-cached (2012-01-06) 3 commits
  (merged to 'next' on 2012-01-08 at 8c6fa4a)
 + upload-pack: avoid parsing tag destinations
 + upload-pack: avoid parsing objects during ref advertisement
 + parse_object: try internal cache before reading object db

These are a bit scary changes, but I do think they are worth doing.
Will defer till the next cycle.

^ permalink raw reply

* Re: Signed tags in octopus merge..
From: Junio C Hamano @ 2012-01-17  3:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jacob Helwig, Git Mailing List
In-Reply-To: <CA+55aFzZXSCt1AwOotMZJ+GcNcJKL2OcsPOtaZ3=cvraJ=PD+Q@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Mon, Jan 16, 2012 at 2:58 PM, Jacob Helwig <jacob@technosorcery.net> wrote:
>>
>> My immediate thought regarding the "side branch #1" version is not
>> wanting to have to do the math (even though it's a simple n+1)
>
> "Math is hard, let's go shopping".
>
> But I think even barbie could do the "add one" thing. That said, I
> think the current thing is already more than good enough, and I don't
> think it's at all confusing to talk about "parent #2". In fact, I
> think it's more obvious than "side branch #1".

I think Jacob is correct to point out that mice are even dumber than
Barbie and cannot do the "add one" thing, so let's leave the output as is.

Thanks for an amusing response.

^ permalink raw reply

* Re: Re* Regulator updates for 3.3
From: Pete Harlan @ 2012-01-17  5:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Linus Torvalds, Mark Brown, Liam Girdwood, linux-kernel,
	Git Mailing List
In-Reply-To: <7v62gbussz.fsf@alter.siamese.dyndns.org>

On 01/16/2012 03:33 PM, Junio C Hamano wrote:
> Pete Harlan <pgit@pcharlan.com> writes:
> 
>> On 01/10/2012 10:59 PM, Junio C Hamano wrote:
>>> There may be existing scripts that leave the standard input and the
>>> standard output of the "git merge" connected to whatever environment the
>>> scripts were started, and such invocation might trigger the above
>>> "interactive session" heuristics. Such scripts can export GIT_MERGE_LEGACY
>>> environment variable set to "yes" to force the traditional behaviour.
>>
>> The name GIT_MERGE_LEGACY gives no clue about what flavor of legacy
>> merge behavior is being enabled.  Something like GIT_MERGE_LEGACY_EDIT
>> might be clearer, or perhaps just have GIT_MERGE_EDIT=0 to get the old
>> behavior without reference to whether or not that behavior is
>> considered legacy.
> 
> Hrm.
> 
> The only case your suggestion may make a difference would be when we find
> another earlier UI mistake we would want to correct in a backward
> incompatible way that affects _existing_ scripts.
> 
> With your suggestion, they need to export "GIT_MERGE_EDIT=0" today, and
> they will need to update again to export "GIT_MERGE_SOMETHINGELSE=0" when
> such an incompatible change comes.

Which is a good thing, because maybe they started using Git after the
current proposed change (which they like), and what you see as new
becomes their "legacy" behavior.  If you change something after that,
you can't use GIT_MERGE_LEGACY=yes for that one also because which
legacy is it preserving?

In general, naming configuration variables "DO_IT_<THIS_WAY>" instead
of "DO_IT_THE_OLD_WAY" is better because it's self-documenting.  The
only time I think I'd prefer "LEGACY" is if you're planning on
deprecating and removing it eventually and you want to indicate
something to that effect in the name.

--
Pete Harlan
pgit@pcharlan.com

^ permalink raw reply

* [PATCH 0/4] Remove a user of extra_refs in clone
From: mhagger @ 2012-01-17  5:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
	Michael Haggerty

From: Michael Haggerty <mhagger@alum.mit.edu>

When cloning, write_remote_refs() creates local packed refs from the
refs read from the remote repository.  It does this by creating extra
refs for the references then calling pack_refs() to bake the extra
refs into the packed-refs file, then calling clear_extra_refs().

This is silly and relies on the kludgy extra_refs mechanism, which I
want to get rid of.  Instead, add a function call add_packed_ref() to
the refs API, and use it to create packed refs (in the memory cache)
directly.  Then call pack_refs() as before to write the packed-refs
file.

Because the new add_packed_ref() function allows references (perhaps
many of them) to be added to an existing ref_array, it would be
inefficient to re-sort the list after every addition.  So instead,
append new entries to the end of the ref_array and note that the array
is unsorted.  Then, before the ref_array is used, check if it is
unsorted and sort it if necessary.

A side effect of this change is that the new packed references are
left in the in-memory packed reference cache after the return from
write_remote_refs() (whereas previously, the refs were stored as
temporary extra refs that were purged before return from the
function).  I can't see any place in the following code where this
would make a difference, but there is quite a bit of code there so it
is hard to audit.  Confirmation that this is OK would be welcome.

Michael Haggerty (4):
  pack_refs(): remove redundant check
  ref_array: keep track of whether references are sorted
  add_packed_ref(): new function in the refs API.
  write_remote_refs(): create packed (rather than extra) refs

 builtin/clone.c |    3 +--
 pack-refs.c     |    3 +--
 refs.c          |   39 ++++++++++++++++++++++++++++++++-------
 refs.h          |    6 ++++++
 4 files changed, 40 insertions(+), 11 deletions(-)

-- 
1.7.8.3

^ permalink raw reply

* [PATCH 2/4] ref_array: keep track of whether references are sorted
From: mhagger @ 2012-01-17  5:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
	Michael Haggerty
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>

From: Michael Haggerty <mhagger@alum.mit.edu>

Keep track of how many entries in a ref_array are already sorted.  In
sort_ref_array(), only call qsort() if the dir contains unsorted
entries (i.e., if references have been appended to the end of the list
since the last call to sort_ref_array()).

Sort ref_arrays only when needed, namely in search_ref_array() and in
do_for_each_ref().  However, never sort the extra_refs, because
extra_refs can contain multiple entries with the same name.

This change is currently not useful, because entries are not added to
ref_arrays after they are created.  But in a moment they will be...

Implementation note: we could store a binary "sorted" value instead of
an integer, but storing the number of sorted entries leaves the way
open for a couple of possible future optimizations:

* In sort_ref_array(), sort *only* the unsorted entries, then merge
  them with the sorted entries.  This should be faster if most of the
  entries are already sorted.

* Teach search_ref_array() to do a binary search of any sorted
  entries, and if unsuccessful do a linear search of any unsorted
  entries.  This would avoid the need to sort the list every time that
  search_ref_array() is called, and (given some intelligence about how
  often to sort) could significantly improve the speed in certain
  hypothetical usage patterns.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   33 ++++++++++++++++++++++++++-------
 1 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index 6f436f1..268816f 100644
--- a/refs.c
+++ b/refs.c
@@ -17,6 +17,15 @@ struct ref_entry {
 
 struct ref_array {
 	int nr, alloc;
+
+	/*
+	 * Entries with index 0 <= i < sorted are sorted by name.  New
+	 * entries are appended to the list unsorted, and are sorted
+	 * only when required; thus we avoid the need to sort the list
+	 * after the addition of every reference.
+	 */
+	int sorted;
+
 	struct ref_entry **refs;
 };
 
@@ -105,12 +114,18 @@ static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2
 	}
 }
 
+/*
+ * Sort the entries in array (if they are not already sorted).
+ */
 static void sort_ref_array(struct ref_array *array)
 {
 	int i, j;
 
-	/* Nothing to sort unless there are at least two entries */
-	if (array->nr < 2)
+	/*
+	 * This check also prevents passing a zero-length array to qsort(),
+	 * which is a problem on some platforms.
+	 */
+	if (array->sorted == array->nr)
 		return;
 
 	qsort(array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
@@ -124,7 +139,7 @@ static void sort_ref_array(struct ref_array *array)
 		}
 		array->refs[++i] = array->refs[j];
 	}
-	array->nr = i + 1;
+	array->sorted = array->nr = i + 1;
 }
 
 static struct ref_entry *search_ref_array(struct ref_array *array, const char *refname)
@@ -137,7 +152,7 @@ static struct ref_entry *search_ref_array(struct ref_array *array, const char *r
 
 	if (!array->nr)
 		return NULL;
-
+	sort_ref_array(array);
 	len = strlen(refname) + 1;
 	e = xmalloc(sizeof(struct ref_entry) + len);
 	memcpy(e->name, refname, len);
@@ -168,6 +183,10 @@ static struct ref_cache {
 
 static struct ref_entry *current_ref;
 
+/*
+ * The extra_refs is never sorted, because it is allowed to contain entries
+ * with duplicate names.
+ */
 static struct ref_array extra_refs;
 
 static void clear_ref_array(struct ref_array *array)
@@ -176,7 +195,7 @@ static void clear_ref_array(struct ref_array *array)
 	for (i = 0; i < array->nr; i++)
 		free(array->refs[i]);
 	free(array->refs);
-	array->nr = array->alloc = 0;
+	array->sorted = array->nr = array->alloc = 0;
 	array->refs = NULL;
 }
 
@@ -268,7 +287,6 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
 		    !get_sha1_hex(refline + 1, sha1))
 			hashcpy(last->peeled, sha1);
 	}
-	sort_ref_array(array);
 }
 
 void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
@@ -404,7 +422,6 @@ static struct ref_array *get_loose_refs(struct ref_cache *refs)
 {
 	if (!refs->did_loose) {
 		get_ref_dir(refs, "refs", &refs->loose);
-		sort_ref_array(&refs->loose);
 		refs->did_loose = 1;
 	}
 	return &refs->loose;
@@ -720,6 +737,8 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
 	for (i = 0; i < extra->nr; i++)
 		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
 
+	sort_ref_array(packed);
+	sort_ref_array(loose);
 	while (p < packed->nr && l < loose->nr) {
 		struct ref_entry *entry;
 		int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH 3/4] add_packed_ref(): new function in the refs API.
From: mhagger @ 2012-01-17  5:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
	Michael Haggerty
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>

From: Michael Haggerty <mhagger@alum.mit.edu>

Add a new function add_packed_ref() that adds a reference directly to
the in-memory packed reference cache.  This will be useful for
creating local references while cloning.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
This new function call only stores the new reference to the in-memory
cache.  The user has to remember to call pack_refs() to actually write
the new reference(s) to the packed-refs file.  (I don't think it is
practical to make the write happen automatically.)

 refs.c |    6 ++++++
 refs.h |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index 268816f..14f764d 100644
--- a/refs.c
+++ b/refs.c
@@ -319,6 +319,12 @@ static struct ref_array *get_packed_refs(struct ref_cache *refs)
 	return &refs->packed;
 }
 
+void add_packed_ref(const char *refname, const unsigned char *sha1)
+{
+	add_ref(get_packed_refs(get_ref_cache(NULL)),
+			create_ref_entry(refname, sha1, REF_ISPACKED, 1));
+}
+
 static void get_ref_dir(struct ref_cache *refs, const char *base,
 			struct ref_array *array)
 {
diff --git a/refs.h b/refs.h
index d498291..00ba1e2 100644
--- a/refs.h
+++ b/refs.h
@@ -51,6 +51,12 @@ extern int for_each_rawref(each_ref_fn, void *);
 extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
 
 /*
+ * Add a reference to the in-memory packed reference cache.  To actually
+ * write the reference to the packed-refs file, call pack_refs().
+ */
+extern void add_packed_ref(const char *refname, const unsigned char *sha1);
+
+/*
  * Extra refs will be listed by for_each_ref() before any actual refs
  * for the duration of this process or until clear_extra_refs() is
  * called. Only extra refs added before for_each_ref() is called will
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH 4/4] write_remote_refs(): create packed (rather than extra) refs
From: mhagger @ 2012-01-17  5:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
	Michael Haggerty
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>

From: Michael Haggerty <mhagger@alum.mit.edu>

write_remote_refs() creates new packed refs from references obtained
from the remote repository, which is "out of thin air" as far as the
local repository is concerned.  Previously it did this by creating
"extra" refs, then calling pack_refs() to bake them into the
packed-refs file.  Instead, create packed refs (in the packed
reference cache) directly, then call pack_refs().

Aside from being more logical, this is another step towards removing
extra refs entirely.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/clone.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 86db954..9413537 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -441,11 +441,10 @@ static void write_remote_refs(const struct ref *local_refs)
 	for (r = local_refs; r; r = r->next) {
 		if (!r->peer_ref)
 			continue;
-		add_extra_ref(r->peer_ref->name, r->old_sha1, 0);
+		add_packed_ref(r->peer_ref->name, r->old_sha1);
 	}
 
 	pack_refs(PACK_REFS_ALL);
-	clear_extra_refs();
 }
 
 static int write_one_config(const char *key, const char *value, void *data)
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH 1/4] pack_refs(): remove redundant check
From: mhagger @ 2012-01-17  5:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
	Michael Haggerty
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>

From: Michael Haggerty <mhagger@alum.mit.edu>

handle_one_ref() only adds refs to the cbdata.ref_to_prune list if
(cbdata.flags & PACK_REFS_PRUNE) is set.  So any references in this
list at the end of pack_refs() can be pruned unconditionally.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Trivial simplification, not essential to the rest of this patch
series.

 pack-refs.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/pack-refs.c b/pack-refs.c
index 23bbd00..f09a054 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -143,7 +143,6 @@ int pack_refs(unsigned int flags)
 	packed.fd = -1;
 	if (commit_lock_file(&packed) < 0)
 		die_errno("unable to overwrite old ref-pack file");
-	if (cbdata.flags & PACK_REFS_PRUNE)
-		prune_refs(cbdata.ref_to_prune);
+	prune_refs(cbdata.ref_to_prune);
 	return 0;
 }
-- 
1.7.8.3

^ permalink raw reply related

* Re: [PATCHv3 10/13] credentials: add "cache" helper
From: Jeff King @ 2012-01-17  6:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <20120110045733.GA12460@sigill.intra.peff.net>

On Mon, Jan 09, 2012 at 11:57:33PM -0500, Jeff King wrote:

> Subject: [PATCH] credential-cache: report more daemon connection errors
> 
> Originally, this code remained relatively silent when we
> failed to connect to the cache. The idea was that it was
> simply a cache, and we didn't want to bother the user with
> temporary failures (the worst case is that we would simply
> ask their password again).
> 
> However, if you have a configuration failure or other
> problem, it is helpful for the daemon to report those
> problems. Git will happily ignore the failed error code, but
> the extra information to stderr can help the user diagnose
> the problem.

This actually has a minor regression, fixed below.

-- >8 --
Subject: [PATCH] credential-cache: ignore "connection refused" errors

The credential-cache helper will try to connect to its
daemon over a unix socket. Originally, a failure to do so
was silently ignored, and we would either give up (if
performing a "get" or "erase" operation), or spawn a new
daemon (for a "store" operation).

But since 8ec6c8d, we try to report more errors. We detect a
missing daemon by checking for ENOENT on our connection
attempt.  If the daemon is missing, we continue as before
(giving up or spawning a new daemon). For any other error,
we die and report the problem.

However, checking for ENOENT is not sufficient for a missing
daemon. We might also get ECONNREFUSED if a dead daemon
process left a stale socket. This generally shouldn't
happen, as the daemon cleans up after itself, but the daemon
may not always be given a chance to do so (e.g., power loss,
"kill -9").

The resulting state is annoying not just because the helper
outputs an extra useless message, but because it actually
blocks the helper from spawning a new daemon to replace the
stale socket.

Fix it by checking for ECONNREFUSED.

Signed-off-by: Jeff King <peff@peff.net>
---
If we really want to go belt-and-suspenders, the logic should perhaps be
changed to:

  if (send_request(socket, &buf < 0) {
          /* if we're starting a new one, who cares why it didn't work */
          if (flags & FLAG_SPAWN) {
                  spawn_daemon(socket);
                  if (send_request(socket, &buf) < 0)
                          die_errno("unable to connect to spawned daemon");
          }
          /* otherwise, report any non-minor errors */
          else if(errno != ENOENT && errno != ECONNREFUSED)
                  die_errno("unable to connect to cache daemon");
          /* otherwise we are just missing the daemon, and we can ignore */
  }

but that implies there is some condition besides ENOENT and ECONNREFUSED
where actually starting a new daemon (which will try to unlink whatever
is there now!) would be a good idea. I'd rather be conservative and
see if anybody reports a real-world case.

 credential-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/credential-cache.c b/credential-cache.c
index 1933018..9a03792 100644
--- a/credential-cache.c
+++ b/credential-cache.c
@@ -72,7 +72,7 @@ static void do_cache(const char *socket, const char *action, int timeout,
 	}
 
 	if (send_request(socket, &buf) < 0) {
-		if (errno != ENOENT)
+		if (errno != ENOENT && errno != ECONNREFUSED)
 			die_errno("unable to connect to cache daemon");
 		if (flags & FLAG_SPAWN) {
 			spawn_daemon(socket);
-- 
1.7.9.rc0.33.gd3c17

^ permalink raw reply related

* Re: Re* Regulator updates for 3.3
From: Junio C Hamano @ 2012-01-17  6:13 UTC (permalink / raw)
  To: Pete Harlan
  Cc: Linus Torvalds, Mark Brown, Liam Girdwood, linux-kernel,
	Git Mailing List
In-Reply-To: <4F15080C.6060004@pcharlan.com>

Pete Harlan <pgit@pcharlan.com> writes:

> ... The
> only time I think I'd prefer "LEGACY" is if you're planning on
> deprecating and removing it eventually and you want to indicate
> something to that effect in the name.

The discussion that led to the naming of that LEGACY token needs to be
re-read, then. The kind of "LEGACY" you prefer is exactly why the
environment variable is called LEGACY in the patch you are commenting on,
written in response to Linus's suggestion to switch the default, even
though I am not 100% buying it.

Having said that, I think I am wasting my time responding to this thread
during the feature-freeze period for v1.7.9, as I am not a big fan of
switching the default without adequate warning and transition plans, after
getting burned by the "'git-foo' vs 'git foo'" flames back in the v1.6.0
release. We would likely to take a gradual and smoother migration route to
transition, e.g. v1.7.9 to introduce "merge --edit", v1.7.10 to introduce
a configuration variable merge.edit (lack of which gives a warning and an
advice message while defaulting to 'no' to preserve the traditional
behaviour), and finally v1.8.0 (or v2.0) to flip the default to 'yes'
(while the configuration still giving a warning and an advice message)
that "merge --no-edit" can still countermand.

So you have until v1.7.10 to decide a good name for the overriding
environment variable.

^ permalink raw reply

* Re: [PATCH 0/4] Remove a user of extra_refs in clone
From: Jeff King @ 2012-01-17  6:35 UTC (permalink / raw)
  To: mhagger; +Cc: Junio C Hamano, git, Jakub Narebski, Heiko Voigt, Johan Herland
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>

On Tue, Jan 17, 2012 at 06:50:30AM +0100, mhagger@alum.mit.edu wrote:

> When cloning, write_remote_refs() creates local packed refs from the
> refs read from the remote repository.  It does this by creating extra
> refs for the references then calling pack_refs() to bake the extra
> refs into the packed-refs file, then calling clear_extra_refs().
> 
> This is silly and relies on the kludgy extra_refs mechanism, which I
> want to get rid of.  Instead, add a function call add_packed_ref() to
> the refs API, and use it to create packed refs (in the memory cache)
> directly.  Then call pack_refs() as before to write the packed-refs
> file.

I certainly approve of the goal.

> Because the new add_packed_ref() function allows references (perhaps
> many of them) to be added to an existing ref_array, it would be
> inefficient to re-sort the list after every addition.  So instead,
> append new entries to the end of the ref_array and note that the array
> is unsorted.  Then, before the ref_array is used, check if it is
> unsorted and sort it if necessary.

Makes sense.

> A side effect of this change is that the new packed references are
> left in the in-memory packed reference cache after the return from
> write_remote_refs() (whereas previously, the refs were stored as
> temporary extra refs that were purged before return from the
> function).  I can't see any place in the following code where this
> would make a difference, but there is quite a bit of code there so it
> is hard to audit.  Confirmation that this is OK would be welcome.

Actually, I think you may be fixing an extremely minor bug with this.

If later code in clone tries to resolve one of the refs in
refs/remotes/<origin>/, the current code will see that it doesn't exist
as a ref file (because we wrote it packed) and call get_packed_ref. That
checks the cached refs list, which will claim that did_packed is true,
but the "packed" array will be empty. Which is wrong; we _do_ have that
ref, and our cache is stale. After writing the packed list, the current
code probably ought to be calling invalidate_ref_cache(). It was only
the fact that most of the remaining code didn't care that this wasn't a
bug in the first place.

Your code makes more sense, in that it will keep the packed_refs list up
to date, and later calls to resolve_ref will properly find those refs.

The only place where I can detect a change in behavior is in the reflog
creation. When we write the refs/remotes/<origin>/HEAD ref, we call
create_symref, which in turn will decide whether to write a reflog entry
or not based on whether the pointed-to ref exists (because if we are
making a symref to something that doesn't exist, we have no sha1 to
write in the reflog entry). So before, we got no reflog for
refs/remotes/<origin>/HEAD (because we erroneously thought that
refs/remotes/origin/master (or whatever) did not exist). With your
patches, the reflog entry is created.

I doubt anyone ever noticed, but now that code is at least working as
intended.

> Michael Haggerty (4):
>   pack_refs(): remove redundant check
>   ref_array: keep track of whether references are sorted
>   add_packed_ref(): new function in the refs API.
>   write_remote_refs(): create packed (rather than extra) refs

I won't respond to each patch individually. All of them looked good to
me. Thanks for a very pleasant read.

-Peff

^ permalink raw reply

* Re: git grep doesn't follow symbolic link
From: Junio C Hamano @ 2012-01-17  6:44 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Pang Yan Han, Thomas Rast, Ramkumar Ramachandra, Bertrand BENOIT,
	git
In-Reply-To: <CACsJy8B9AGuRSx_5P22TOsqrA1rTEjQb78NN7PcTuK53iUmP_w@mail.gmail.com>

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

> OK resolving links to untracked contents is bad and should only be
> supported in --no-index case, resolving links to tracked contents should
> be ok in principal?

Conceptually it is not as bad, but I doubt it is still "ok".

It would defeat one of the fundamental properties of Git (or any content
based revision control scheme for that matter): a tree object records the
hash of its contents, so if two subtrees agree at the content hash level,
you do not have to descend into them to compare what they contain.

Imagine that you have a symlink at a/b/c/d that points a file e at the
root level, and you are running "git log a/b/c".  Even if the entire
hierarchy a/ does not change in a commit since its parent, you may have to
show a/b/c/d only because "e" has changed.

So I suspect that the required change would involve a lot more than a
naïve "when we reach the leaf level, if it is a symlink, read the link
contents and call get_tree_entry() to dereference the blob, or if the link
points outside the tree, use 0{40} to say 'contents undefined'".

After you compare 'a' of parent and child and find them to be identical,
you still need to anticipate that the hierarchy _might_ have a symbolic
link somewhere deep inside, and read _everything_ at least once in order
to find symbolic links and where they point at (if you did that to parent
already, and if you know that the child agrees with it at 'a', then you
can obviously do not have to read everything in the child---you know the
parent and the child have the same _contents_ in 'a' at that point). And
then grab the pointee out of parent tree and child tree to compare.

I personally do not think it is worth it.

^ permalink raw reply

* Re: [PATCH 0/4] Remove a user of extra_refs in clone
From: Junio C Hamano @ 2012-01-17  6:51 UTC (permalink / raw)
  To: mhagger; +Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>

Yay ;-)

Thanks.

^ permalink raw reply

* Re: [PATCHv3 10/13] credentials: add "cache" helper
From: Junio C Hamano @ 2012-01-17  6:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Jonathan Nieder
In-Reply-To: <20120117060232.GA27343@sigill.intra.peff.net>

Thanks, really appreciated.

^ permalink raw reply

* Re: Cannot push a commit
From: Matthias Fechner @ 2012-01-17  7:35 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20120116212036.GA21132@sigill.intra.peff.net>

Am 16.01.2012 22:20, schrieb Jeff King:
>   1. Something in the connection cut out but only in a half-duplex way.
>      I guess I could see ssh doing that if its input pipe closed, but
>      that would mean the local pack-objects failed, and I believe git
>      already detects and reports that case.
> 
>   2. The packfile sent indicated that it should have more bytes than it
>      does (i.e., either git indicated there were more objects than there
>      actually are, or zlib failed to give a stream-end marker in the
>      middle of an object). This is one of:
> 
>      a. A bug in git or zlib.
> 
>      b. Something in the connection corrupting the data (e.g., a
>         transport that is not 8-bit clean).
> 
> But in either 2a or 2b, I would expect us to have seen this before, and
> I don't recall seeing anything like it.
> 
> You could try generating a bundle with this pack, like:
> 
>   git bundle create foo.bundle master ^origin/master
> 
> and then shipping the resulting foo.bundle to the other side, and
> pulling from it like:
> 
>   git pull /path/to/foo.bundle master

thanks a lot for your answer.
I created a new repository which is not a bare repo to use your commands
posted above (i tested it with the normal push command from the client
and same error).

I created the bundle on the client, copied it with winscp to the server
and it was applied fine to the repository:
$ git pull ../foo.bundle master
Receiving objects: 100% (3/3), 91.63 KiB, done.
From ../foo.bundle
 * branch            master     -> FETCH_HEAD
Updating 53b9b5e..301e730
Fast-forward
 APP_UD.sch |19643
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 19643 insertions(+), 0 deletions(-)
 create mode 100644 APP_UD.sch


So the problem should be located anywhere else.
I tested now the same on a mac osx as client and there everything works
as expected, I added the file, commited it and pushed it without errors
to the server.

So the problem has something to do with windows as client. I will do
some additional test this evening.

Where can we continue to search?

Bye
Matthias

-- 

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the universe trying to
produce bigger and better idiots. So far, the universe is winning." --
Rich Cook

^ permalink raw reply

* Re: Bug? Git checkout fails with a wrong error message
From: Yves Goergen @ 2012-01-17  7:41 UTC (permalink / raw)
  To: Jeff King; +Cc: Holger Hellmuth, git, Carlos Martín Nieto
In-Reply-To: <20120116212709.GA21770@sigill.intra.peff.net>

On 16.01.2012 22:27 CE(S)T, Jeff King wrote:
> On Mon, Jan 16, 2012 at 10:20:42PM +0100, Yves Goergen wrote:
> 
>> On 16.01.2012 20:09 CE(S)T, Jeff King wrote:
>>> What is the output of "git config core.ignorecase" in your repository?
>> None, i.e. an empty line.
> 
> That's odd. When the repository is first created, git will do a test to
> see whether the filesystem supports case-sensitivity, and will set
> core.ignorecase if it does not. Might this repository have been created
> on a different filesystem, and then moved onto the case-insensitive
> filesystem?
> 
> Or might it have been created by something other than core git? I don't
> know whether one can create a repo in TortoiseGit, or if so how it does
> so.

It may have been created through the Visual Studio source provider for
Git, which is configured to use TortoiseGit which in turn uses msysGit.
But I have not written any of those programmes so I cannot guarantee for
what they do.

> In any case, try doing:
> 
>   git config core.ignorecase true
> 
> and see if that clears up your problems.

'git config core.ignorecase' now outputs "true" but I can still commit
the same file (modified) twice. So this doesn't help.

Meanwhile I have browsed my other code projects and found that the
designer-generated file name is sometimes with a "D" and sometimes with
a "d", so it's usually inconsistent. I haven't figured out where that
comes from but it means that this is a common issue that Git needs to
handle well to be usable on Windows. But we're not there yet.

-- 
Yves Goergen "LonelyPixel" <nospam.list@unclassified.de>
Visit my web laboratory at http://beta.unclassified.de

^ permalink raw reply

* Re: [PATCH] merge: Make merge strategy message follow the diffstat
From: Miles Bader @ 2012-01-17  8:03 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Nguyen Thai Ngoc Duy, Junio C Hamano, Paul Gortmaker, Mark Brown,
	Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <CA+55aFxw_-0h1FDmPRVif3LM03Qh3-6haA7=KYbae8pSFbpW2w@mail.gmail.com>

So ... "--shortish-diffthingy"

-miles

-- 
Zeal, n. A certain nervous disorder afflicting the young and inexperienced.

^ permalink raw reply

* [PATCH] test-lib: add the test_pause convenience function
From: Jens Lehmann @ 2012-01-17  8:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Git Mailing List, Pete Wyckoff
In-Reply-To: <7vk44ruupe.fsf@alter.siamese.dyndns.org>

Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a
"bash &&" into a test for debugging purposes anymore. Instead you'll have
to use "bash <&6 >&3 2>&4".

As that invocation is not that easy to remember add the test_pause
convenience function. This function also checks if the -v flag is given
and will error out if that is not the case instead of letting the test
hang until ^D is pressed.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 16.01.2012 23:51, schrieb Junio C Hamano:
> Jeff King <peff@peff.net> writes:
> 
>> Nice. Many times I have added such a "bash" or "gdb" invocation then
>> forgotten "-v", only to scratch my head at why the test seemed to be
>> hanging.
>>
>> Two minor nits on the patch itself:
>> ...
>> 1. It may be worth putting a warning in the comment that this is never
>>    to be used in a real test, but only temporarily inserted.
>>
>> 2. I do this not just with bash, but with "gdb". I wonder if it is worth
>>    making this "test_foo bash", for some value of "foo" (the ones that
>>    occur to me are "debug" and "run", but of course they are taken).
>>
>>    Actually, I wonder if the existing test_debug could handle this
>>    already (though you do have to remember to add "--debug" to your
>>    command line, then).
> 
> I wondered the same thing from a different angle. My first reaction was
> "Why is this called 'bash' not 'sh'?" which naturally led to the same
> direction as yours "why not an arbitrary command 'test_debug xxx'?"
> 
> test_pause perhaps?

I really don't care deeply about the name, so test_pause is absolutely
ok for me. I added some documentation in t/README too and made it an
error when --verbose is not used.

Is it ok to invoke bash here or should sh be used?


 t/README      |   13 +++++++++++++
 t/test-lib.sh |   13 +++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index c85abaf..c09c582 100644
--- a/t/README
+++ b/t/README
@@ -548,6 +548,19 @@ library for your script to use.
 		...
 	'

+ - test_pause
+
+	This command is useful for writing and debugging tests and must be
+	removed before submitting. It halts the execution of the test and
+	spawns a shell in the trash directory. Exit the shell to continue
+	the test. Example:
+
+	test_expect_success 'test' '
+		git do-something >actual &&
+		test_pause &&
+		test_cmp expected actual
+	'
+
 Prerequisites
 -------------

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a65dfc7..85084c4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -329,6 +329,19 @@ test_tick () {
 	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }

+# Stop execution and start a shell. This is useful for debugging tests and
+# only makes sense together with "-v".
+#
+# Be sure to remove all invocations of this command before submitting.
+
+test_pause () {
+	if test "$verbose" = t; then
+		bash <&6 >&3 2>&4
+	else
+		error >&5 "test_pause requires --verbose"
+	fi
+}
+
 # Call test_commit with the arguments "<message> [<file> [<contents>]]"
 #
 # This will commit a file with the given contents and the given commit
-- 
1.7.9.rc1.2.g0b847.dirty

^ permalink raw reply related

* Re: Bug? Git checkout fails with a wrong error message
From: Thomas Rast @ 2012-01-17  8:45 UTC (permalink / raw)
  To: Yves Goergen
  Cc: Holger Hellmuth, Jeff King, Carlos Martín Nieto, git,
	Erik Faye-Lund
In-Reply-To: <4F152767.9010104@unclassified.de>

Yves Goergen <nospam.list@unclassified.de> writes:

> On 16.01.2012 20:17 CE(S)T, Thomas Rast wrote:
>> If you work together with developers who have a case-sensitive FS (such
>> as Linux, or with the right options OS X), it's entirely possible that
>> this file exists in both spellings within the repository.
>
> Just FTR, I am working on the project alone, only on Windows with Visual
> Studio 2010 and I have two copies of the repository which I am
> occasionally synchronising via a USB memory stick when I work on the
> other machine. I have not pulled anything since the first issue came up
> on last Friday. No case-sensitive filesystem in the game.

Ok.

  $ git ls-tree -r HEAD
  100644 blob 5369994b8f905514661ee58b396dec31f8575a4d    PosterWantsItCensored.Designer.cs
  100644 blob 5369994b8f905514661ee58b396dec31f8575a4d    PosterWantsItCensored.designer.cs
  ^      ^    ^
  |      |    content hash
  |      type
  file mode

That tells us that you have identical file contents in two files whose
names differ only in case.

This is important: Different name for git.  Same name for OS.  Same
contents.

That should also settle your remark at the end:

> I find it interesting to see that both files with the equal file name
> (like what the only relevant file system considers equal) have the same
> hash value. Does that qualify for your description of the "pretty bad bug"?

No, not a bug, just the same contents.

[And before you sue me for disclosing the SHA1 above: inferring the
contents of the file from the SHA1 is equivalent to breaking SHA1.  If
anyone could, he'd already be busy writing a paper about it (or perhaps
working for the NSA).]

>> * You have the byte-for-byte identical file name listed twice in the
>>   index.  That would be a pretty bad bug.
>
> The index should usually be empty here, I guess. I really do not use
> it.  No index interaction at all.

Please read up on the index before making such statements.  You do use
the index, because it is a very important part of how git operates
whenever the operation also involves the worktree.  And except in border
cases (new empty repo etc.) it should never be empty.

Your paste of

  $ git status -s
  [no output]

tells us that the index has *no differences* to your worktree, nor to
HEAD.


So in summary, the picture in your repository is:

* Somehow you got a different-only-in-case file pair into your
  repository.  It's already in HEAD.  See below.

* The index and worktree are healthy and unchanged (w.r.t. HEAD) from
  Git's POV.  (This is possible despite the different-only-in-case files
  because they have the same contents.)

For now I'm siding with Erik's theory

Erik Faye-Lund wrote:
} Very speculative comment: This might be a bug in TortoiseGit. Looking
} at the sources, it seems they are using libgit2 to mess around with
} the index; perhaps it's case-sensitivity code isn't as well tested as
} Git for Windows'?

It would also be interesting to know for how long this problem has
existed.  You can search for the offending commit with something like

  git log --name-status --diff-filter=A -- "PosterWantsItCensored.*"

which should normally give you just one or two commits, namely the
one(s) that introduced the two files.

As for the fix, there are two-and-two-thirds cases.  First I'd like to
point out, however, that I have no idea how core.ignoreCase interacts
with rm --cached.  I'm assuming you have to set it to 'false' for the
recipes below to work.  Erik or Peff may correct me.  You should set it
to 'true' again for real work.

Case 1: The commit that introduced the second spelling is HEAD

  In this case you're sort of lucky because it's easy to fix.  You can
  do

    git rm --cached PosterWantsItCensored.designer.cs

  to get rid of the spelling you do not want.  Then run

    git status -s

  again to verify that it did the right thing; it should say

    D  PosterWantsItCensored.designer.cs

  where it's important that a) the other spelling does *not* show up in
  the list anywhere and b) the D is in the leftmost column.  Once you
  have verified this, run

    git commit --amend

  to fix HEAD.

Case 2a: The commit that introduced it is older, but you don't care if
         you cannot sanely checkout old commits

  This is the case that I personally would never choose, since I care
  about history, but for completeness: proceed as for case 1, except at
  the end run

    git commit  # no --amend

  and write a nice message saying that you fixed the
  different-only-in-case issue.

Case 2b: The commit that introduced it is older, but history since its
         parent has been linear (use gitk or some such to establish
         this)

  First run

    git log --full-history --oneline -- "PosterWantsItCensored.*"

  to see which commits touched the file.  Let C be the SHA1 (or a unique
  prefix) of the earliest commit that contains
  PosterWantsItCensored.designer.cs (i.e., the wrong spelling) as
  established earlier.  Then run

    git rebase -i C^

  (That's right, the SHA1 of C and then a hat.)

  In the editor that pops up, change 'pick' to 'edit' on every line that
  shows a SHA1 you found in the preceding git-log command.  Save and
  exit.

  Whenever rebase stops to let you edit (you can tell by the advice
  messages it gives you), run

    git ls-tree HEAD -- PosterWantsItCensored.designer.cs PosterWantsItCensored.Designer.cs

  and check whether the SHA1s are different.  Judging by what you said
  they should always be the same (otherwise please come back for more
  advice).  You can then again do something very similar to Case 1 to
  the commit you're editing, like

    git rm --cached PosterWantsItCensored.designer.cs
    git commit --amend

  and finally

    git rebase --continue

  to edit the next commit.  Repeat until the rebase is complete.

Case 2c: History wasn't linear since C; or you're just lazy and have a
         good backup

  The all-safeties-off, please-fix-it-for-me version goes

    git filter-branch --tag-name-filter cat --index-filter '
      git rm --ignore-unmatch --cached PosterWantsItCensored.designer.cs
    ' -- --all

  I'm dead serious about the safeties off.  You have been warned.

I have not tested most of this because it would simply take even more
time than writing an essay-length email.  If something fails or got you
confused, paste everything you did and the full output again so we can
establish what happened.

All sub-items of case 2 rewrite history.  You will have to force the
push to your "hub" repository that you use to exchange history, and you
may have to reset or rebase in the other repository.  Read e.g. the
'recovering from upstream rebase' section in man git-rebase.

> (What a mess it would be if I committed something different than my
> working directory, however that works.)

You should really read up on this, e.g.

  http://tomayko.com/writings/the-thing-about-git

AFAIK everyone who groks the feature uses it daily.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* git-grep while excluding files in a blacklist
From: Dov Grobgeld @ 2012-01-17  9:14 UTC (permalink / raw)
  To: git

Does git-grep allow searching for a pattern in all files *except*
files matching a pattern. E.g. in our project we have multiple DLL's
in git, but when searching I would like to exclude these for speed. Is
that possible with git-grep?

Thanks,
Dov

^ 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