Git development
 help / color / mirror / Atom feed
* Re: [RFCv3 2/4] Add Python support library for CVS remote helper
From: Johan Herland @ 2009-08-13  0:20 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: David Aguilar, git, barkalow, gitster, Johannes.Schindelin,
	Sverre Rabbelier
In-Reply-To: <4A83579E.70302@alum.mit.edu>

On Thursday 13 August 2009, Michael Haggerty wrote:
> Johan Herland wrote:
> > On Wednesday 12 August 2009, David Aguilar wrote:
> >> On Wed, Aug 12, 2009 at 02:13:49AM +0200, Johan Herland wrote:
> >>> +	def __init__ (self, date, author, message):
> >>> +		self.revs    = {}      # dict: path -> CvsRev object
> >>> +		self.date    = date    # CvsDate object
> >>> +		self.author  = author
> >>> +		self.message = message # Lines of commit message
> >>
> >> pep8 and other parts of the git codebase recommend against
> >> lining up the equals signs like that.  Ya, sorry for the nits
> >> being that they're purely stylistic.
> >
> > I can't find a good rationale for this rule in PEP8 (other than Guido's
> > personal style), and I personally find the above much more readable
> > (otherwise I wouldn't go through the trouble of lining them all up...).
> > Can I claim exception (1) (readability)?
>
> I think you are missing the point.  It may be true that the rules in
> PEP8 were *originally* written according to the unjustified whims of the
> BDFL, but now that they are established the reason for following them is
> not that Guido likes them but rather to be consistent with the bulk of
> other Python code on the planet.

Ok. I will try to follow PEP8 as closely as possible.

> With respect to the rule to use 4-space indents, there are serious
> practical problems with using tabs *in addition to* the consistency
> argument.

There are? What arguments? Assuming I don't mix spaces and tabs (which I 
certainly don't), I can't see any "practical problems" with using tabs 
(except for the PEP8/consistency issue).


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: GCC Git mirror no longer updating
From: Bernie Innocenti @ 2009-08-13  0:28 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Daniel Berlin, Frank Ch. Eigler, overseers, git
In-Reply-To: <4A82C786.5060602@redhat.com>

El Wed, 12-08-2009 a las 09:45 -0400, Jason Merrill escribió:
> On 08/12/2009 06:56 AM, Bernie Innocenti wrote:
> > The git repository format should support concurrent access, but perhaps
> > it only applies to git-receive-pack, not fancy operations such as
> > repacking.
> 
> The git repository format, yes, but maybe not the stuff in .git/svn.  It 
> seems like a temporary index file was referring to an object that got 
> garbage collected away.  Or maybe the index file was left over from the 
> initial import, and not there due to a collision; there don't seem to be 
> index files there normally.

git-svn might be keeping extra information in files that the other git
tools don't know about.  This would explain why some objects looked
like orphans and were thus culled.  [cc'ing the git list to catch the
attention of the git-svn maintainer(s)].

Ah, and I just fixed a problem I have introduced myself while fiddling
to recover the repository: HEAD should point at "refs/remotes/trunk",
otherwise new commits won't show up in gitweb.

-- 
   // Bernie Innocenti - http://codewiz.org/
 \X/  Sugar Labs       - http://sugarlabs.org/

^ permalink raw reply

* Re: [PATCH] svn: Add && to t9107-git-svn-migrarte.sh
From: Adam Brewster @ 2009-08-13  0:35 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20090812094940.GA22273@dcvr.yhbt.net>

Eric,

Thanks the help in getting this right.

> I think the following change on top of yours would make most
> sense:
>

Looks good to me.

> The map_path() changes you originally made didn't work, either, since
> the -f $1 never took GIT_DIR or GIT_DIR/svn into account.
>

It was ugly, too.

> I think the below is a good enough test case to for compatibility
> against existing repos.  Let me know what you think, thanks!
>

Looks about right.

Thanks,
Adam

^ permalink raw reply

* Re: [RFCv3 2/4] Add Python support library for CVS remote helper
From: Junio C Hamano @ 2009-08-13  0:55 UTC (permalink / raw)
  To: David Aguilar; +Cc: Johan Herland, git, barkalow, Johannes.Schindelin
In-Reply-To: <20090812021017.GB62301@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> This also had me wondering about the following:
> 	git uses tabs for indentation

Not relevant.  That is a rule for our "C" source code.  We also use it in
our Perl scripts and shell scripts because there is no single "one right
way" that is strongly defined and everybody adheres to, like the 4-space
rule Python folks have.

> BUT, the python convention is to use 4-space indents ala PEP-8
> http://www.python.org/dev/peps/pep-0008/
>
> It might be appealing to when-in-Rome (Rome being Python) here
> and do things the python way when we code in Python.

Yes, this is more important.

>> +		if len(msg) > 25: msg = msg[:22] + "..." # Max 25 chars long
>> +		return "<Changeset @(%s) by %s (%s) updating %i files>" % (
>> +			self.date, self.author, msg, len(self.revs))
>
> Similar to the git coding style, this might be better written:

So is this one.  If experienced Python folks also frown on single-line
conditionals, then by all means please update this.  But if this
suggestion is solely because we don't do a single-line conditional in our
C source code, then please do not insist on it too strongly.  The code
should look familiar to Pythonistas with good tastes (if such a class of
people exist, that is ;-)).

^ permalink raw reply

* Re: [RFCv3 2/4] Add Python support library for CVS remote helper
From: Johan Herland @ 2009-08-13  1:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Aguilar, git, barkalow, Johannes.Schindelin
In-Reply-To: <7vzla4r0qc.fsf@alter.siamese.dyndns.org>

On Thursday 13 August 2009, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> > This also had me wondering about the following:
> > 	git uses tabs for indentation
>
> Not relevant.  That is a rule for our "C" source code.  We also use it in
> our Perl scripts and shell scripts because there is no single "one right
> way" that is strongly defined and everybody adheres to, like the 4-space
> rule Python folks have.
>
> > BUT, the python convention is to use 4-space indents ala PEP-8
> > http://www.python.org/dev/peps/pep-0008/
> >
> > It might be appealing to when-in-Rome (Rome being Python) here
> > and do things the python way when we code in Python.
>
> Yes, this is more important.
>
> >> +		if len(msg) > 25: msg = msg[:22] + "..." # Max 25 chars long
> >> +		return "<Changeset @(%s) by %s (%s) updating %i files>" % (
> >> +			self.date, self.author, msg, len(self.revs))
> >
> > Similar to the git coding style, this might be better written:
>
> So is this one.  If experienced Python folks also frown on single-line
> conditionals, then by all means please update this.  But if this
> suggestion is solely because we don't do a single-line conditional in our
> C source code, then please do not insist on it too strongly.  The code
> should look familiar to Pythonistas with good tastes (if such a class of
> people exist, that is ;-)).

Ok. Thanks. I will follow PEP8 as closely as possible, including the 4-space 
indent.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* What's cooking in git.git (Aug 2009, #02; Wed, 12)
From: Junio C Hamano @ 2009-08-13  2:14 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 ones
marked with '.' do not appear in any of the branches, but I am still
holding onto them.

After the 1.6.5 cycle, the next release will be 1.7.0, and we will push
out the planned "push safety" change.  1.7.0 would be a good time to
introduce "justifiable" changes that are not strictly backward compatible.

During 1.6.5 cycle, 'next' will hold topics meant for 1.6.5 and 1.7.0.

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

* ld/p4 (Thu Jul 30 00:13:46 2009 +0100) 1 commit
 - git-p4: stream from perforce to speed up clones

Waiting for Ack/Nack from stakeholders.  Probably I should merge this to
'next' soon, keep it there for a week and see if anybody screams, aka
"Nack now or forever hold your peace" ;-).

* mr/gitweb-xz (Thu Aug 6 10:28:25 2009 -0400) 3 commits
 - gitweb: add support for XZ compressed snapshots
 - gitweb: update INSTALL regarding specific snapshot settings
 - gitweb: support to globally disable a snapshot format

This should be safe as long as it does not silently enable itself for
folks who merely updated gitweb to this version without explicitly asking
for the new format (but I do not remember if that is what the patch does).

