Git development
 help / color / mirror / Atom feed
* Re: [msysGit] Re: Compiling on Windows
From: Johannes Schindelin @ 2011-10-19 23:17 UTC (permalink / raw)
  To: Philip Oakley; +Cc: Andrew Ardill, Vincent van Ravesteijn, Git MsysGit, git
In-Reply-To: <2015B7F2CEAE4B449EA4EF744F9B8FD9@PhilipOakley>

Hi,

On Wed, 19 Oct 2011, Philip Oakley wrote:

> From: "Vincent van Ravesteijn" <vfr@lyx.org>
> > Op 18-10-2011 6:08, Andrew Ardill schreef:
> > > Hi list, I have been searching for details on what is required to 
> > > compile on Windows, but haven't found anything conclusive. Perhaps 
> > > there is something on the wiki, but unfortunately it is down at the 
> > > moment.

The quickest way to get Git for Windows compiled is to download & run
msysGit-netinstall from http://msysgit.googlecode.com/.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 08/12] do_for_each_ref_in_arrays(): new function
From: Junio C Hamano @ 2011-10-19 22:39 UTC (permalink / raw)
  To: mhagger
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips
In-Reply-To: <1319060692-27216-9-git-send-email-mhagger@alum.mit.edu>

Is this necessary?  IOW, is the helper function usable in any context
other than merge-iterate loose and packed refs?

^ permalink raw reply

* Re: [IGNORETHIS/PATCH] Choosing the sha1 prefix of your commits
From: Jonathan Nieder @ 2011-10-19 22:09 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List
In-Reply-To: <CACBZZX5PqYa0uWiGgs952rk2cy+QRCU95kF63qzSi3fKK-YrCQ@mail.gmail.com>

Ævar Arnfjörð Bjarmason wrote:

> Someone asked me if having a "lulz" header wouldn't
> break things, but since we introduced the "encoding" header a while
> back clients have learned to ignore unknown headers, so it doesn't.
>
> Which is why we can discuss e.g. adding GPG headers without worrying
> about breaking everything.

Just for the record (and I think you understood this already): it is
not quite so simple.  Clients are allowed to assume that no "unknown"
header lines will appear before any of the known fields.  A future git
version could even assume that, say, a

	length-of-remainder-of-header

field appears before any lines after encoding and the GPG stuff.

So it would definitely be frowned upon to add new lines to that part
of the commit object without at least discussing it on this list.
Jeff mentioned one way to get your lulz without violating that
constraint. ;-)

http://thread.gmane.org/gmane.comp.version-control.git/138848/focus=138921

^ permalink raw reply

* Re: [RFC 04/13] add_ref(): move the call of check_refname_format() to callers
From: Michael Haggerty @ 2011-10-19 21:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7vsjmobqim.fsf@alter.siamese.dyndns.org>

On 10/19/2011 11:49 PM, Junio C Hamano wrote:
> mhagger@alum.mit.edu writes:
>> I'm still not clear on how extra_refs are used.  Are they generated
>> from local refs or are they generated from remote refs?  If the
>> latter, then it is probably irresponsible not to do *some* sanity
>> checking in add_extra_ref() to prevent any chance of refnames like
>> "../../../etc/passwd".
> 
> No, add_extra_ref() already tells us what their values are, these are
> never used to actually read from filesystem. Their refname field has
> almost no value other than for debugging and we probably shouldn't even
> insist on uniqueness among extra refs or for that matter collision with
> the real refs. [...]

Thanks for the explanation.  I'm inspired to separate them a little bit
more from "real" refs because they are such a special case.  For
example, maybe it would make sense to add a function
for_each_extra_ref() to avoid having to mix them with real refs in the
iteration.  OTOH not important AFAICS.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* What's cooking in git.git (Oct 2011, #07; Wed, 19)
From: Junio C Hamano @ 2011-10-19 21:57 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'.

The fifth batch of topics started graduating to the master branch. It is
probably a good point to stop taking new topics and start switching our
focus to fixing bugs in the topics already in 'master'.

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo, html and man:

	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, master, html and man:

	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 but not todo, html or man:

	https://github.com/gitster/git

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

* jc/match-refs-clarify (2011-09-12) 2 commits
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message

* jn/libperl-git-config (2011-10-18) 1 commit
 - libperl-git: refactor Git::config_*

* ss/inet-ntop (2011-10-18) 1 commit
 - inet_ntop.c: Work around GCC 4.6's detection of uninitialized variables

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

* bk/submodule-in-recursive-merge (2011-10-13) 2 commits
  (merged to 'next' on 2011-10-15 at e02205c)
 + submodule: Search for merges only at end of recursive merge
 + submodule: Demonstrate known breakage during recursive merge

Brad helped resurrecting good bits earlier tangled in the stalled topic
hv/submodule-merge-search by mistake.

* cb/httpd-test-fix-port (2011-10-17) 1 commit
  (merged to 'next' on 2011-10-17 at 84fb7e6)
 + use test number as port number

* jm/maint-apply-detects-corrupt-patch-header (2011-10-12) 1 commit
  (merged to 'next' on 2011-10-12 at 80d9503)
 + fix "git apply --index ..." not to deref NULL

* jn/gitweb-manpages (2011-10-16) 5 commits
  (merged to 'next' on 2011-10-16 at 6555a07)
 + gitweb: Add gitweb manpages to 'gitweb' package in git.spec
 + Documentation: Add gitweb config variables to git-config(1)
 + Documentation: Link to gitweb(1) and gitweb.conf(5) in other manpages
 + gitweb: Add gitweb(1) manpage for gitweb itself
 + gitweb: Add gitweb.conf(5) manpage for gitweb configuration files

* js/log-show-children (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at de8f6f2)
 + log --children

* js/maint-no-cherry-pick-head-after-punted (2011-10-06) 2 commits
 + cherry-pick: do not give irrelevant advice when cherry-pick punted
 + revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
 (this branch is used by js/no-cherry-pick-head-after-punted.)

Will merge to 'maint' later.

* js/merge-edit-option (2011-10-12) 1 commit
  (merged to 'next' on 2011-10-12 at db28da3)
 + Teach merge the '[-e|--edit]' option

* js/no-cherry-pick-head-after-punted (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-10 at acb29ee)
 + Merge branch 'js/maint-no-cherry-pick-head-after-punted' into js/no-cherry-pick-head-after-punted
 (this branch uses js/maint-no-cherry-pick-head-after-punted.)

* md/smtp-tls-hello-again (2011-10-15) 1 commit
  (merged to 'next' on 2011-10-16 at 3e27de4)
 + send-email: Honour SMTP domain when using TLS

* pt/mingw-misc-fixes (2011-10-15) 7 commits
  (merged to 'next' on 2011-10-16 at 2dc0b55)
 + t9901: fix line-ending dependency on windows
 + mingw: ensure sockets are initialized before calling gethostname
 + mergetools: use the correct tool for Beyond Compare 3 on Windows
 + t9300: do not run --cat-blob-fd related tests on MinGW
 + git-svn: On MSYS, escape and quote SVN_SSH also if set by the user
 + t9001: do not fail only due to CR/LF issues
 + t1020: disable the pwd test on MinGW

* rs/diff-whole-function (2011-10-10) 2 commits
  (merged to 'next' on 2011-10-11 at 6196752)
 + diff: add option to show whole functions as context
 + xdiff: factor out get_func_line()

* rs/pickaxe (2011-10-07) 7 commits
  (merged to 'next' on 2011-10-11 at 27d02b2)
 + pickaxe: factor out pickaxe
 + pickaxe: give diff_grep the same signature as has_changes
 + pickaxe: pass diff_options to contains and has_changes
 + pickaxe: factor out has_changes
 + pickaxe: plug regex/kws leak
 + pickaxe: plug regex leak
 + pickaxe: plug diff filespec leak with empty needle

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

* hv/submodule-merge-search (2011-10-13) 4 commits
 - submodule.c: make two functions static
 - allow multiple calls to submodule merge search for the same path
 - push: Don't push a repository with unpushed submodules
 - push: teach --recurse-submodules the on-demand option

What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.

The fix-up at the tip queued on fg/submodule-auto-push topic has been
moved to this topic.

* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
 - t5800: point out that deleting branches does not work
 - t5800: document inability to push new branch with old content

Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) would help.

* rr/revert-cherry-pick (2011-10-12) 7 commits
 - revert: further simplify parsing of a line in insn sheet
 - revert: Simplify passing command-line arguments around
 - revert: Allow mixed pick and revert instructions
 - revert: Make commit descriptions in insn sheet optional
 - revert: Fix buffer overflow in insn sheet parser
 - revert: Simplify getting commit subject
 - revert: Free memory after get_message call

Probably needs a little bit more polish, e.g. squashing the tip fixup into
an earlier one in the series.

* jc/lookup-object-hash (2011-08-11) 6 commits
 - object hash: replace linear probing with 4-way cuckoo hashing
 - object hash: we know the table size is a power of two
 - object hash: next_size() helper for readability
 - pack-objects --count-only
 - object.c: remove duplicated code for object hashing
 - object.c: code movement for readability

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload.

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

* jc/check-ref-format-fixup (2011-10-12) 2 commits
  (merged to 'next' on 2011-10-15 at 8e89bc5)
 + Restrict ref-like names immediately below $GIT_DIR
 + refs.c: move dwim_ref()/dwim_log() from sha1_name.c

An attempt to fix-up unfortunate side effect of mh/check-ref-format-3
topic. "git show -s config" is never meant to refer to $GIT_DIR/config
and treat it as a file that records an object name. But tightening the
rule for reading side means we would need to tighten the rule for the
writing side, too, which turned out to be rather nasty fallout that
needs to be sorted out before the next release.

* pw/p4-update (2011-10-17) 6 commits
  (merged to 'next' on 2011-10-17 at f69f6cc)
 + git-p4: handle files with shell metacharacters
 + git-p4: keyword flattening fixes
 + git-p4: stop ignoring apple filetype
 + git-p4: recognize all p4 filetypes
 + git-p4: handle utf16 filetype properly
 + git-p4 tests: refactor and cleanup

Will merge to 'master' in the fifth wave.

* cn/doc-config-bare-subsection (2011-10-16) 1 commit
  (merged to 'next' on 2011-10-17 at a6412d4)
 + Documentation: update [section.subsection] to reflect what git does

Will merge to 'master' in the fifth wave.

* cb/daemon-permission-errors (2011-10-17) 2 commits
 - daemon: report permission denied error to clients
 - daemon: add tests
 (this branch uses jk/daemon-msgs.)

The tip commit might be loosening things a bit too much.
Will keep in 'pu' until hearing a convincing argument for the patch.

* jc/verbose-checkout (2011-10-16) 2 commits
 - checkout -v: give full status output after switching branches
 - checkout: move the local changes report to the end

This is just to leave a record that the reason why we do not do this not
because we are incapable of coding this, but because it is not a good idea
to do this. I suspect people who are new to git that might think they need
it would soon realize the don't.

Will keep in 'pu' as a showcase for a while and then will drop.

* kk/gitweb-side-by-side-diff (2011-10-17) 2 commits
 - gitweb: add a feature to show side-by-side diff
 - gitweb: change format_diff_line() to remove leading SP from $diff_class

Fun.
Will keep in 'pu' until the planned re-roll comes.

* mh/ref-api-2 (2011-10-17) 14 commits
  (merged to 'next' on 2011-10-19 at cc89f0e)
 + resolve_gitlink_ref_recursive(): change to work with struct ref_cache
 + Pass a (ref_cache *) to the resolve_gitlink_*() helper functions
 + resolve_gitlink_ref(): improve docstring
 + get_ref_dir(): change signature
 + refs: change signatures of get_packed_refs() and get_loose_refs()
 + is_dup_ref(): extract function from sort_ref_array()
 + add_ref(): add docstring
 + parse_ref_line(): add docstring
 + is_refname_available(): remove the "quiet" argument
 + clear_ref_array(): rename from free_ref_array()
 + refs: rename parameters result -> sha1
 + refs: rename "refname" variables
 + struct ref_entry: document name member
 + cache.h: add comments for git_path() and git_path_submodule()
 (this branch uses mh/ref-api.)

* po/insn-editor (2011-10-17) 1 commit
  (merged to 'next' on 2011-10-19 at cbf5e0b)
 + "rebase -i": support special-purpose editor to edit insn sheet

* dm/pack-objects-update (2011-10-18) 4 commits
 - pack-objects: don't traverse objects unnecessarily
 - pack-objects: rewrite add_descendants_to_write_order() iteratively
 - pack-objects: use unsigned int for counter and offset values
 - pack-objects: mark add_to_write_order() as inline

* jk/git-tricks (2011-10-18) 3 commits
 - completion: match ctags symbol names in grep patterns
 - contrib: add git-jump script
 - contrib: add diff highlight script

* jc/make-tags (2011-10-18) 1 commit
 - Makefile: ask "ls-files" to list source files if available

* jc/signed-commit (2011-10-18) 3 commits
 - log: --show-signature
 - commit: teach --gpg-sign option
 - Split GPG interface into its own helper library

This is to replace the earlier "signed push" experiments. "verify-tag"
equivalent needs to be written before this can proceed. I suspect that
teaching "verify-tag" to notice and also handle signed commits would be
the easiest, but "git tag --verify $commit" might look slightly funny
from the UI POV. I dunno.

* mh/ref-api (2011-10-16) 7 commits
  (merged to 'next' on 2011-10-17 at 219000f)
 + clear_ref_cache(): inline function
 + write_ref_sha1(): only invalidate the loose ref cache
 + clear_ref_cache(): extract two new functions
 + clear_ref_cache(): rename parameter
 + invalidate_ref_cache(): expose this function in the refs API
 + invalidate_ref_cache(): take the submodule as parameter
 + invalidate_ref_cache(): rename function from invalidate_cached_refs()
 (this branch is used by mh/ref-api-2.)

Will merge to 'master' in the sixth wave.

* sg/complete-refs (2011-10-15) 10 commits
 - completion: remove broken dead code from __git_heads() and __git_tags()
 - completion: fast initial completion for config 'remote.*.fetch' value
 - completion: improve ls-remote output filtering in __git_refs_remotes()
 - completion: query only refs/heads/ in __git_refs_remotes()
 - completion: support full refs from remote repositories
 - completion: improve ls-remote output filtering in __git_refs()
 - completion: make refs completion consistent for local and remote repos
 - completion: optimize refs completion
 - completion: document __gitcomp()
 - Merge branches 'tm/completion-push-set-upstream', 'tm/completion-commit-fixup-squash' and 'sg/completion' into HEAD

Rerolled.

Will keep in 'pu' until an Ack or two from people who have worked on the
completion in the past comes.

* jc/unseekable-bundle (2011-10-13) 2 commits
  (merged to 'next' on 2011-10-19 at 2978ee0)
 + bundle: add parse_bundle_header() helper function
 + bundle: allowing to read from an unseekable fd

I am not entirely happy with the first patch but it is not so bad either.

* jk/daemon-msgs (2011-10-15) 1 commit
  (merged to 'next' on 2011-10-15 at 415cf53)
 + daemon: give friendlier error messages to clients
 (this branch is used by cb/daemon-permission-errors.)

Will merge to 'master' in the fifth wave.

* jk/maint-pack-objects-compete-with-delete (2011-10-14) 2 commits
  (merged to 'next' on 2011-10-15 at 49479e4)
 + downgrade "packfile cannot be accessed" errors to warnings
 + pack-objects: protect against disappearing packs

Will merge to 'master' in the fifth wave.

* cn/fetch-prune (2011-10-15) 5 commits
  (merged to 'next' on 2011-10-16 at 02a449e)
 + fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
 + fetch: honor the user-provided refspecs when pruning refs
 + remote: separate out the remote_find_tracking logic into query_refspecs
 + t5510: add tests for fetch --prune
 + fetch: free all the additional refspecs

Will merge to 'master' in the sixth wave.

* sc/difftool-skip (2011-10-14) 2 commits
  (merged to 'next' on 2011-10-14 at b91c581)
 + t7800: avoid arithmetic expansion notation
  (merged to 'next' on 2011-10-11 at 38d7e84)
 + git-difftool: allow skipping file by typing 'n' at prompt

Will merge to 'master' in the fifth wave.

* jc/maint-remove-renamed-ref (2011-10-12) 1 commit
  (merged to 'next' on 2011-10-12 at 819c3e4)
 + branch -m/-M: remove undocumented RENAMED-REF

Will merge to 'master' in the sixth wave.