* jh/cvs-helper (Wed Aug 12 02:13:51 2009 +0200) 4 commits
 - Add simple selftests of git-remote-cvs functionality
 - Third draft of CVS remote helper program
 - Add Python support library for CVS remote helper
 - Basic build infrastructure for Python scripts

Builds on db/vcs-helper.  The last round failed its own selftest for me,
but this one seems to be Ok.  Documentation/git-remote-cvs.txt needs to be
fixed to build, though.

* bc/maint-am-email (Thu Aug 6 20:08:13 2009 -0500) 2 commits
 + git-am: print fair error message when format detection fails
 + am: allow individual e-mail files as input

It seems that the "not mbox but a single piece of e-mail" format was
something many people relied on.  I'm cooking this in 'next', and
hopefully it can graduate both to 'master' and 'maint'.

* js/maint-cover-letter-non-ascii (Mon Aug 10 18:22:22 2009 +0200) 2 commits
 + Correctly mark cover letters' encodings if they are not pure ASCII
 + Expose the has_non_ascii() function

* jc/verify-pack-stat (Fri Aug 7 15:45:30 2009 -0700) 2 commits
 + verify-pack --stat-only: show histogram without verifying
 + Merge branch 'maint' into jc/verify-pack-stat

* lt/block-sha1 (Wed Aug 12 15:47:55 2009 -0400) 15 commits
 + block-sha1: support for architectures with memory alignment
   restrictions
 + block-sha1: split the different "hacks" to be individually
   selected
 + block-sha1: move code around
 + block-sha1: improve code on large-register-set machines
 + block-sha1: improved SHA1 hashing
 + block-sha1: perform register rotation using cpp
 + block-sha1: get rid of redundant 'lenW' context
 + block-sha1: Use '(B&C)+(D&(B^C))' instead of '(B&C)|(D&(B|C))' in
   round 3
 + block-sha1: macroize the rounds a bit further
 + block-sha1: re-use the temporary array as we calculate the SHA1
 + block-sha1: make the 'ntohl()' part of the first SHA1 loop
 + block-sha1: minor fixups
 + block-sha1: try to use rol/ror appropriately
 + block-sha1: undo ctx->size change
 + Add new optimized C 'block-sha1' routines

Linus's "written in C, faster than handcrafted asm" SHA-1 implementation
with clean-up and ARM support from Nico.

* nd/sparse (Tue Aug 11 22:44:06 2009 +0700) 8 commits
 . --sparse for porcelains
 . Support sparse checkout in unpack_trees() and read-tree
 . unpack-trees.c: generalize verify_* functions
 . dir.c: export excluded_1() and add_excludes_from_file_1()
 . excluded_1(): support exclude "directories" in index
 . Read .gitignore from index if it is assume-unchanged
 . Avoid writing to buffer in add_excludes_from_file_1()
 . Prevent diff machinery from examining assume-unchanged entries on
   worktree

Privately applied but not yet queued to 'pu', expecting a minor reroll
near the tip.

* jk/maint-merge-msg-fix (Sun Aug 9 06:02:51 2009 -0400) 3 commits
 + merge: indicate remote tracking branches in merge message
 + merge: fix incorrect merge message for ambiguous tag/branch
 + add tests for merge message headings

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

* zf/maint-gitweb-acname (Sun Aug 2 09:42:24 2009 +0200) 1 commit
 - gitweb: parse_commit_text encoding fix

* jc/maint-merge-recursive-fix (Thu Jul 30 17:38:15 2009 -0700) 1 commit
 - merge-recursive: don't segfault while handling rename clashes

* js/run-command-updates (Tue Aug 4 11:28:40 2009 +0200) 7 commits
 + run-command.c: squelch a "use before assignment" warning
 + receive-pack: remove unnecessary run_status report
 + run_command: report failure to execute the program, but optionally
   don't
 + run_command: encode deadly signal number in the return value
 + run_command: report system call errors instead of returning error
   codes
 + run_command: return exit code as positive value
 + MinGW: simplify waitpid() emulation macros

* mk/grep-max-depth (Wed Jul 22 19:52:15 2009 +0200) 1 commit
 + grep: Add --max-depth option.

* jp/symlink-dirs (Wed Jul 29 20:22:25 2009 -0700) 3 commits
 + git-checkout: be careful about untracked symlinks
 + lstat_cache: guard against full match of length of 'name'
   parameter
 + Demonstrate bugs when a directory is replaced with a symlink

----------------------------------------------------------------
[In 'next']

* bc/mailsplit-cr-at-eol (Tue Aug 4 22:31:59 2009 -0500) 4 commits
 + Allow mailsplit (and hence git-am) to handle mails with CRLF line-
   endings
 + builtin-mailsplit.c: remove read_line_with_nul() since it is no
   longer used
 + builtin-mailinfo,builtin-mailsplit: use strbufs
 + strbuf: add new function strbuf_getwholeline()

* gb/apply-ignore-whitespace (Tue Aug 4 13:16:49 2009 +0200) 1 commit
 + git apply: option to ignore whitespace differences

* cc/replace (Wed May 27 07:14:09 2009 +0200) 14 commits
 + t6050: check pushing something based on a replaced commit
 + Documentation: add documentation for "git replace"
 + Add git-replace to .gitignore
 + builtin-replace: use "usage_msg_opt" to give better error messages
 + parse-options: add new function "usage_msg_opt"
 + builtin-replace: teach "git replace" to actually replace
 + Add new "git replace" command
 + environment: add global variable to disable replacement
 + mktag: call "check_sha1_signature" with the replacement sha1
 + replace_object: add a test case
 + object: call "check_sha1_signature" with the replacement sha1
 + sha1_file: add a "read_sha1_file_repl" function
 + replace_object: add mechanism to replace objects found in
   "refs/replace/"
 + refs: add a "for_each_replace_ref" function

* jc/1.7.0-diff-whitespace-only-status (Sat May 23 01:15:35 2009 -0700) 2 commits
 + diff: Rename QUIET internal option to QUICK
 + diff: change semantics of "ignore whitespace" options

This changes exit code from "git diff --ignore-whitespace" and friends
when there is no actual output.  It is a backward incompatible change, but
we could argue that it is a bugfix.

* jc/1.7.0-push-safety (Mon Feb 9 00:19:46 2009 -0800) 2 commits
 + Refuse deleting the current branch via push
 + Refuse updating the current branch in a non-bare repository via
   push

----------------------------------------------------------------
[In 'pu']

* jn/gitweb-blame (Thu Aug 6 19:11:52 2009 +0200) 14 commits
 - gitweb: Create links leading to 'blame_incremental' using
   JavaScript
 - gitweb: Incremental blame (proof of concept)
 - gitweb: Add optional "time to generate page" info in footer
 + Revert the four topmost commits from jn/gitweb-blame topic
 + gitweb: Create links leading to 'blame_incremental' using
   JavaScript
 + gitweb: Incremental blame (proof of concept)
 + gitweb: Add optional "time to generate page" info in footer
 + gitweb: Add -partial_query option to href() subroutine
 + gitweb: Use light/dark for class names also in 'blame' view
 + gitweb: Add author initials in 'blame' view, a la "git gui blame"
 + gitweb: Mark commits with no "previous" in 'blame' view
 + gitweb: Use "previous" header of git-blame -p in 'blame' view
 + gitweb: Mark boundary commits in 'blame' view
 + gitweb: Make .error style generic

I haven't picked up Jakub's replacement to the second one from the tip.
We probably should merge up to "Use light/dark" (aef3768) to 'master' and
start the rest over.

* jc/maint-clean-nested-dir-safety (Tue Jun 30 15:33:45 2009 -0700) 1 commit
 + clean: require double -f options to nuke nested git repository and
   work tree

This probably should go in 'master' soonish.

* jc/shortstatus (Tue Aug 4 23:55:22 2009 -0700) 12 commits
 - git stat -s: short status output
 - git stat: pathspec limits, unlike traditional "git status"
 - git stat: the beginning
 + wt-status: collect untracked files in a separate "collect" phase
 + Make git_status_config() file scope static to builtin-commit.c
 + wt-status: move wt_status_colors[] into wt_status structure
 + wt-status: move many global settings to wt_status structure
 + commit: --dry-run
 + status: show worktree status of conflicted paths separately
 + wt-status.c: rework the way changes to the index and work tree are
   summarized
 + diff-index: keep the original index intact
 + diff-index: report unmerged new entries

Slowly progressing.  I've addressed issues pointed out by Jeff in his
review, and hopefully the whole thing would be ready to go.

* db/vcs-helper (Sun Aug 9 15:28:28 2009 -0400) 17 commits
 - Allow helpers to request marks for fast-import
 - Allow helpers to report in "list" command that the ref is
   unchanged
 - Add support for "import" helper command
 - transport-helper_init(): fix a memory leak in error path
 - Add a config option for remotes to specify a foreign vcs
 - Allow programs to not depend on remotes having urls
 - Allow fetch to modify refs
 - Use a function to determine whether a remote is valid
 - Use a clearer style to issue commands to remote helpers
 + Makefile: install hardlinks for git-remote-<scheme> supported by
   libcurl if possible
 + Makefile: do not link three copies of git-remote-* programs
 + Makefile: git-http-fetch does not need expat
 + http-fetch: Fix Makefile dependancies
 + Add transport native helper executables to .gitignore
 + git-http-fetch: not a builtin
 + Use an external program to implement fetching with curl
 + Add support for external programs for handling native fetches

This consolidates various previous rounds from Daniel and Johan.  It seems
that the use of colon ':' before vcs helper name needs to be corrected
before this series can go further.

* je/send-email-no-subject (Wed Aug 5 18:49:54 2009 +0200) 1 commit
 - send-email: confirm on empty mail subjects

This seems to break t9001.  Near the tip of 'pu' I have a iffy workaround.

* jh/notes (Wed Jul 29 04:25:26 2009 +0200) 8 commits
 - t3302-notes-index-expensive: Speed up create_repo()
 - fast-import: Add support for importing commit notes
 - First draft of notes tree parser with support for fanout subtrees
 - Teach "-m <msg>" and "-F <file>" to "git notes edit"
 - Add an expensive test for git-notes
 - Speed up git notes lookup
 - Add a script to edit/inspect notes
 - Introduce commit notes

* cc/sequencer-rebase-i (Wed Aug 5 22:53:00 2009 +0200) 8 commits
 - rebase -i: use "git sequencer--helper --reset-hard"
 - sequencer: add "--reset-hard" option to "git sequencer--helper"
 - sequencer: add comments about reset_almost_hard()
 - sequencer: add "reset_almost_hard()" and related functions
 - rebase -i: use "git sequencer--helper --make-patch"
 - sequencer: free memory used in "make_patch" function
 - sequencer: add "make_patch" function to save a patch
 - sequencer: add "builtin-sequencer--helper.c"

More sequencer updates.  I didn't look at the latest round that had a
handful "oops, fix that earlier botch" patches, expecting a cleaner
reroll.

* jc/mailinfo-remove-brackets (Wed Jul 15 15:31:12 2009 -0700) 1 commit
 - mailinfo: -b option keeps [bracketed] strings that is not a
   [PATCH] marker

* tr/reset-checkout-patch (Tue Jul 28 23:20:12 2009 +0200) 8 commits
 - DWIM 'git stash save -p' for 'git stash -p'
 - Merge branch 'js/stash-dwim' into tr/reset-checkout-patch
 - Make 'git stash -k' a short form for 'git stash save --keep-index'
 - Implement 'git stash save --patch'
 - Implement 'git checkout --patch'
 - Implement 'git reset --patch'
 - builtin-add: refactor the meat of interactive_add()
 - git-apply--interactive: Refactor patch mode code

Progress?

* js/stash-dwim (Mon Jul 27 20:37:10 2009 +0200) 1 commit
 - Make 'git stash -k' a short form for 'git stash save --keep-index'

This should be merged to 'next' soonish, but I keep forgetting.

* pb/tracking (Thu Jul 16 16:26:15 2009 -0500) 7 commits
 . branch.c: if remote is not config'd for branch, don't try delete
   push config
 . branch, checkout: introduce autosetuppush
 . move deletion of merge configuration to branch.c
 . remote: add per-remote autosetupmerge and autosetuprebase
   configuration
 . introduce a struct tracking_config
 . branch: install_branch_config and struct tracking refactoring
 . config: allow false and true values for branch.autosetuprebase

Has been ejected from 'pu' for some time, expecting a reroll.

* jc/log-tz (Tue Mar 3 00:45:37 2009 -0800) 1 commit
 - Allow --date=local --date=other-format to work as expected

^ permalink raw reply

* Re: fatal: bad revision 'HEAD'
From: Jeff King @ 2009-08-13  2:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <7vab24ve97.fsf@alter.siamese.dyndns.org>

On Wed, Aug 12, 2009 at 03:49:56PM -0700, Junio C Hamano wrote:

> When doing a "pull --rebase", we check to make sure that the index and
> working tree are clean. The index-clean check compares the index against
> HEAD. The test erroneously reports dirtiness if we don't have a HEAD yet.
> 
> In such an "unborn branch" case, by definition, a non-empty index won't
> be based on whatever we are pulling down from the remote, and will lose
> the local change.  Just check if $GIT_DIR/index exists and error out.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Looks sane to me, but see below.

> +		# On an unborn branch
> +		if test -f "$GIT_DIR/index"
> +		then
> +			die "updating an unborn branch with changes added to the index"
> +		fi

I had a brief moment where I thought this might not actually be
sufficient. That is, can somebody create an unborn branch in an existing
repo (e.g., because they want an alternate project root) in such a way
that they have an index, but it is empty? In which case we should be
actually checking for index emptiness instead of existence.

I.e., I have done in the past (but not frequently):

  git symbolic-ref HEAD refs/heads/to-be-born

in an existing repository to create a new root. But I don't think we
really need to worry about that case:

  1. It is somewhat crazy and rare in the first place. Even more crazy
     and rare would be to "git pull --rebase" immediately afterwards.

  2. If you _did_ have an index, either:

     2a. You want to keep it, in which case your check is sane.

     2b. You want to start fresh, in which case your next step would be
         be "rm -f .git/index".

The only way this check would not be OK is is if you did something like:

  git symbolic-ref HEAD refs/heads/new-branch
  git ls-files | xargs git rm --cached
  git pull --rebase ...

which just seems crazy.

So I think I have convinced myself that what you have is fine, but I
just wanted a sanity check.

-Peff

^ permalink raw reply

* [RFC] Store subtree entries in the same hash map as the note entries
From: Johan Herland @ 2009-08-13  3:00 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, gitster, trast, tavestbo, git, chriscool, spearce
In-Reply-To: <200908010436.57480.johan@herland.net>

This is a first draft at implementing Dscho's idea of improving note parser
performance by storing the subtree entries in the same hash map as the note
entries.

In order to tell subtree entries and note entries apart, another member has
been added to struct entry: unsigned char commit_sha1_len. This member
stores the number of "valid" bytes in the commit_sha1 member, meaning that
a value of 1-19 indicates a subtree entry, and a value of 20 indicates a
note entry. There are two more special values for this new member as well:

- Since a null SHA1 is also a valid subtree entry (e.g. the "00/*" subtree
  in a 2/38 fanout scheme), we can no longer use is_null_sha1() to identify
  unused entries in the hash map. Instead, a value of 0 in the new
  commit_sha1_len member indicates that this is entry is null/unused.

  There is one exception to this rule: the root notes tree which is just a
  special case of a subtree entry with 0 "valid" bytes in the commit_sha1
  member. However, this exception is acceptable, as the root entry is never
  stored in the hash map (the hash map is initialized by unpacking this
  root tree entry).

- The second special commit_sha1_len value is 255, and is used to indicate
  a subtree entry that has already been unpacked, and should therefore be
  removed from the hash map. It is expensive and non-trivial to _delete_
  an entry in the hash map, and we therefore use this special value to
  _ignore_ the entry. If/when the hash map is grown/reallocated, we simply
  avoid bringing the unpacked subtree entries into the new hash map.