* ph/transport-with-gitfile (2011-10-11) 5 commits
  (merged to 'next' on 2011-10-12 at 6d58417)
 + Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
  (merged to 'next' on 2011-10-06 at 891b8b6)
 + Add test showing git-fetch groks gitfiles
 + Teach transport about the gitfile mechanism
 + Learn to handle gitfiles in enter_repo
 + enter_repo: do not modify input

Will merge to 'master' in the fifth wave.

* jc/request-pull-show-head-4 (2011-10-15) 11 commits
  (merged to 'next' on 2011-10-15 at 7e340ff)
 + fmt-merge-msg.c: Fix an "dubious one-bit signed bitfield" sparse error
  (merged to 'next' on 2011-10-10 at 092175e)
 + environment.c: Fix an sparse "symbol not declared" warning
 + builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
  (merged to 'next' on 2011-10-07 at fcaeca0)
 + fmt-merge-msg: use branch.$name.description
  (merged to 'next' on 2011-10-06 at fa5e0fe)
 + request-pull: use the branch description
 + request-pull: state what commit to expect
 + request-pull: modernize style
 + branch: teach --edit-description option
 + format-patch: use branch description in cover letter
 + branch: add read_branch_desc() helper function
 + Merge branch 'bk/ancestry-path' into jc/branch-desc

Will merge to 'master' in the sixth wave.

--------------------------------------------------
[Discarded]

* jk/http-auth-keyring (2011-10-12) 19 commits
 . http_init: accept separate URL parameter
 . credential-cache: don't cache items without context
 . check_expirations: don't copy over same element
 . t0300: add missing EOF terminator for <<
 . credential-store: use a better storage format
 . t0300: make alternate username tests more robust
 . t0300: make askpass tests a little more robust
 . credential-cache: fix expiration calculation corner cases
 . docs: minor tweaks to credentials API
 . credentials: make credential_fill_gently() static
 . credentials: add "getpass" helper
 . credentials: add "store" helper
 . credentials: add "cache" helper
 . docs: end-user documentation for the credential subsystem
 . http: use hostname in credential description
 . allow the user to configure credential helpers
 . look for credentials in config before prompting
 . http: use credential API to get passwords
 . introduce credentials API
 (this branch is used by js/cred-macos-x-keychain-2.)

Discarded without prejudice to allow design level discussions to continue.
Expecting a re-roll based on jk/http-auth

* js/cred-macos-x-keychain-2 (2011-10-12) 1 commit
 . contrib: add a pair of credential helpers for Mac OS X's keychain
 (this branch uses jk/http-auth-keyring.)

Discarded without prejudice to allow design level discussions to continue.
Expecting a re-roll based on jk/http-auth

^ permalink raw reply

* Re: [RFC 09/13] new_branch(): verify that new branch name is a valid full refname
From: Junio C Hamano @ 2011-10-19 21:52 UTC (permalink / raw)
  To: mhagger
  Cc: Junio C Hamano, git, Jeff King, cmn, A Large Angry SCM,
	Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <1319057716-28094-10-git-send-email-mhagger@alum.mit.edu>

mhagger@alum.mit.edu writes:

> From: Michael Haggerty <mhagger@alum.mit.edu>
>
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>
> Is it possible to omit the REFNAME_ALLOW_ONELEVEL option from this
> call?

I _think_ it takes an unadorned branch name, so the most prudent would be
to check the result of prefixing "refs/heads/" to *name with REFNAME_FULL.

>  fast-import.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index 8d8ea3c..51cf898 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -722,7 +722,7 @@ static struct branch *new_branch(const char *name)
>  
>  	if (b)
>  		die("Invalid attempt to create duplicate branch: %s", name);
> -	if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
> +	if (check_refname_format(name, REFNAME_FULL|REFNAME_ALLOW_ONELEVEL))
>  		die("Branch name doesn't conform to GIT standards: %s", name);
>  
>  	b = pool_calloc(1, sizeof(struct branch));

^ permalink raw reply

* [PATCH 10/12] names_conflict(): new function, extracted from is_refname_available()
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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


This costs an extra strlen() in the loop, but even that small price
will be clawed back in the next patch.

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