$ ./test_performance.sh
Timing 100 reps of 'git log -n 10 refs/heads/master >/dev/null' at fanout level 0...
21.80user 2.15system 0:24.12elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+1051886minor)pagefaults 0swaps

Timing 100 reps of 'git log -n 10 refs/heads/master >/dev/null' at fanout level 1...
1.24user 0.24system 0:01.49elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+120478minor)pagefaults 0swaps

Timing 100 reps of 'git log -n 10 refs/heads/master >/dev/null' at fanout level 2...
0.73user 0.20system 0:00.95elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+105782minor)pagefaults 0swaps

Signed-off-by: Johan Herland <johan@herland.net>
---

On Wednesday 29 July 2009, Johannes Schindelin wrote:
> I know you want to have something working first and optimize then, but I
> imagined that the hashmap can actually contain the entries of the partial
> hashes, too.  You'll need to extend the data type, of course, to be able
> to say just how many digits of the SHA-1 are valid, and I guess for
> consistency you'll need to pad with 0s.

Ok. Here's a draft implementation of your proposal to store the subtree
entries in the same hash map as the note entries.

This code is somewhat faster than my first subtree-in-linked-list draft,
but considerably slower than my 256-tree proposal:

                    fanout level 0   fanout level 1   fanout level 2
  subtree-in-ll         32.71s            1.88s            0.65s
  all-in-hash-map       24.12s            1.49s            0.95s
  all-in-256-tree       20.39s            1.06s            0.63s

I'm not sure how closely my implementation follows your vision, so please
suggest improvements, fixes, etc.


Have fun! :)

...Johan


 notes.c |  155 +++++++++++++++++++++++++--------------------------------------
 1 files changed, 62 insertions(+), 93 deletions(-)

diff --git a/notes.c b/notes.c
index 7e9dc49..34b8892 100644
--- a/notes.c
+++ b/notes.c
@@ -9,6 +9,15 @@
 struct entry {
 	unsigned char commit_sha1[20];
 	unsigned char notes_sha1[20];
+	/*
+	 * The following member can have the following values:
+	 * 0    - This is a NULL/blank entry in the hash_map
+	 *        (Exception: The root_tree entry in initialize_hash_map())
+	 * 1-19 - This is a subtree with the given number of valid prefix bytes
+	 * 20   - This is a note
+	 * 255  - This is a subtree which has been unpacked, please ignore
+	 */
+	unsigned char commit_sha1_len;
 };
 
 struct hash_map {
@@ -16,43 +25,35 @@ struct hash_map {
 	off_t count, size;
 };
 
-struct subtree_entry {
-	/*
-	 * SHA1 prefix is stored in the first 19 bytes (w/trailing NUL bytes);
-	 * length of SHA1 prefix is stored in the last byte
-	 */
-	unsigned char sha1_prefix_w_len[20];
-	unsigned char subtree_sha1[20];
-	struct subtree_entry *next;
-};
-
 static int initialized;
 static struct hash_map hash_map;
-static struct subtree_entry *subtree_list;
 
 static int hash_index(struct hash_map *map, const unsigned char *sha1)
 {
 	int i = ((*(unsigned int *)sha1) % map->size);
 
 	for (;;) {
-		unsigned char *current = map->entries[i].commit_sha1;
+		struct entry *current = &(map->entries[i]);
 
-		if (!hashcmp(sha1, current))
-			return i;
-
-		if (is_null_sha1(current))
+		if (!current->commit_sha1_len)
 			return -1 - i;
 
+		if (!hashcmp(sha1, current->commit_sha1)
+		 && current->commit_sha1_len <= 20)
+			return i;
+
 		if (++i == map->size)
 			i = 0;
 	}
 }
 
 static void add_entry(const unsigned char *commit_sha1,
-		const unsigned char *notes_sha1)
+		const unsigned char *notes_sha1,
+		unsigned char commit_sha1_len)
 {
 	int index;
 
+	assert(commit_sha1_len > 0 && commit_sha1_len <= 20);
 	if (hash_map.count + 1 > hash_map.size >> 1) {
 		int i, old_size = hash_map.size;
 		struct entry *old = hash_map.entries;
@@ -61,13 +62,20 @@ static void add_entry(const unsigned char *commit_sha1,
 		hash_map.entries = (struct entry *)
 			xcalloc(sizeof(struct entry), hash_map.size);
 
-		for (i = 0; i < old_size; i++)
-			if (!is_null_sha1(old[i].commit_sha1)) {
+		for (i = 0; i < old_size; i++) {
+			switch (old[i].commit_sha1_len) {
+			case 255:
+				hash_map.count--;
+				/* fall through */
+			case 0:
+				continue;
+			default:
 				index = -1 - hash_index(&hash_map,
 						old[i].commit_sha1);
 				memcpy(hash_map.entries + index, old + i,
 					sizeof(struct entry));
 			}
+		}
 		free(old);
 	}
 
@@ -79,6 +87,7 @@ static void add_entry(const unsigned char *commit_sha1,
 
 	hashcpy(hash_map.entries[index].commit_sha1, commit_sha1);
 	hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
+	hash_map.entries[index].commit_sha1_len = commit_sha1_len;
 }
 
 /*
@@ -110,22 +119,26 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
 	return len;
 }
 
-static void load_subtree(struct subtree_entry *se)
+static void load_subtree(struct entry *subtree)
 {
 	unsigned char commit_sha1[20];
 	unsigned int prefix_len;
 	void *buf;
 	struct tree_desc desc;
 	struct name_entry entry;
-	struct subtree_entry *tmp_list = NULL, *tmp_last = NULL;
 
-	buf = fill_tree_descriptor(&desc, se->subtree_sha1);
+	buf = fill_tree_descriptor(&desc, subtree->notes_sha1);
 	if (!buf)
 		die("Could not read %s for notes-index",
-		     sha1_to_hex(se->subtree_sha1));
+		     sha1_to_hex(subtree->notes_sha1));
+
+	prefix_len = subtree->commit_sha1_len;
+	assert(prefix_len < 20);
+	memcpy(commit_sha1, subtree->commit_sha1, prefix_len);
+
+	/* Invalidate this subtree from further consideration */
+	subtree->commit_sha1_len = 255;
 
-	prefix_len = se->sha1_prefix_w_len[19];
-	memcpy(commit_sha1, se->sha1_prefix_w_len, prefix_len);
 	while (tree_entry(&desc, &entry)) {
 		int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
 				commit_sha1 + prefix_len, 20 - prefix_len);
@@ -133,110 +146,66 @@ static void load_subtree(struct subtree_entry *se)
 			continue; /* entry.path is not a SHA1 sum. Skip */
 		len += prefix_len;
 
-		/* If commit SHA1 is complete, assume note object */
-		if (len == 20)
-			add_entry(commit_sha1, entry.sha1);
-		/* If commit SHA1 is incomplete, assume note subtree */
-		else if (len < 20 && entry.mode == S_IFDIR) {
-			struct subtree_entry *n = (struct subtree_entry *)
-				xcalloc(sizeof(struct subtree_entry), 1);
-			hashcpy(n->sha1_prefix_w_len, commit_sha1);
-			n->sha1_prefix_w_len[19] = (unsigned char) len;
-			hashcpy(n->subtree_sha1, entry.sha1);
-
-			if (!tmp_list) {
-				tmp_list = n;
-				tmp_last = n;
-			}
-			else {
-				assert(!tmp_last->next);
-				assert(hashcmp(n->sha1_prefix_w_len,
-					tmp_last->sha1_prefix_w_len) > 0);
-				tmp_last->next = n;
-				tmp_last = n;
-			}
-		}
+		if (len == 20 || (len < 20 && entry.mode == S_IFDIR))
+			add_entry(commit_sha1, entry.sha1, len);
 	}
 	free(buf);
-	if (tmp_list) {
-		/* insert tmp_list immediately after se */
-		assert(hashcmp(tmp_list->sha1_prefix_w_len,
-				se->sha1_prefix_w_len) > 0);
-		if (se->next) {
-			assert(hashcmp(se->next->sha1_prefix_w_len,
-					tmp_last->sha1_prefix_w_len) > 0);
-			tmp_last->next = se->next;
-		}
-		se->next = tmp_list;
-	}
 }
 
 static void initialize_hash_map(const char *notes_ref_name)
 {
 	unsigned char sha1[20], commit_sha1[20];
 	unsigned mode;
-	struct subtree_entry root_tree;
+	struct entry root_tree;
 
 	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
 	    get_tree_entry(commit_sha1, "", sha1, &mode))
 		return;
 
-	hashclr(root_tree.sha1_prefix_w_len);
-	hashcpy(root_tree.subtree_sha1, sha1);
-	root_tree.next = NULL;
+	hashclr(root_tree.commit_sha1);
+	hashcpy(root_tree.notes_sha1, sha1);
+	root_tree.commit_sha1_len = 0;
 	load_subtree(&root_tree);
-	subtree_list = root_tree.next;
 }
 
-/*
- * Compare the given commit SHA1 against the given subtree entry.
- * Return -1 if the commit SHA1 cannot exist within the given subtree, or any
- * subtree following it.
- * Return 0 if the commit SHA1 _may_ exist within the given subtree.
- * Return 1 if the commit SHA1 cannot exist within the given subtree, but may
- * exist within a subtree following it.
- */
-static int commit_subtree_cmp(const unsigned char *commit_sha1,
-		const struct subtree_entry *entry)
+static struct entry *lookup_subtree(const unsigned char *commit_sha1)
 {
-	unsigned int prefix_len = entry->sha1_prefix_w_len[19];
-	return memcmp(commit_sha1, entry->sha1_prefix_w_len, prefix_len);
-}
+	unsigned char prefix_sha1[20];
+	unsigned char i;
+	int index;
 
-static struct subtree_entry *lookup_subtree(const unsigned char *commit_sha1)
-{
-	struct subtree_entry *found = NULL, *cur = subtree_list;
-	while (cur) {
-		int cmp = commit_subtree_cmp(commit_sha1, cur);
-		if (!cmp)
-			found = cur;
-		if (cmp < 0)
-			break;
-		cur = cur->next;
+	hashcpy(prefix_sha1, commit_sha1);
+	for (i = 19; i; --i) {
+		prefix_sha1[i] = 0;
+		index = hash_index(&hash_map, prefix_sha1);
+		if (index >= 0 && hash_map.entries[index].commit_sha1_len == i)
+			return &(hash_map.entries[index]);
 	}
-	return found;
+	return NULL;
 }
 
 static unsigned char *lookup_notes(const unsigned char *commit_sha1)
 {
 	int index;
-	struct subtree_entry *subtree;
+	struct entry *subtree;
+
+	if (!hash_map.size)
+		return NULL;
 
 	/* First, try to find the commit SHA1 directly in hash map */
-	index = hash_map.size ? hash_index(&hash_map, commit_sha1) : -1;
+	index = hash_index(&hash_map, commit_sha1);
 	if (index >= 0)
 		return hash_map.entries[index].notes_sha1;
 
 	/* Next, try finding a subtree that may contain the commit SHA1 */
 	subtree = lookup_subtree(commit_sha1);
 
-	/* Give up if no subtree found, or if subtree is already loaded */
-	if (!subtree || is_null_sha1(subtree->subtree_sha1))
+	/* Give up if no subtree found */
+	if (!subtree)
 		return NULL;
 
 	/* Load subtree into hash_map, and retry lookup recursively */
 	load_subtree(subtree);
-	hashclr(subtree->subtree_sha1);
 	return lookup_notes(commit_sha1);
 }
 
-- 
1.6.4.rc3.138.ga6b98.dirty

^ permalink raw reply related

* [PATCH 3/5] Document --ignore-unmatch in git-add.txt
From: Luke Dashjr @ 2009-08-13  3:20 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <1250133624-2272-2-git-send-email-luke-jr+git@utopios.org>

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 Documentation/git-add.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index ab1943c..6b93b4e 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
-	  [--refresh] [--ignore-errors] [--] <filepattern>...
+	  [--refresh] [--ignore-errors] [--ignore-unmatch] [--]
+	  <filepattern>...
 
 DESCRIPTION
 -----------
@@ -119,6 +120,9 @@ apply.
 	them, do not abort the operation, but continue adding the
 	others. The command shall still exit with non-zero status.
 
+--ignore-unmatch::
+	Exit with a zero status even if no files matched.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 1/5] port --ignore-unmatch to "git add"
From: Luke Dashjr @ 2009-08-13  3:20 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <200908121726.52121.luke-jr@utopios.org>

"git rm" has a --ignore-unmatch option that is also applicable to "git add"
and may be useful for persons wanting to ignore unmatched arguments, but not
all errors.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 builtin-add.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..0597fb9 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,6 +19,7 @@ static const char * const builtin_add_usage[] = {
 };
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
+static int ignore_unmatch = 0;
 
 static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 {
@@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 	fill_pathspec_matches(pathspec, seen, specs);
 
 	for (i = 0; i < specs; i++) {
-		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
+		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) && !ignore_unmatch)
 			die("pathspec '%s' did not match any files",
 					pathspec[i]);
 	}
@@ -108,7 +109,7 @@ static void refresh(int verbose, const char **pathspec)
 	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
 		      pathspec, seen);
 	for (i = 0; i < specs; i++) {
-		if (!seen[i])
+		if (!seen[i] && !ignore_unmatch)
 			die("pathspec '%s' did not match any files", pathspec[i]);
 	}
         free(seen);
@@ -226,6 +227,8 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
+	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
+				"exit with a zero status even if nothing matched"),
 	OPT_END(),
 };
 
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 4/5] implement error_errno and warning_errno
From: Luke Dashjr @ 2009-08-13  3:20 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <1250133624-2272-3-git-send-email-luke-jr+git@utopios.org>

This allows for easier conversion of code that currently does a (fatal)
die_errno to a safer error (which can be ignored), or perhaps even warning
status.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 git-compat-util.h |    2 +
 usage.c           |   66 ++++++++++++++++++++++++-----------------------------
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 9f941e4..25d4c1e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -181,7 +181,9 @@ extern void usage(const char *err) NORETURN;
 extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 extern void die_errno(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
 extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
 
diff --git a/usage.c b/usage.c
index b6aea45..dce90dc 100644
--- a/usage.c
+++ b/usage.c
@@ -39,7 +39,7 @@ static void warn_builtin(const char *warn, va_list params)
 static void (*usage_routine)(const char *err) NORETURN = usage_builtin;
 static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;
 static void (*error_routine)(const char *err, va_list params) = error_builtin;
-static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
+static void (*warning_routine)(const char *err, va_list params) = warn_builtin;
 
 void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN)
 {
@@ -51,19 +51,8 @@ void usage(const char *err)
 	usage_routine(err);
 }
 
-void die(const char *err, ...)
+static void _e_errno(const char *fmt, char *fmt_with_err, size_t sizeof_fmt_with_err)
 {
-	va_list params;
-
-	va_start(params, err);
-	die_routine(err, params);
-	va_end(params);
-}
-
-void die_errno(const char *fmt, ...)
-{
-	va_list params;
-	char fmt_with_err[1024];
 	char str_error[256], *err;
 	int i, j;
 
@@ -81,28 +70,33 @@ void die_errno(const char *fmt, ...)
 		}
 	}
 	str_error[j] = 0;
-	snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, str_error);
-
-	va_start(params, fmt);
-	die_routine(fmt_with_err, params);
-	va_end(params);
-}
-
-int error(const char *err, ...)
-{
-	va_list params;
-
-	va_start(params, err);
-	error_routine(err, params);
-	va_end(params);
-	return -1;
+	snprintf(fmt_with_err, sizeof_fmt_with_err, "%s: %s", fmt, str_error);
 }
 