diff --git a/refs.c b/refs.c
index 7c2bcab..ada691b 100644
--- a/refs.c
+++ b/refs.c
@@ -1088,6 +1088,30 @@ static int remove_empty_directories(const char *file)
 }
 
 /*
+ * Return true iff refname1 and refname2 conflict with each other.
+ * Two reference names conflict if one of them exactly matches the
+ * leading components of the other; e.g., "foo/bar" conflicts with
+ * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
+ * "foo/barbados".
+ */
+static int names_conflict(const char *refname1, const char *refname2)
+{
+	int len1 = strlen(refname1);
+	int len2 = strlen(refname2);
+	int cmplen;
+	const char *lead;
+
+	if (len1 < len2) {
+		cmplen = len1;
+		lead = refname2;
+	} else {
+		cmplen = len2;
+		lead = refname1;
+	}
+	return !strncmp(refname1, refname2, cmplen) && lead[cmplen] == '/';
+}
+
+/*
  * Return true iff a reference named refname could be created without
  * conflicting with the name of an existing reference.  If oldrefname
  * is non-NULL, ignore potential conflicts with oldrefname (e.g.,
@@ -1097,20 +1121,15 @@ static int remove_empty_directories(const char *file)
 static int is_refname_available(const char *refname, const char *oldrefname,
 				struct ref_array *array)
 {
-	int i, namlen = strlen(refname); /* e.g. 'foo/bar' */
+	int i;
 	for (i = 0; i < array->nr; i++ ) {
 		struct ref_entry *entry = array->refs[i];
-		/* entry->name could be 'foo' or 'foo/bar/baz' */
-		if (!oldrefname || strcmp(oldrefname, entry->name)) {
-			int len = strlen(entry->name);
-			int cmplen = (namlen < len) ? namlen : len;
-			const char *lead = (namlen < len) ? entry->name : refname;
-			if (!strncmp(refname, entry->name, cmplen) &&
-			    lead[cmplen] == '/') {
-				error("'%s' exists; cannot create '%s'",
-				      entry->name, refname);
-				return 0;
-			}
+		if (oldrefname && !strcmp(oldrefname, entry->name))
+			continue;
+		if (names_conflict(refname, entry->name)) {
+			error("'%s' exists; cannot create '%s'",
+			      entry->name, refname);
+			return 0;
 		}
 	}
 	return 1;
-- 
1.7.7

^ permalink raw reply related

* [PATCH 11/12] names_conflict(): simplify implementation
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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


Save a bunch of lines of code and a couple of strlen() calls.

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

diff --git a/refs.c b/refs.c
index ada691b..4bc6041 100644
--- a/refs.c
+++ b/refs.c
@@ -1096,19 +1096,10 @@ static int remove_empty_directories(const char *file)
  */
 static int names_conflict(const char *refname1, const char *refname2)
 {
-	int len1 = strlen(refname1);
-	int len2 = strlen(refname2);
-	int cmplen;
-	const char *lead;
-
-	if (len1 < len2) {
-		cmplen = len1;
-		lead = refname2;
-	} else {
-		cmplen = len2;
-		lead = refname1;
-	}
-	return !strncmp(refname1, refname2, cmplen) && lead[cmplen] == '/';
+	for (; *refname1 && *refname1 == *refname2; refname1++, refname2++)
+		;
+	return (*refname1 == '\0' && *refname2 == '/')
+		|| (*refname1 == '/' && *refname2 == '\0');
 }
 
 /*
-- 
1.7.7

^ permalink raw reply related

* [PATCH 12/12] is_refname_available(): reimplement using do_for_each_ref_in_array()
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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

This implementation will survive upcoming changes to the ref_array
data structure.

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

diff --git a/refs.c b/refs.c
index 4bc6041..c41d995 100644
--- a/refs.c
+++ b/refs.c
@@ -1102,6 +1102,25 @@ static int names_conflict(const char *refname1, const char *refname2)
 		|| (*refname1 == '/' && *refname2 == '\0');
 }
 
+struct name_conflict_cb {
+       const char *refname;
+       const char *oldrefname;
+       const char *conflicting_refname;
+};
+
+static int name_conflict_fn(const char *existingrefname, const unsigned char *sha1,
+			    int flags, void *cb_data)
+{
+       struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
+       if (data->oldrefname && !strcmp(data->oldrefname, existingrefname))
+	       return 0;
+       if (names_conflict(data->refname, existingrefname)) {
+	       data->conflicting_refname = existingrefname;
+	       return 1;
+       }
+       return 0;
+}
+
 /*
  * Return true iff a reference named refname could be created without
  * conflicting with the name of an existing reference.  If oldrefname
@@ -1112,18 +1131,19 @@ static int names_conflict(const char *refname1, const char *refname2)
 static int is_refname_available(const char *refname, const char *oldrefname,
 				struct ref_array *array)
 {
-	int i;
-	for (i = 0; i < array->nr; i++ ) {
-		struct ref_entry *entry = array->refs[i];
-		if (oldrefname && !strcmp(oldrefname, entry->name))
-			continue;
-		if (names_conflict(refname, entry->name)) {
-			error("'%s' exists; cannot create '%s'",
-			      entry->name, refname);
-			return 0;
-		}
-	}
-	return 1;
+       struct name_conflict_cb data;
+       data.refname = refname;
+       data.oldrefname = oldrefname;
+       data.conflicting_refname = NULL;
+
+       if (do_for_each_ref_in_array(array, 0, "", name_conflict_fn,
+				    0, DO_FOR_EACH_INCLUDE_BROKEN,
+				    &data)) {
+	       error("'%s' exists; cannot create '%s'",
+		     data.conflicting_refname, refname);
+	       return 0;
+       }
+       return 1;
 }
 
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
-- 
1.7.7

^ permalink raw reply related

* Re: [RFC 04/13] add_ref(): move the call of check_refname_format() to callers
From: Junio C Hamano @ 2011-10-19 21:49 UTC (permalink / raw)
  To: mhagger
  Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <1319057716-28094-5-git-send-email-mhagger@alum.mit.edu>

mhagger@alum.mit.edu writes:

> I'm still not clear on how extra_refs are used.  Are they generated
> from local refs or are they generated from remote refs?  If the
> latter, then it is probably irresponsible not to do *some* sanity
> checking in add_extra_ref() to prevent any chance of refnames like
> "../../../etc/passwd".

No, add_extra_ref() already tells us what their values are, these are
never used to actually read from filesystem. Their refname field has
almost no value other than for debugging and we probably shouldn't even
insist on uniqueness among extra refs or for that matter collision with
the real refs. As I mentioned in an earlier message, their only raison
d'être is to be found by for_each_ref() that feeds revision machinery with
up to which commits we know we have complete histories for. A sample call
chain looks like this:

 - cmd_clone()
  - setup_reference()
   - add_one_reference()
     - add_extra_ref()
       adds refs in other repositories we borrow from as "extra"
  - transport_fetch_refs()
   - fetch_refs_via_pack()
    - fetch_pack()
     - do_fetch_pack()
      - find_common()
       - for_each_ref(rev_list_insert_ref)

That way find_common() thinks histories leading to these extra refs are
already complete on our end (i.e. we have all the necessary objects), and
by subtracting that from what we are asking from the other end, we can
reduce the amount of history that needs to be transferred.

^ permalink raw reply

* Re: Compiling on Windows
From: Philip Oakley @ 2011-10-19 21:46 UTC (permalink / raw)
  To: Andrew Ardill, Vincent van Ravesteijn; +Cc: Git MsysGit, git
In-Reply-To: <4E9E811C.10205@lyx.org>

From: "Vincent van Ravesteijn" <vfr@lyx.org>
> Op 18-10-2011 6:08, Andrew Ardill schreef:
>> Hi list, I have been searching for details on what is required to
>> compile on Windows, but haven't found anything conclusive. Perhaps
>> there is something on the wiki, but unfortunately it is down at the
>> moment.
>>
>> Can anyone point me in the right direction? I would like to be able to
>> compile and test topic branches, and perhaps even do some dev work on
>> my windows machine.
>
> I once wrote a little step-by-step tutorial on how to compile the native 
> Windows Git with MSVC (Express).
>
> http://blog.vfrconsultancy.nl/#post0

The blog post filled in a few gaps in the Msysgit README instructions about 
where to place the various downloads described.

>
> Be aware that git runs without apparent problems, but that all 
> functionality written in shell scripts can't be used.
>
> HTH,
>
> Vincent
> --

Many thanks, it saved me asking.
Philip 

^ permalink raw reply

* [PATCH 07/12] do_for_each_ref_in_array(): new function
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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

Extract function do_for_each_ref_in_array() from do_for_each_ref().

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

diff --git a/refs.c b/refs.c
index 7aef76c..4e60edb 100644
--- a/refs.c
+++ b/refs.c
@@ -696,21 +696,31 @@ fallback:
 	return -1;
 }
 
+static int do_for_each_ref_in_array(struct ref_array *array, int offset,
+				    const char *base,
+				    each_ref_fn fn, int trim, int flags, void *cb_data)
+{
+	int i;
+	for (i = offset; i < array->nr; i++) {
+		int retval = do_one_ref(base, fn, trim, flags, cb_data, array->refs[i]);
+		if (retval)
+			return retval;
+	}
+	return 0;
+}
+
 static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
 			   int trim, int flags, void *cb_data)
 {
-	int retval = 0, i, p = 0, l = 0;
+	int retval = 0, p = 0, l = 0;
 	struct ref_cache *refs = get_ref_cache(submodule);
 	struct ref_array *packed = get_packed_refs(refs);
 	struct ref_array *loose = get_loose_refs(refs);
 
-	struct ref_array *extra = &extra_refs;
-
-	for (i = 0; i < extra->nr; i++) {
-		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
-		if (retval)
-			goto end_each;
-	}
+	retval = do_for_each_ref_in_array(&extra_refs, 0,
+					  base, fn, trim, flags, cb_data);
+	if (retval)
+		goto end_each;
 
 	while (p < packed->nr && l < loose->nr) {
 		struct ref_entry *entry;
@@ -730,14 +740,11 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
 	}
 
 	if (l < loose->nr) {
-		p = l;
-		packed = loose;
-	}
-
-	for (; p < packed->nr; p++) {
-		retval = do_one_ref(base, fn, trim, flags, cb_data, packed->refs[p]);
-		if (retval)
-			goto end_each;
+		retval = do_for_each_ref_in_array(loose, l,
+						  base, fn, trim, flags, cb_data);
+	} else {
+		retval = do_for_each_ref_in_array(packed, p,
+						  base, fn, trim, flags, cb_data);
 	}
 
 end_each:
-- 
1.7.7

^ permalink raw reply related

* [PATCH 04/12] create_ref_entry(): extract function from add_ref()
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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

Separate the creation of the ref_entry from its addition to a ref_array.

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

diff --git a/refs.c b/refs.c
index 752938c..acb098c 100644
--- a/refs.c
+++ b/refs.c
@@ -57,27 +57,33 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
 	return line;
 }
 
-/* Add a ref_entry to the end of the ref_array (unsorted). */
-static void add_ref(const char *refname, const unsigned char *sha1,
-		    int flag, struct ref_array *refs,
-		    struct ref_entry **new_entry)
+static struct ref_entry *create_ref_entry(const char *refname,
+					  const unsigned char *sha1, int flag)
 {
 	int len;
-	struct ref_entry *entry;
+	struct ref_entry *ref;
 
-	/* Allocate it and add it in.. */
-	len = strlen(refname) + 1;
-	entry = xmalloc(sizeof(struct ref_entry) + len);
-	hashcpy(entry->sha1, sha1);
-	hashclr(entry->peeled);
 	if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
 		die("Reference has invalid format: '%s'", refname);
-	memcpy(entry->name, refname, len);
-	entry->flag = flag;
-	if (new_entry)
-		*new_entry = entry;
+	len = strlen(refname) + 1;
+	ref = xmalloc(sizeof(struct ref_entry) + len);
+	hashcpy(ref->sha1, sha1);
+	hashclr(ref->peeled);
+	memcpy(ref->name, refname, len);
+	ref->flag = flag;
+	return ref;
+}
+
+/* Add a ref_entry to the end of the ref_array (unsorted). */
+static void add_ref(const char *refname, const unsigned char *sha1,
+		    int flag, struct ref_array *refs,
+		    struct ref_entry **new_ref)
+{
+	struct ref_entry *ref = create_ref_entry(refname, sha1, flag);
+	if (new_ref)
+		*new_ref = ref;
 	ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
-	refs->refs[refs->nr++] = entry;
+	refs->refs[refs->nr++] = ref;
 }
 
 static int ref_entry_cmp(const void *a, const void *b)
-- 
1.7.7

^ permalink raw reply related

* [PATCH 06/12] do_for_each_ref(): correctly terminate while processesing extra_refs
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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

If the user-supplied function returns a nonzero value while processing
extra_refs, terminate without processing the rest of the list.

This probably has no practical importance, but makes the handling of
extra_refs a little bit more consistent with the handling of other
refs.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

 refs.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index ae90993..7aef76c 100644
--- a/refs.c
+++ b/refs.c
@@ -706,8 +706,11 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
 
 	struct ref_array *extra = &extra_refs;
 
-	for (i = 0; i < extra->nr; i++)
+	for (i = 0; i < extra->nr; i++) {
 		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
+		if (retval)
+			goto end_each;
+	}
 
 	while (p < packed->nr && l < loose->nr) {
 		struct ref_entry *entry;
-- 
1.7.7

^ permalink raw reply related

* [PATCH 09/12] repack_without_ref(): reimplement using do_for_each_ref_in_array()
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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


It costs a bit of boilerplate, but it means that the function can be
ignorant of how cached refs are stored.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

 refs.c |   46 ++++++++++++++++++++++++++++------------------
 1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/refs.c b/refs.c
index cd3acf8..7c2bcab 100644
--- a/refs.c
+++ b/refs.c
@@ -1213,36 +1213,46 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,
 	return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
 }
 
+struct repack_without_ref_sb {
+	const char *refname;
+	int fd;
+};
+
+static int repack_without_ref_fn(const char *refname, const unsigned char *sha1,
+				 int flags, void *cb_data)
+{
+	struct repack_without_ref_sb *data = cb_data;
+	char line[PATH_MAX + 100];
+	int len;
+
+	if (!strcmp(data->refname, refname))
+		return 0;
+	len = snprintf(line, sizeof(line), "%s %s\n",
+		       sha1_to_hex(sha1), refname);
+	/* this should not happen but just being defensive */
+	if (len > sizeof(line))
+		die("too long a refname '%s'", refname);
+	write_or_die(data->fd, line, len);
+	return 0;
+}
+
 static struct lock_file packlock;
 
 static int repack_without_ref(const char *refname)
 {
+	struct repack_without_ref_sb data;
 	struct ref_array *packed;
-	int fd, i;
 
 	packed = get_packed_refs(get_ref_cache(NULL));
 	if (search_ref_array(packed, refname) == NULL)
 		return 0;
-	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0) {
+	data.refname = refname;
+	data.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
+	if (data.fd < 0) {
 		unable_to_lock_error(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
 	}
-
-	for (i = 0; i < packed->nr; i++) {
-		char line[PATH_MAX + 100];
-		int len;
-		struct ref_entry *ref = packed->refs[i];
-
-		if (!strcmp(refname, ref->name))
-			continue;
-		len = snprintf(line, sizeof(line), "%s %s\n",
-			       sha1_to_hex(ref->sha1), ref->name);
-		/* this should not happen but just being defensive */
-		if (len > sizeof(line))
-			die("too long a refname '%s'", ref->name);
-		write_or_die(fd, line, len);
-	}
+	do_for_each_ref_in_array(packed, 0, "", repack_without_ref_fn, 0, 0, &data);
 	return commit_lock_file(&packlock);
 }
 
-- 
1.7.7

^ permalink raw reply related

* [PATCH 08/12] do_for_each_ref_in_arrays(): new function
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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

Extract function do_for_each_ref_in_arrays() from do_for_each_ref().

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

diff --git a/refs.c b/refs.c
index 4e60edb..cd3acf8 100644
--- a/refs.c
+++ b/refs.c
@@ -709,45 +709,58 @@ static int do_for_each_ref_in_array(struct ref_array *array, int offset,
 	return 0;
 }
 
-static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
-			   int trim, int flags, void *cb_data)
+static int do_for_each_ref_in_arrays(struct ref_array *array1,
+				     struct ref_array *array2,
+				     const char *base, each_ref_fn fn, int trim,
+				     int flags, void *cb_data)
 {
-	int retval = 0, p = 0, l = 0;
-	struct ref_cache *refs = get_ref_cache(submodule);
-	struct ref_array *packed = get_packed_refs(refs);
-	struct ref_array *loose = get_loose_refs(refs);
-
-	retval = do_for_each_ref_in_array(&extra_refs, 0,
-					  base, fn, trim, flags, cb_data);
-	if (retval)
-		goto end_each;
+	int retval;
+	int i1 = 0, i2 = 0;
 
-	while (p < packed->nr && l < loose->nr) {
-		struct ref_entry *entry;
-		int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
-		if (!cmp) {
-			p++;
+	while (1) {
+		struct ref_entry *e1, *e2;
+		int cmp;
+		if (i1 == array1->nr) {
+			return do_for_each_ref_in_array(array2, i2,
+							base, fn, trim, flags, cb_data);
+		}
+		if (i2 == array2->nr) {
+			return do_for_each_ref_in_array(array1, i1,
+							base, fn, trim, flags, cb_data);
+		}
+		e1 = array1->refs[i1];
+		e2 = array2->refs[i2];
+		cmp = strcmp(e1->name, e2->name);
+		if (cmp == 0) {
+			/* Two refs with the same name; ignore the one from array1. */
+			i1++;
 			continue;
 		}
-		if (cmp > 0) {
-			entry = loose->refs[l++];
+		if (cmp < 0) {
+			retval = do_one_ref(base, fn, trim, flags, cb_data, e1);
+			i1++;
 		} else {
-			entry = packed->refs[p++];
+			retval = do_one_ref(base, fn, trim, flags, cb_data, e2);
+			i2++;
 		}
-		retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
 		if (retval)
-			goto end_each;
+			return retval;
 	}
+}
 
-	if (l < loose->nr) {
-		retval = do_for_each_ref_in_array(loose, l,
-						  base, fn, trim, flags, cb_data);
-	} else {
-		retval = do_for_each_ref_in_array(packed, p,
-						  base, fn, trim, flags, cb_data);
-	}
+static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
+			   int trim, int flags, void *cb_data)
+{
+	int retval = 0;
+	struct ref_cache *refs = get_ref_cache(submodule);
+
+	retval = do_for_each_ref_in_array(&extra_refs, 0,
+					  base, fn, trim, flags, cb_data);
+	if (!retval)
+		retval = do_for_each_ref_in_arrays(get_packed_refs(refs),
+						   get_loose_refs(refs),
+						   base, fn, trim, flags, cb_data);
 
-end_each:
 	current_ref = NULL;
 	return retval;
 }
-- 
1.7.7

^ permalink raw reply related

* [PATCH 05/12] add_ref(): take a (struct ref_entry *) parameter
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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

Take a pointer to the ref_entry to add to the array, rather than
creating the ref_entry within the function.  This opens the way to
having multiple kinds of ref_entries.

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

diff --git a/refs.c b/refs.c
index acb098c..ae90993 100644
--- a/refs.c
+++ b/refs.c
@@ -75,13 +75,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
 }
 
 /* Add a ref_entry to the end of the ref_array (unsorted). */
-static void add_ref(const char *refname, const unsigned char *sha1,
-		    int flag, struct ref_array *refs,
-		    struct ref_entry **new_ref)
+static void add_ref(struct ref_array *refs, struct ref_entry *ref)
 {
-	struct ref_entry *ref = create_ref_entry(refname, sha1, flag);
-	if (new_ref)
-		*new_ref = ref;
 	ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
 	refs->refs[refs->nr++] = ref;
 }
@@ -266,7 +261,8 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
 
 		refname = parse_ref_line(refline, sha1);
 		if (refname) {
-			add_ref(refname, sha1, flag, array, &last);
+			last = create_ref_entry(refname, sha1, flag);
+			add_ref(array, last);
 			continue;
 		}
 		if (last &&
@@ -281,7 +277,7 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
 
 void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
 {
-	add_ref(refname, sha1, flag, &extra_refs, NULL);
+	add_ref(&extra_refs, create_ref_entry(refname, sha1, flag));
 }
 
 void clear_extra_refs(void)
@@ -368,7 +364,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
 					hashclr(sha1);
 					flag |= REF_BROKEN;
 				}
-			add_ref(refname, sha1, flag, array, NULL);
+			add_ref(array, create_ref_entry(refname, sha1, flag));
 		}
 		free(refname);
 		closedir(dir);
-- 
1.7.7

^ permalink raw reply related

* [PATCH 03/12] parse_ref_line(): add a check that the refname is properly formatted
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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


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

diff --git a/refs.c b/refs.c
index 431d3c6..752938c 100644
--- a/refs.c
+++ b/refs.c
@@ -51,6 +51,9 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
 		return NULL;
 	line[len] = 0;
 
+	if (check_refname_format(line, REFNAME_ALLOW_ONELEVEL))
+		return NULL;
+
 	return line;
 }
 