-void warning(const char *warn, ...)
-{
-	va_list params;
-
-	va_start(params, warn);
-	warn_routine(warn, params);
-	va_end(params);
-}
+#define BUILD_E(RTYPE, NAME, CODE)  \
+RTYPE NAME(const char *err, ...)  \
+{  \
+	va_list params;  \
+  \
+	va_start(params, err);  \
+	NAME ## _routine(err, params);  \
+	va_end(params);  \
+	CODE  \
+}  \
+  \
+RTYPE NAME ## _errno(const char *fmt, ...)  \
+{  \
+	va_list params;  \
+	static char fmt_with_err[1024];  \
+  \
+	va_start(params, fmt);  \
+	_e_errno(fmt, fmt_with_err, sizeof(fmt_with_err));  \
+	NAME ## _routine(fmt_with_err, params);  \
+	va_end(params);  \
+	CODE  \
+}  \
+// END OF BUILD_E MACRO
+
+BUILD_E(void, die, )
+BUILD_E(int, error, return -1;)
+BUILD_E(void, warning, )
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors
From: Luke Dashjr @ 2009-08-13  3:20 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <1250133624-2272-1-git-send-email-luke-jr+git@utopios.org>

Unmatched files are errors, and should be ignored with the rest of them.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 builtin-add.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 0597fb9..e3132c8 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -280,6 +280,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		add_interactive = 1;
 	if (add_interactive)
 		exit(interactive_add(argc - 1, argv + 1, prefix));
+	if (ignore_add_errors)
+		ignore_unmatch = 1;
 
 	if (edit_interactive)
 		return(edit_patch(argc, argv, prefix));
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 5/5] Convert add_file_to_index's lstat failure from a die to an error
From: Luke Dashjr @ 2009-08-13  3:20 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <1250133624-2272-4-git-send-email-luke-jr+git@utopios.org>

In order to make --ignore-errors able to ignore a lstat failure in
add_file_to_index, it needs to raise an error, NOT a fatal die.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 read-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 4e3e272..074e6b8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -638,7 +638,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
 	struct stat st;
 	if (lstat(path, &st))
-		die_errno("unable to stat '%s'", path);
+		return error_errno("unable to stat '%s'", path);
 	return add_to_index(istate, path, &st, flags);
 }
 
-- 
1.6.3.3

^ permalink raw reply related

* Re: GCC Git mirror no longer updating
From: Eric Wong @ 2009-08-13  3:37 UTC (permalink / raw)
  To: Bernie Innocenti
  Cc: Jason Merrill, Daniel Berlin, Frank Ch. Eigler, overseers, git
In-Reply-To: <1250123299.8074.1593.camel@giskard>

Bernie Innocenti <bernie@codewiz.org> wrote:
> El Wed, 12-08-2009 a las 09:45 -0400, Jason Merrill escribió:
> > On 08/12/2009 06:56 AM, Bernie Innocenti wrote:
> > > The git repository format should support concurrent access, but perhaps
> > > it only applies to git-receive-pack, not fancy operations such as
> > > repacking.
> > 
> > The git repository format, yes, but maybe not the stuff in .git/svn.  It 
> > seems like a temporary index file was referring to an object that got 
> > garbage collected away.  Or maybe the index file was left over from the 
> > initial import, and not there due to a collision; there don't seem to be 
> > index files there normally.
> 
> git-svn might be keeping extra information in files that the other git
> tools don't know about.  This would explain why some objects looked
> like orphans and were thus culled.  [cc'ing the git list to catch the
> attention of the git-svn maintainer(s)].

Hi,

As far as I can remember, no version of git svn has ever relied on
orphanable objects.

Of course there are unavoidable race conditions that happen while git
svn is running.  It is never safe to run repack concurrently while git
svn is running (I wouldn't repack/gc simultaneously with _any_ write
activity on the repo).   git svn itself can/will run "git gc" in-between
revisions if needed.  You can safely repack manually whenever git svn is
not running.

-- 
Eric Wong

^ permalink raw reply

* [PATCH] block-sha1: more good unaligned memory access candidates
From: Nicolas Pitre @ 2009-08-13  4:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git

In addition to X86, PowerPC and S390 are capable of unaligned memory 
accesses.

Signed-off-by: Nicolas Pitre <nico@cam.org>

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index d3121f7..e5a1007 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -67,7 +67,10 @@
  * and is faster on architectures with memory alignment issues.
  */
 
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__) || defined(__x86_64__) || \
+    defined(__ppc__) || defined(__ppc64__) || \
+    defined(__powerpc__) || defined(__powerpc64__) || \
+    defined(__s390__) || defined(__s390x__)
 
 #define get_be32(p)	ntohl(*(unsigned int *)(p))
 #define put_be32(p, v)	do { *(unsigned int *)(p) = htonl(v); } while (0)

^ permalink raw reply related

* Re: fatal: bad revision 'HEAD'
From: Junio C Hamano @ 2009-08-13  4:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <20090813023137.GA17358@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I.e., I have done in the past (but not frequently):
>
>   git symbolic-ref HEAD refs/heads/to-be-born
>
> in an existing repository to create a new root.

I honestly do not know of a sane reason (other than "because I can")
anybody would want to _start_ a new root in a repository with an existing
history.  And doing a "pull" with or without --rebase immediately after
starting a new root is doubly insane, as you say.

I do not think _ending up to_ have more than one root in your repository
is necessarily insane.  You may find a related project that earlier
started independently but later turned out to be better off managed
together with your project, and at that point you may perform Linus's
"coolest merge ever" to bind the two histories together, resulting in a
history with more than one root.

But that is the kind of "ending up to have" I am talking about; it is not
something you _aim to_ create on purpose.  If you want to _start_ a
separate history, and if you are sane, you would start the separate
history in a separate repository.

^ permalink raw reply

* Re: fatal: bad revision 'HEAD'
From: Jeff King @ 2009-08-13  4:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <7v1vngmitn.fsf@alter.siamese.dyndns.org>

On Wed, Aug 12, 2009 at 09:36:04PM -0700, Junio C Hamano wrote:

> I honestly do not know of a sane reason (other than "because I can")
> anybody would want to _start_ a new root in a repository with an existing
> history.  And doing a "pull" with or without --rebase immediately after
> starting a new root is doubly insane, as you say.

IIRC, the reason I did it was to throw away history, starting a new root
at the current state. Which is at least a little bit sane, though I
think I might just do it with a graft and filter-branch these days.

> But that is the kind of "ending up to have" I am talking about; it is not
> something you _aim to_ create on purpose.  If you want to _start_ a
> separate history, and if you are sane, you would start the separate
> history in a separate repository.

Agreed. Let's not worry about it, then.

-Peff

^ permalink raw reply

* Re: fatal: bad revision 'HEAD'
From: Joel Mahoney @ 2009-08-13  5:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20090813043855.GA21158@coredump.intra.peff.net>

Hello,

I'm not sure I completely understand where you guys are at with this  
thread : - ) but I thought I would mention that the question arose  
from my inability to install a plugin into a Ruby on Rails project  
based on my having (unknowingly) set branch.master.rebase = true.

I bring this up because a lot of people are getting their first  
exposure to git through Rails/github, and installing plugins in Rails  
is (I think) a good example of brining an existing history (the  
plugin) into a separate repository (your project).  and because this  
maneuver is built into the "./script/plugin install" action, it is not  
something that cannot be easily customized.

again, I may be a little off track here, but I thought I would remind  
you of the original context for what that is worth : - )

thanks again for your help!

Joel


On Aug 12, 2009, at 10:38 PM, Jeff King wrote:

> On Wed, Aug 12, 2009 at 09:36:04PM -0700, Junio C Hamano wrote:
>
>> I honestly do not know of a sane reason (other than "because I can")
>> anybody would want to _start_ a new root in a repository with an  
>> existing
>> history.  And doing a "pull" with or without --rebase immediately  
>> after
>> starting a new root is doubly insane, as you say.
>
> IIRC, the reason I did it was to throw away history, starting a new  
> root
> at the current state. Which is at least a little bit sane, though I
> think I might just do it with a graft and filter-branch these days.
>
>> But that is the kind of "ending up to have" I am talking about; it  
>> is not
>> something you _aim to_ create on purpose.  If you want to _start_ a
>> separate history, and if you are sane, you would start the separate
>> history in a separate repository.
>
> Agreed. Let's not worry about it, then.
>
> -Peff