-- 
1.7.7

^ permalink raw reply related

* [PATCH 01/12] Rename another local variable name -> refname
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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


Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

I missed this one earlier.  (There are probably others too.  I hope
you don't feel that I'm morally obliged to find every single analogous
example...)

 refs.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/refs.c b/refs.c
index 7e6cea5..f6752a4 100644
--- a/refs.c
+++ b/refs.c
@@ -317,11 +317,11 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
 	if (dir) {
 		struct dirent *de;
 		int baselen = strlen(base);
-		char *ref = xmalloc(baselen + 257);
+		char *refname = xmalloc(baselen + 257);
 
-		memcpy(ref, base, baselen);
+		memcpy(refname, base, baselen);
 		if (baselen && base[baselen-1] != '/')
-			ref[baselen++] = '/';
+			refname[baselen++] = '/';
 
 		while ((de = readdir(dir)) != NULL) {
 			unsigned char sha1[20];
@@ -337,31 +337,31 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
 				continue;
 			if (has_extension(de->d_name, ".lock"))
 				continue;
-			memcpy(ref + baselen, de->d_name, namelen+1);
+			memcpy(refname + baselen, de->d_name, namelen+1);
 			refdir = *refs->name
-				? git_path_submodule(refs->name, "%s", ref)
-				: git_path("%s", ref);
+				? git_path_submodule(refs->name, "%s", refname)
+				: git_path("%s", refname);
 			if (stat(refdir, &st) < 0)
 				continue;
 			if (S_ISDIR(st.st_mode)) {
-				get_ref_dir(refs, ref, array);
+				get_ref_dir(refs, refname, array);
 				continue;
 			}
 			if (*refs->name) {
 				hashclr(sha1);
 				flag = 0;
-				if (resolve_gitlink_ref(refs->name, ref, sha1) < 0) {
+				if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
 					hashclr(sha1);
 					flag |= REF_BROKEN;
 				}
 			} else
-				if (!resolve_ref(ref, sha1, 1, &flag)) {
+				if (!resolve_ref(refname, sha1, 1, &flag)) {
 					hashclr(sha1);
 					flag |= REF_BROKEN;
 				}
-			add_ref(ref, sha1, flag, array, NULL);
+			add_ref(refname, sha1, flag, array, NULL);
 		}
-		free(ref);
+		free(refname);
 		closedir(dir);
 	}
 }
-- 
1.7.7

^ permalink raw reply related

* [PATCH 02/12] repack_without_ref(): remove temporary
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>

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


Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

Trivial and boring.

 refs.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index f6752a4..431d3c6 100644
--- a/refs.c
+++ b/refs.c
@@ -1190,12 +1190,10 @@ static struct lock_file packlock;
 static int repack_without_ref(const char *refname)
 {
 	struct ref_array *packed;
-	struct ref_entry *ref;
 	int fd, i;
 
 	packed = get_packed_refs(get_ref_cache(NULL));
-	ref = search_ref_array(packed, refname);
-	if (ref == NULL)
+	if (search_ref_array(packed, refname) == NULL)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
 	if (fd < 0) {
@@ -1206,8 +1204,7 @@ static int repack_without_ref(const char *refname)
 	for (i = 0; i < packed->nr; i++) {
 		char line[PATH_MAX + 100];
 		int len;
-
-		ref = packed->refs[i];
+		struct ref_entry *ref = packed->refs[i];
 
 		if (!strcmp(refname, ref->name))
 			continue;
-- 
1.7.7

^ permalink raw reply related

* [PATCH 00/12] Use refs API more consistently
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

This patch series primarily has to do with using the refs API more
consistently within refs.c itself.  We want to minimize the surface
area for accessing the ref_cache data structure so that when it is
changed into a tree (we're on the verge of that change now, I
promise!) we don't have to change callers more than necessary.  There
is also a bit of an eat-your-own-dogfood thing going on here; if
for_each_ref() and its friends are good enough for "outsiders", they
should be good enough for most internal use.

The first two patches are trivial preparation cleanups.

The third verifies that refnames read from packed-ref files are
properly formatted.  When the REFNAME_FULL changes becomes available,
this check can be tightened up.

The do_for_each_ref_in_{array,arrays}() functions provide an
iterator-like interface to iterating over a single ref_array and
iterating over two ref_arrays in parallel.  These are useful
abstractions in and of themselves (the former is used to reimplement
repack_without_ref() and is_refname_available() in patches 9 and 12
respectively).  And they will be even more useful when references are
stored hierarchically.

This series applies on top of mh/ref-api-2.  It conflicts with the
"Checking full vs. partial refnames" series that I just submitted but
not in any fundamental way.  I'll be happy to reconcile them for you
in a later round when it is clear if and when each series is accepted.

Michael Haggerty (12):
  Rename another local variable name -> refname
  repack_without_ref(): remove temporary
  parse_ref_line(): add a check that the refname is properly formatted
  create_ref_entry(): extract function from add_ref()
  add_ref(): take a (struct ref_entry *) parameter
  do_for_each_ref(): correctly terminate while processesing extra_refs
  do_for_each_ref_in_array(): new function
  do_for_each_ref_in_arrays(): new function
  repack_without_ref(): reimplement using do_for_each_ref_in_array()
  names_conflict(): new function, extracted from is_refname_available()
  names_conflict(): simplify implementation
  is_refname_available(): reimplement using do_for_each_ref_in_array()

 refs.c |  259 ++++++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 162 insertions(+), 97 deletions(-)

-- 
1.7.7

^ permalink raw reply

* [PATCH 6/6] revert: simplify communicating command-line arguments
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>

From: Jonathan Nieder <jrnieder@gmail.com>

Currently, command-line arguments are communicated using (argc, argv)
until a prepare_revs() turns it into a terse structure.  However,
since we plan to expose the cherry-picking machinery through a public
API in the future, we want callers to be able to call in with a
filled-in structure.  For the revert builtin, this means that the
chief argument parser, parse_args(), should parse into such a
structure.  Make this change.

[rr: minor improvements, commit message]

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c |   53 +++++++++++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 0201e98..576b9e7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,14 @@ struct replay_opts {
 	int allow_rerere_auto;
 
 	int mainline;
-	int commit_argc;
-	const char **commit_argv;
 
 	/* Merge strategy */
 	const char *strategy;
 	const char **xopts;
 	size_t xopts_nr, xopts_alloc;
+
+	/* Only used by REPLAY_NONE */
+	struct rev_info *revs;
 };
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -161,9 +162,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			die(_("program error"));
 	}
 
-	opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
-					PARSE_OPT_KEEP_ARGV0 |
-					PARSE_OPT_KEEP_UNKNOWN);
+	argc = parse_options(argc, argv, NULL, options, usage_str,
+			PARSE_OPT_KEEP_ARGV0 |
+			PARSE_OPT_KEEP_UNKNOWN);
 
 	/* Check for incompatible subcommands */
 	verify_opt_mutually_compatible(me,
@@ -198,9 +199,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				NULL);
 	}
 
-	else if (opts->commit_argc < 2)
-		usage_with_options(usage_str, options);
-
 	if (opts->allow_ff)
 		verify_opt_compatible(me, "--ff",
 				"--signoff", opts->signoff,
@@ -208,7 +206,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				"-x", opts->record_origin,
 				"--edit", opts->edit,
 				NULL);