^ permalink raw reply

* [PATCH 0/4] fast-import: add a new option command
From: Sverre Rabbelier @ 2009-08-13  5:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List

Sverre Rabbelier (4):
      fast-import: put option parsing code in seperate functions
      fast-import: define a new option command
      fast-import: add option command
      fast-import: test the new option command

 Documentation/git-fast-import.txt |   23 ++++++
 fast-import.c                     |  149 +++++++++++++++++++++++++++----------
 t/t9300-fast-import.sh            |   33 ++++++++
 3 files changed, 165 insertions(+), 40 deletions(-)

^ permalink raw reply

* [PATCH 1/4] fast-import: put option parsing code in seperate functions
From: Sverre Rabbelier @ 2009-08-13  5:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
  Cc: Sverre Rabbelier
In-Reply-To: <1250140186-12363-1-git-send-email-srabbelier@gmail.com>

Putting the options in their own functions increases readability of
the option parsing block and makes it easier to reuse the option
parsing code later on.
---

    This is nearly identical to the RFC, but with parse_one_option
    also factored out for easy reuse.

 fast-import.c |  132 +++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 92 insertions(+), 40 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 7ef9865..17d57ab 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -291,6 +291,7 @@ static unsigned long branch_count;
 static unsigned long branch_load_count;
 static int failure;
 static FILE *pack_edges;
+static unsigned int show_stats = 1;
 
 /* Memory pools */
 static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -2337,7 +2338,7 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
-static void import_marks(const char *input_file)
+static void option_import_marks(const char *input_file)
 {
 	char line[512];
 	FILE *f = fopen(input_file, "r");
@@ -2372,6 +2373,93 @@ static void import_marks(const char *input_file)
 	fclose(f);
 }
 
+static void option_date_format(const char *fmt)
+{
+	if (!strcmp(fmt, "raw"))
+		whenspec = WHENSPEC_RAW;
+	else if (!strcmp(fmt, "rfc2822"))
+		whenspec = WHENSPEC_RFC2822;
+	else if (!strcmp(fmt, "now"))
+		whenspec = WHENSPEC_NOW;
+	else
+		die("unknown --date-format argument %s", fmt);
+}
+
+static void option_max_pack_size(const char *packsize)
+{
+	max_packsize = strtoumax(packsize, NULL, 0) * 1024 * 1024;
+}
+
+static void option_depth(const char *depth)
+{
+	max_depth = strtoul(depth, NULL, 0);
+	if (max_depth > MAX_DEPTH)
+		die("--depth cannot exceed %u", MAX_DEPTH);
+}
+
+static void option_active_branches(const char *branches)
+{
+	max_active_branches = strtoul(branches, NULL, 0);
+}
+
+static void option_export_marks(const char *marks)
+{
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_addstr(&buf, marks);
+	mark_file = strbuf_detach(&buf, NULL);
+}
+
+static void option_export_pack_edges(const char *edges)
+{
+	if (pack_edges)
+		fclose(pack_edges);
+	pack_edges = fopen(edges, "a");
+	if (!pack_edges)
+		die_errno("Cannot open '%s'", edges);
+}
+
+static void option_force()
+{
+	force_update = 1;
+}
+
+static void option_quiet()
+{
+	show_stats = 0;
+}
+
+static void option_stats()
+{
+	show_stats = 1;
+}
+
+static void parse_one_option(const char *option)
+{
+    if (!prefixcmp(option, "date-format=")) {
+		option_date_format(option + 12);
+    } else if (!prefixcmp(option, "max-pack-size=")) {
+		option_max_pack_size(option + 14);
+    } else if (!prefixcmp(option, "depth=")) {
+		option_depth(option + 6);
+    } else if (!prefixcmp(option, "active-branches=")) {
+		option_active_branches(option + 16);
+    } else if (!prefixcmp(option, "import-marks=")) {
+		option_import_marks(option + 13);
+    } else if (!prefixcmp(option, "export-marks=")) {
+		option_export_marks(option + 13);
+    } else if (!prefixcmp(option, "export-pack-edges=")) {
+		option_export_pack_edges(option + 18);
+    } else if (!prefixcmp(option, "force")) {
+		option_force();
+    } else if (!prefixcmp(option, "quiet")) {
+		option_quiet();
+    } else if (!prefixcmp(option, "stats")) {
+		option_stats();
+    } else {
+		die("Unsupported option: %s", option);
+    }
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2398,7 +2486,7 @@ static const char fast_import_usage[] =
 
 int main(int argc, const char **argv)
 {
-	unsigned int i, show_stats = 1;
+	unsigned int i;
 
 	git_extract_argv0_path(argv[0]);
 
@@ -2419,44 +2507,8 @@ int main(int argc, const char **argv)
 
 		if (*a != '-' || !strcmp(a, "--"))
 			break;
-		else if (!prefixcmp(a, "--date-format=")) {
-			const char *fmt = a + 14;
-			if (!strcmp(fmt, "raw"))
-				whenspec = WHENSPEC_RAW;
-			else if (!strcmp(fmt, "rfc2822"))
-				whenspec = WHENSPEC_RFC2822;
-			else if (!strcmp(fmt, "now"))
-				whenspec = WHENSPEC_NOW;
-			else
-				die("unknown --date-format argument %s", fmt);
-		}
-		else if (!prefixcmp(a, "--max-pack-size="))
-			max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
-		else if (!prefixcmp(a, "--depth=")) {
-			max_depth = strtoul(a + 8, NULL, 0);
-			if (max_depth > MAX_DEPTH)
-				die("--depth cannot exceed %u", MAX_DEPTH);
-		}
-		else if (!prefixcmp(a, "--active-branches="))
-			max_active_branches = strtoul(a + 18, NULL, 0);
-		else if (!prefixcmp(a, "--import-marks="))
-			import_marks(a + 15);
-		else if (!prefixcmp(a, "--export-marks="))
-			mark_file = a + 15;
-		else if (!prefixcmp(a, "--export-pack-edges=")) {
-			if (pack_edges)
-				fclose(pack_edges);
-			pack_edges = fopen(a + 20, "a");
-			if (!pack_edges)
-				die_errno("Cannot open '%s'", a + 20);
-		} else if (!strcmp(a, "--force"))
-			force_update = 1;
-		else if (!strcmp(a, "--quiet"))
-			show_stats = 0;
-		else if (!strcmp(a, "--stats"))
-			show_stats = 1;
-		else
-			die("unknown option %s", a);
+
+		parse_one_option(a + 2);
 	}
 	if (i != argc)
 		usage(fast_import_usage);
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-08-13  5:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
  Cc: Sverre Rabbelier
In-Reply-To: <1250140186-12363-2-git-send-email-srabbelier@gmail.com>

This allows the frontend to specify any of the supported options as
long as no non-option command has been given.
---

    As requested, updated the documentation of the language format

 Documentation/git-fast-import.txt |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c2f483a..6b5bc1b 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -303,6 +303,11 @@ and control the current import process.  More detailed discussion
 	standard output.  This command is optional and is not needed
 	to perform an import.
 
+`option`::
+    Specify any of the options listed under OPTIONS to change
+    fast-import's behavior to suit the frontends needs. This command
+    is optional and is not needed to perform an import.
+
 `commit`
 ~~~~~~~~
 Create or update a branch with a new commit, recording one logical
@@ -813,6 +818,24 @@ Placing a `progress` command immediately after a `checkpoint` will
 inform the reader when the `checkpoint` has been completed and it
 can safely access the refs that fast-import updated.
 
+`option`
+~~~~~~~~
+Processes the specified option so that git fast-import behaves in a
+way that suits the front-ends needs.
+Note that options specified by the frontend override any options the
+user may specify to git fast-import itself.
+
+....
+    'option' SP <option> LF
+....
+
+The `<option>` part of the command may contain any of the options
+listed in the OPTIONS section, without the leading '--' and is
+treated in the same way.
+
+Option commands must be the first commands on the input, to give an
+option command after any non-option command is an error.
+
 Crash Reports
 -------------
 If fast-import is supplied invalid input it will terminate with a
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH 4/4] fast-import: test the new option command
From: Sverre Rabbelier @ 2009-08-13  5:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
  Cc: Sverre Rabbelier
In-Reply-To: <1250140186-12363-4-git-send-email-srabbelier@gmail.com>

---

    Only difference with the previous version is
    s/export-marks /export-marks=/

 t/t9300-fast-import.sh |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 821be7c..4152c3a 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1088,4 +1088,37 @@ INPUT_END
 test_expect_success 'P: fail on blob mark in gitlink' '
     test_must_fail git fast-import <input'
 
+###
+### series Q (options)
+###
+
+cat >input << EOF
+option quiet
+blob
+data 3
+hi
+
+EOF
+
+touch empty
+
+test_expect_success 'Q: quiet option results in no stats being output' '
+    cat input | git fast-import 2> output &&
+    test_cmp empty output
+'
+
+cat >input << EOF
+option export-marks=git.marks
+blob
+mark :1
+data 3
+hi
+
+EOF
+
+test_expect_success \
+    'Q: export-marks option results in a marks file being created' \
+    'cat input | git fast-import 2> output &&
+    grep :1 git.marks'
+
 test_done
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH 3/4] fast-import: add option command
From: Sverre Rabbelier @ 2009-08-13  5:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
  Cc: Sverre Rabbelier
In-Reply-To: <1250140186-12363-3-git-send-email-srabbelier@gmail.com>

This allows the frontend to specify any of the supported options as
long as no non-option command has been given. This way the
user does not have to include any frontend-specific options, but
instead she can rely on the frontend to tell fast-import what it
needs.
---

    This is a lot simpler from the previous version as we can now
    reuse parse_one_option from 1/4.

 fast-import.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 17d57ab..2ec804d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -348,6 +348,7 @@ static struct recent_command *rc_free;
 static unsigned int cmd_save = 100;
 static uintmax_t next_mark;
 static struct strbuf new_data = STRBUF_INIT;
+static int seen_non_option_command;
 
 static void write_branch_report(FILE *rpt, struct branch *b)
 {
@@ -1663,6 +1664,10 @@ static int read_next_command(void)
 			if (stdin_eof)
 				return EOF;
 
+			if (!seen_non_option_command
+				&& prefixcmp(command_buf.buf, "option "))
+				seen_non_option_command = 1;
+
 			rc = rc_free;
 			if (rc)
 				rc_free = rc->next;
@@ -2460,6 +2465,16 @@ static void parse_one_option(const char *option)
     }
 }
 
+static void parse_option(void)
+{
+    char* option = command_buf.buf + 7;
+
+	if (seen_non_option_command)
+		die("Got option command '%s' after non-option command", option);
+
+    parse_one_option(option);
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2534,6 +2549,8 @@ int main(int argc, const char **argv)
 			parse_checkpoint();
 		else if (!prefixcmp(command_buf.buf, "progress "))
 			parse_progress();
+		else if (!prefixcmp(command_buf.buf, "option "))
+			parse_option();
 		else
 			die("Unsupported command: %s", command_buf.buf);
 	}
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* Re: fatal: bad revision 'HEAD'
From: Jeff King @ 2009-08-13  5:10 UTC (permalink / raw)
  To: Joel Mahoney; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <89456E84-1A85-4D0C-9643-C191EE877E61@gmail.com>

On Wed, Aug 12, 2009 at 11:02:45PM -0600, Joel Mahoney wrote:

> I'm not sure I completely understand where you guys are at with this
> thread : - ) but I thought I would mention that the question arose
> from my inability to install a plugin into a Ruby on Rails project
> based on my having (unknowingly) set branch.master.rebase = true.
>
> I bring this up because a lot of people are getting their first
> exposure to git through Rails/github, and installing plugins in Rails
> is (I think) a good example of brining an existing history (the
> plugin) into a separate repository (your project).  and because this
> maneuver is built into the "./script/plugin install" action, it is
> not something that cannot be easily customized.

Yes, the discussion moved away bit from your original issue. To
summarize what is happening:

  - Setting branch.master.rebase in your ~/.gitconfig is probably not a
    good idea, as that tends to be a per-repository property, anyway.
    You can work around the problem for now by removing it.

  - Junio has committed a patch to fix the bug. It will definitely be in
    v1.6.5. If this is a common setup for Rails people, maybe we should
    consider putting it into 'maint' for v1.6.4.1.

-Peff

^ permalink raw reply

* [PATCH] svn: initial "master" points to trunk if possible
From: Eric Wong @ 2009-08-13  5:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Since "trunk" is a convention for the main development branch in
the SVN world, try to make that the master branch upon initial
checkout if it exists.  This is probably less surprising based
on user requests.

t9135 was the only test which relied on the previous behavior
and thus needed to be modified.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

  pushed out to git://yhbt.net/git-svn along with some others

  Adam Brewster (1):
        svn: allow branches outside of refs/remotes

  Eric Wong (2):
        svn: initial "master" points to trunk if possible
        svn: (cleanup) use predefined constant for rev_map_fmt


 git-svn.perl                               |   11 +++++++++++
 t/t9135-git-svn-moved-branch-empty-file.sh |    7 ++++++-
 t/t9145-git-svn-master-branch.sh           |   25 +++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletions(-)
 create mode 100755 t/t9145-git-svn-master-branch.sh

diff --git a/git-svn.perl b/git-svn.perl
index b0bfb74..fad2960 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1156,6 +1156,17 @@ sub post_fetch_checkout {
 	my $gs = $Git::SVN::_head or return;
 	return if verify_ref('refs/heads/master^0');
 
+	# look for "trunk" ref if it exists
+	my $remote = Git::SVN::read_all_remotes()->{$gs->{repo_id}};
+	my $fetch = $remote->{fetch};
+	if ($fetch) {
+		foreach my $p (keys %$fetch) {
+			basename($fetch->{$p}) eq 'trunk' or next;
+			$gs = Git::SVN->new($fetch->{$p}, $gs->{repo_id}, $p);
+			last;
+		}
+	}
+
 	my $valid_head = verify_ref('HEAD^0');
 	command_noisy(qw(update-ref refs/heads/master), $gs->refname);
 	return if ($valid_head || !verify_ref('HEAD^0'));
diff --git a/t/t9135-git-svn-moved-branch-empty-file.sh b/t/t9135-git-svn-moved-branch-empty-file.sh
index 03705fa..5280e5f 100755
--- a/t/t9135-git-svn-moved-branch-empty-file.sh
+++ b/t/t9135-git-svn-moved-branch-empty-file.sh
@@ -10,7 +10,12 @@ test_expect_success 'load svn dumpfile'  '
 test_expect_success 'clone using git svn' 'git svn clone -s "$svnrepo" x'
 
 test_expect_success 'test that b1 exists and is empty' '
-	(cd x && test -f b1 && ! test -s b1)
+	(
+		cd x &&
+		git reset --hard branch-c &&
+		test -f b1 &&
+		! test -s b1
+	)
 	'
 
 test_done
diff --git a/t/t9145-git-svn-master-branch.sh b/t/t9145-git-svn-master-branch.sh
new file mode 100755
index 0000000..16852d2
--- /dev/null
+++ b/t/t9145-git-svn-master-branch.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Eric Wong
+#
+test_description='git svn initial master branch is "trunk" if possible'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup test repository' '
+	mkdir i &&
+	> i/a &&
+	svn_cmd import -m trunk i "$svnrepo/trunk" &&
+	svn_cmd import -m b/a i "$svnrepo/branches/a" &&
+	svn_cmd import -m b/b i "$svnrepo/branches/b"
+'
+
+test_expect_success 'git svn clone --stdlayout sets up trunk as master' '
+	git svn clone -s "$svnrepo" g &&
+	(
+		cd g &&
+		test x`git rev-parse --verify refs/remotes/trunk^0` = \
+		     x`git rev-parse --verify refs/heads/master^0`
+	)
+'
+
+test_done
-- 
Eric Wong

^ permalink raw reply related


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