-	opts->commit_argv = argv;
+
+	if (opts->subcommand == REPLAY_NONE) {
+		opts->revs = xmalloc(sizeof(*opts->revs));
+		init_revisions(opts->revs, NULL);
+		opts->revs->no_walk = 1;
+		if (argc < 2)
+			usage_with_options(usage_str, options);
+		argc = setup_revisions(argc, argv, opts->revs, NULL);
+	} else
+		opts->revs = NULL;
+
+	/* Forbid stray command-line arguments */
+	if (argc > 1)
+		usage_with_options(usage_str, options);
 }
 
 struct commit_message {
@@ -605,23 +616,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
 	return res;
 }
 
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
 {
-	int argc;
-
-	init_revisions(revs, NULL);
-	revs->no_walk = 1;
 	if (opts->action != REPLAY_REVERT)
-		revs->reverse = 1;
+		opts->revs->reverse ^= 1;
 
-	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
-	if (argc > 1)
-		usage(*revert_or_cherry_pick_usage(opts));
-
-	if (prepare_revision_walk(revs))
+	if (prepare_revision_walk(opts->revs))
 		die(_("revision walk setup failed"));
 
-	if (!revs->commits)
+	if (!opts->revs->commits)
 		die(_("empty commit set passed"));
 }
 
@@ -809,14 +812,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
-	struct rev_info revs;
 	struct commit *commit;
 	struct replay_insn_list **next;
 
-	prepare_revs(&revs, opts);
+	prepare_revs(opts);
 
 	next = todo_list;
-	while ((commit = get_revision(&revs)))
+	while ((commit = get_revision(opts->revs)))
 		next = replay_insn_list_append(opts->action, commit, next);
 }
 
@@ -940,6 +942,9 @@ static int pick_revisions(struct replay_opts *opts)
 	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
+	if (opts->subcommand == REPLAY_NONE)
+		assert(opts->revs);
+
 	read_and_refresh_cache(opts);
 
 	/*
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 5/6] revert: allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>

Parse the instruction sheet in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming that all instructions use
the same action.  Now you can do:

  pick fdc0b12 picked
  revert 965fed4 anotherpick

For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
This patch lays the foundation for extending the parser to support
more actions so 'git rebase -i' can reuse this machinery in the
future.  While at it, also improve the error messages reported by the
insn sheet parser.

Helped-by: Jonathan Nieder <jrnider@gmail.com>
Acked-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c                |  130 +++++++++++++++++++--------------------
 sequencer.h                     |    8 +++
 t/t3510-cherry-pick-sequence.sh |   58 +++++++++++++++++
 3 files changed, 129 insertions(+), 67 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 8e34dc0..0201e98 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-enum replay_action { REVERT, CHERRY_PICK };
 enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
 
 struct replay_opts {
@@ -68,14 +67,14 @@ struct replay_opts {
 
 static const char *action_name(const struct replay_opts *opts)
 {
-	return opts->action == REVERT ? "revert" : "cherry-pick";
+	return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
 }
 
 static char *get_encoding(const char *message);
 
 static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
 {
-	return opts->action == REVERT ? revert_usage : cherry_pick_usage;
+	return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
 }
 
 static int option_parse_x(const struct option *opt,
@@ -152,7 +151,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		OPT_END(),
 	};
 
-	if (opts->action == CHERRY_PICK) {
+	if (opts->action == REPLAY_PICK) {
 		struct option cp_extra[] = {
 			OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
 			OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -346,7 +345,7 @@ static int error_dirty_index(struct replay_opts *opts)
 		return error_resolve_conflict(action_name(opts));
 
 	/* Different translation strings for cherry-pick and revert */
-	if (opts->action == CHERRY_PICK)
+	if (opts->action == REPLAY_PICK)
 		error(_("Your local changes would be overwritten by cherry-pick."));
 	else
 		error(_("Your local changes would be overwritten by revert."));
@@ -450,7 +449,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
 	return run_command_v_opt(args, RUN_GIT_CMD);
 }
 
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+			struct replay_opts *opts)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
@@ -525,7 +525,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 
 	defmsg = git_pathdup("MERGE_MSG");
 
-	if (opts->action == REVERT) {
+	if (action == REPLAY_REVERT) {
 		base = commit;
 		base_label = msg.label;
 		next = parent;
@@ -568,7 +568,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 			write_cherry_pick_head(commit);
 	}
 
-	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
 					 head, &msgbuf, opts);
 		write_message(&msgbuf, defmsg);
@@ -587,7 +587,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	}
 
 	if (res) {
-		error(opts->action == REVERT
+		error(action == REPLAY_REVERT
 		      ? _("could not revert %s... %s")
 		      : _("could not apply %s... %s"),
 		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -611,7 +611,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
 
 	init_revisions(revs, NULL);
 	revs->no_walk = 1;
-	if (opts->action != REVERT)
+	if (opts->action != REPLAY_REVERT)
 		revs->reverse = 1;
 
 	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -657,49 +657,53 @@ static void read_and_refresh_cache(struct replay_opts *opts)
  *     assert(commit_list_count(list) == 2);
  *     return list;
  */
-static struct commit_list **commit_list_append(struct commit *commit,
-					       struct commit_list **next)
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+						struct commit *operand,
+						struct replay_insn_list **next)
 {
-	struct commit_list *new = xmalloc(sizeof(struct commit_list));
-	new->item = commit;
+	struct replay_insn_list *new = xmalloc(sizeof(*new));
+	new->action = action;
+	new->operand = operand;
 	*next = new;
 	new->next = NULL;
 	return &new->next;
 }
 
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
-		struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
 {
-	struct commit_list *cur = NULL;
-	const char *sha1_abbrev = NULL;
-	const char *action_str = opts->action == REVERT ? "revert" : "pick";
-	const char *subject;
-	int subject_len;
+	struct replay_insn_list *cur;
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
-		subject_len = find_commit_subject(cur->item->buffer, &subject);
+		const char *sha1_abbrev, *action_str, *subject;
+		int subject_len;
+
+		action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+		sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+		subject_len = find_commit_subject(cur->operand->buffer, &subject);
 		strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
 			subject_len, subject);
 	}
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
 {
 	unsigned char commit_sha1[20];
-	enum replay_action action;
 	char *end_of_object_name;
 	int saved, status;
 
 	if (!prefixcmp(bol, "pick ")) {
-		action = CHERRY_PICK;
+		item->action = REPLAY_PICK;
 		bol += strlen("pick ");
 	} else if (!prefixcmp(bol, "revert ")) {
-		action = REVERT;
+		item->action = REPLAY_REVERT;
 		bol += strlen("revert ");
-	} else
-		return NULL;
+	} else {
+		size_t len = strchrnul(bol, '\n') - bol;
+		if (len > 255)
+			len = 255;
+		return error(_("Unrecognized action: %.*s"), (int)len, bol);
+	}
 
 	end_of_object_name = bol + strcspn(bol, " \n");
 	saved = *end_of_object_name;
@@ -707,37 +711,29 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
 	status = get_sha1(bol, commit_sha1);
 	*end_of_object_name = saved;
 
-	/*
-	 * Verify that the action matches up with the one in
-	 * opts; we don't support arbitrary instructions
-	 */
-	if (action != opts->action) {
-		const char *action_str;
-		action_str = action == REVERT ? "revert" : "cherry-pick";
-		error(_("Cannot %s during a %s"), action_str, action_name(opts));
-		return NULL;
-	}
-
 	if (status < 0)
-		return NULL;
+		return error(_("Malformed object name: %s"), bol);
 
-	return lookup_commit_reference(commit_sha1);
+	item->operand = lookup_commit_reference(commit_sha1);
+	if (!item->operand)
+		return error(_("Not a valid commit: %s"), bol);
+
+	item->next = NULL;
+	return 0;
 }
 
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
-			struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
 {
-	struct commit_list **next = todo_list;
-	struct commit *commit;
+	struct replay_insn_list **next = todo_list;
+	struct replay_insn_list item = {0, NULL, NULL};
 	char *p = buf;
 	int i;
 
 	for (i = 1; *p; i++) {
 		char *eol = strchrnul(p, '\n');
-		commit = parse_insn_line(p, eol, opts);
-		if (!commit)
-			return error(_("Could not parse line %d."), i);
-		next = commit_list_append(commit, next);
+		if (parse_insn_line(p, eol, &item) < 0)
+			return error(_("on line %d."), i);
+		next = replay_insn_list_append(item.action, item.operand, next);
 		p = *eol ? eol + 1 : eol;
 	}
 	if (!*todo_list)
@@ -745,8 +741,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 	return 0;
 }
 
-static void read_populate_todo(struct commit_list **todo_list,
-			struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	struct strbuf buf = STRBUF_INIT;
@@ -762,7 +757,7 @@ static void read_populate_todo(struct commit_list **todo_list,
 	}
 	close(fd);
 
-	res = parse_insn_buffer(buf.buf, todo_list, opts);
+	res = parse_insn_buffer(buf.buf, todo_list);
 	strbuf_release(&buf);
 	if (res)
 		die(_("Unusable instruction sheet: %s"), todo_file);
@@ -811,18 +806,18 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 		die(_("Malformed options sheet: %s"), opts_file);
 }
 
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
 	struct rev_info revs;
 	struct commit *commit;
-	struct commit_list **next;
+	struct replay_insn_list **next;
 
 	prepare_revs(&revs, opts);
 
 	next = todo_list;
 	while ((commit = get_revision(&revs)))
-		next = commit_list_append(commit, next);
+		next = replay_insn_list_append(opts->action, commit, next);
 }
 
 static int create_seq_dir(void)
@@ -851,7 +846,7 @@ static void save_head(const char *head)
 		die(_("Error wrapping up %s."), head_file);
 }
 
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	static struct lock_file todo_lock;
@@ -859,7 +854,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 	int fd;
 
 	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
-	if (format_todo(&buf, todo_list, opts) < 0)
+	if (format_todo(&buf, todo_list) < 0)
 		die(_("Could not format %s."), todo_file);
 	if (write_in_full(fd, buf.buf, buf.len) < 0) {
 		strbuf_release(&buf);
@@ -903,9 +898,10 @@ static void save_opts(struct replay_opts *opts)
 	}
 }
 
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list,
+			struct replay_opts *opts)
 {
-	struct commit_list *cur;
+	struct replay_insn_list *cur;
 	int res;
 
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -915,8 +911,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 	read_and_refresh_cache(opts);
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		save_todo(cur, opts);
-		res = do_pick_commit(cur->item, opts);
+		save_todo(cur);
+		res = do_pick_commit(cur->operand, cur->action, opts);
 		if (res) {
 			if (!cur->next)
 				/*
@@ -941,7 +937,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 
 static int pick_revisions(struct replay_opts *opts)
 {
-	struct commit_list *todo_list = NULL;
+	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
 	read_and_refresh_cache(opts);
@@ -958,7 +954,7 @@ static int pick_revisions(struct replay_opts *opts)
 		if (!file_exists(git_path(SEQ_TODO_FILE)))
 			goto error;
 		read_populate_opts(&opts);
-		read_populate_todo(&todo_list, opts);
+		read_populate_todo(&todo_list);
 
 		/* Verify that the conflict has been resolved */
 		if (!index_differs_from("HEAD", 0))
@@ -978,7 +974,7 @@ static int pick_revisions(struct replay_opts *opts)
 			return -1;
 		}
 		if (get_sha1("HEAD", sha1)) {
-			if (opts->action == REVERT)
+			if (opts->action == REPLAY_REVERT)
 				return error(_("Can't revert as initial commit"));
 			return error(_("Can't cherry-pick into empty head"));
 		}
@@ -998,7 +994,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	if (isatty(0))
 		opts.edit = 1;
-	opts.action = REVERT;
+	opts.action = REPLAY_REVERT;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
 	res = pick_revisions(&opts);
@@ -1013,7 +1009,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	int res;
 
 	memset(&opts, 0, sizeof(opts));
-	opts.action = CHERRY_PICK;
+	opts.action = REPLAY_PICK;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
 	res = pick_revisions(&opts);
diff --git a/sequencer.h b/sequencer.h
index 905d295..f4db257 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,14 @@
 #define SEQ_TODO_FILE	"sequencer/todo"
 #define SEQ_OPTS_FILE	"sequencer/opts"
 
+enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+
+struct replay_insn_list {
+	enum replay_action action;
+	struct commit *operand;
+	struct replay_insn_list *next;
+};
+
 /*
  * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
  * any errors.  Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 39b55c1..4b12244 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -239,4 +239,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
 	test_line_count = 4 commits
 '
 
+test_expect_success 'revert --continue continues after cherry-pick' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	git revert --continue &&
+	test_path_is_missing .git/sequencer &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	oldsha=`git rev-parse --short HEAD~1` &&
+	echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 4/6] revert: make commit subjects in insn sheet optional
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>

Change the instruction sheet format subtly so that the subject of the
commit message that follows the object name is optional.  As a result,
an instruction sheet like this is now perfectly valid:

  pick 35b0426
  pick fbd5bbcbc2e
  pick 7362160f

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c                |   37 ++++++++++++++++---------------------
 t/t3510-cherry-pick-sequence.sh |   14 ++++++++++++++
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 474dda1..8e34dc0 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -685,31 +685,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
 {
 	unsigned char commit_sha1[20];
-	char sha1_abbrev[40];
 	enum replay_action action;
-	int insn_len = 0;
-	char *p, *q;
+	char *end_of_object_name;
+	int saved, status;
 
-	if (!prefixcmp(start, "pick ")) {
+	if (!prefixcmp(bol, "pick ")) {
 		action = CHERRY_PICK;
-		insn_len = strlen("pick");
-		p = start + insn_len + 1;
-	} else if (!prefixcmp(start, "revert ")) {
+		bol += strlen("pick ");
+	} else if (!prefixcmp(bol, "revert ")) {
 		action = REVERT;
-		insn_len = strlen("revert");
-		p = start + insn_len + 1;
+		bol += strlen("revert ");
 	} else
 		return NULL;
 
-	q = strchr(p, ' ');
-	if (!q || q - p + 1 > sizeof(sha1_abbrev))
-		return NULL;
-	q++;
-
-	strlcpy(sha1_abbrev, p, q - p);
+	end_of_object_name = bol + strcspn(bol, " \n");
+	saved = *end_of_object_name;
+	*end_of_object_name = '\0';
+	status = get_sha1(bol, commit_sha1);
+	*end_of_object_name = saved;
 
 	/*
 	 * Verify that the action matches up with the one in
@@ -722,7 +718,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 		return NULL;
 	}
 
-	if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+	if (status < 0)
 		return NULL;
 
 	return lookup_commit_reference(commit_sha1);
@@ -737,13 +733,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 	int i;
 
 	for (i = 1; *p; i++) {
-		commit = parse_insn_line(p, opts);
+		char *eol = strchrnul(p, '\n');
+		commit = parse_insn_line(p, eol, opts);
 		if (!commit)
 			return error(_("Could not parse line %d."), i);
 		next = commit_list_append(commit, next);
-		p = strchrnul(p, '\n');
-		if (*p)
-			p++;
+		p = *eol ? eol + 1 : eol;
 	}
 	if (!*todo_list)
 		return error(_("No commits parsed."));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2113308..39b55c1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -225,4 +225,18 @@ test_expect_success 'malformed instruction sheet 3' '
 	test_must_fail git cherry-pick --continue
 '
 
+test_expect_success 'commit descriptions in insn sheet are optional' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	git rev-list HEAD >commits
+	test_line_count = 4 commits
+'
+
 test_done
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 3/6] revert: fix buffer overflow in insn sheet parser
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>

Check that the commit name argument to a "pick" or "revert" action in
'.git/sequencer/todo' is not too long, to avoid overflowing an
on-stack buffer.  This fixes a regression introduced by 5a5d80f4
(revert: Introduce --continue to continue the operation, 2011-08-04).

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c                |    2 +-
 t/t3510-cherry-pick-sequence.sh |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index acb357d..474dda1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -705,7 +705,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 		return NULL;
 
 	q = strchr(p, ' ');
-	if (!q)
+	if (!q || q - p + 1 > sizeof(sha1_abbrev))
 		return NULL;
 	q++;
 
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..2113308 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -12,6 +12,9 @@ test_description='Test cherry-pick continuation features
 
 . ./test-lib.sh
 
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
 pristine_detach () {
 	git cherry-pick --reset &&
 	git checkout -f "$1^0" &&
@@ -211,4 +214,15 @@ test_expect_success 'malformed instruction sheet 2' '
 	test_must_fail git cherry-pick --continue
 '
 
+test_expect_success 'malformed instruction sheet 3' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "resolved" >foo &&
+	git add foo &&
+	git commit &&
+	sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	test_must_fail git cherry-pick --continue
+'
+
 test_done
-- 
1.7.6.351.gb35ac.dirty

^ 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