* Re: [PATCH] Documentation: filter-branch env-filter example
From: Jeff King @ 2013-02-14 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tadeusz Andrzej Kadłubowski, git
In-Reply-To: <7vy5eqy6z3.fsf@alter.siamese.dyndns.org>
On Thu, Feb 14, 2013 at 12:29:36PM -0800, Junio C Hamano wrote:
> Quote the variable in double-quotes, like this:
>
> if [ "$GIT_AUTHOR_EMAIL" = john@old.example.com ]
>
> Otherwise the comparison will break, if the e-mail part had a
> whitespace in it, or if it were empty, which is an example of a more
> likely situation where you would want to fix commits using a
> procedure like this, no?
Yeah, definitely. If you are cleaning up a broken ident that cannot be
parsed, the failure mode is to put the whole author line in
$GIT_AUTHOR_EMAIL, which would almost certainly include spaces.
> Taking all of the above, the added text may look more like this, I
> think:
>
> The `--env-filter` can be used to modify committer and/or
> author identity. For example, if you found out that your
> commits have wrong identity of yours due to misconfigured
> user.email, you can make correction, before publishing the
> project, like this:
>
> --------------------------------------------------------
> git filter-branch --env-filter '
> if test "$GIT_AUTHOR_EMAIL" = "root@localhost"
> then
> GIT_AUTHOR_EMAIL=yess@example.com
> export GIT_AUTHOR_EMAIL
> fi
> if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
> then
> GIT_COMMITTER_EMAIL=yess@example.com
> export GIT_COMMITTER_EMAIL
> fi
> ' -- --all
> --------------------------------------------------------
That looks better, though there are a few English nits; here's my edited
version:
The `--env-filter` option can be used to modify committer and/or
author identity. For example, if you found out that your
commits have the wrong identity due to a misconfigured
user.email, you can make a correction, before publishing the
project, like this:
> By the way, I left the "export" in; "git filter-branch --help"
> explicitly says that you need to re-export it. But I am not sure if
> they are necessary, especially after 3c730fab2cae (filter-branch:
> use git-sh-setup's ident parsing functions, 2012-10-18) by Peff,
> which added extra "export" to make sure all six identity variables
> are exported. After applying the above rewrite, we may want to do
> the following as a separate, follow-up patch.
I think it has always been the case that we export them after setting
them; just look at the preimage from 3c730fab, and you can see exports
there.
I think the advice in the documentation about re-exporting is because
some versions of the bourne shell will not reliably pass the new version
of the variable when you do this:
VAR=old
export VAR
VAR=new
some_subprocess ;# we see $VAR=old here!
I do not recall ever running across such a shell myself, but rather
hearing about it third-hand in a portability guide somewhere. Apple's
shell documentation seems to indicate that /bin/sh in older versions of
OS X had this behavior:
https://developer.apple.com/library/mac/documentation/opensource/conceptual/shellscripting/shell_scripts/shell_scripts.html#//apple_ref/doc/uid/TP40004268-CH237-SW11
which makes me think that BSD ash may behave that way. It is certainly
not necessary to re-export under bash or dash. I shudder to think what
horrible, 1980's-era behavior is codified in Solaris /bin/sh.
We could explicitly re-export all of the ident variables preemptively
before calling commit-tree, just to save the user the hassle of
remembering to do so. It would be a no-op on sane shells, and I doubt
the runtime cost is very high. I suppose it would break somebody who
explicitly did:
unset GIT_COMMITTER_NAME ;# use the value from user.name
in their env-filter, but that seems like a pretty unlikely corner case.
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: filter-branch env-filter example
From: Jeff King @ 2013-02-14 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tadeusz Andrzej Kadłubowski, git
In-Reply-To: <20130214210910.GA6660@sigill.intra.peff.net>
On Thu, Feb 14, 2013 at 04:09:10PM -0500, Jeff King wrote:
> I think the advice in the documentation about re-exporting is because
> some versions of the bourne shell will not reliably pass the new version
> of the variable when you do this:
>
> VAR=old
> export VAR
> VAR=new
> some_subprocess ;# we see $VAR=old here!
>
> I do not recall ever running across such a shell myself, but rather
> hearing about it third-hand in a portability guide somewhere.
The closest I could find in the autoconf shell guidelines[1] is that the
automagic export marking for incoming variables is not always accurate:
export
The builtin export dubs a shell variable environment
variable. Each update of exported variables corresponds to
an update of the environment variables. Conversely, each
environment variable received by the shell when it is
launched should be imported as a shell variable marked as
exported.
Alas, many shells, such as Solaris 2.5, IRIX 6.3, IRIX 5.2,
AIX 4.1.5, and Digital UNIX 4.0, forget to export the
environment variables they receive. As a result, two
variables coexist: the environment variable and the shell
variable. The following code demonstrates this failure:
#! /bin/sh
echo $FOO
FOO=bar
echo $FOO
exec /bin/sh $0
when run with `FOO=foo' in the environment, these shells
will print alternately `foo' and `bar', although it should
only print `foo' and then a sequence of `bar's.
Therefore you should export again each environment variable
that you update.
I don't know what the behavior would be on such shells of:
#!/bin/sh
echo $FOO
FOO=bar
export FOO
echo $FOO
exec /bin/sh $0
I.e., would the "export" correctly reconcile the local and environment
copies of the variable, or are they forever broken? I don't have such a
system to test on. But that would more closely match what we are doing.
-Peff
[1] https://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins
^ permalink raw reply
* Re: [PATCH] Documentation: filter-branch env-filter example
From: Junio C Hamano @ 2013-02-14 21:51 UTC (permalink / raw)
To: Jeff King; +Cc: Tadeusz Andrzej Kadłubowski, git
In-Reply-To: <20130214210910.GA6660@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think it has always been the case that we export them after setting
> them; just look at the preimage from 3c730fab, and you can see exports
> there.
Yeah, I think the only difference is a broken commit case where sed
expression did not find what it was looking for, in which case we do
not do the export.
> I think the advice in the documentation about re-exporting is because
> some versions of the bourne shell will not reliably pass the new version
> of the variable ...
Ahh, old and painful memory of Solaris days comes back to me. OK,
let's keep the export then.
Thanks.
^ permalink raw reply
* What's cooking in git.git (Feb 2013, #06; Thu, 14)
From: Junio C Hamano @ 2013-02-14 21:51 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'.
A preview of the upcoming release 1.8.2-rc0 is expected to be tagged
late this week. At around that time, we may want to discard
long-stalled topics that did not see activities as well.
You can find the changes described here in the integration branches of the
repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* al/mergetool-printf-fix (2013-02-10) 2 commits
(merged to 'next' on 2013-02-11 at 5f9bc4e)
+ difftool--helper: fix printf usage
+ git-mergetool: print filename when it contains %
* bw/get-tz-offset-perl (2013-02-09) 3 commits
(merged to 'next' on 2013-02-11 at b9c8893)
+ cvsimport: format commit timestamp ourselves without using strftime
+ perl/Git.pm: fix get_tz_offset to properly handle DST boundary cases
+ Move Git::SVN::get_tz to Git::get_tz_offset
"git-cvsimport" and "git-svn" miscomputed TZ offset at DST boundary.
* dg/subtree-fixes (2013-02-05) 6 commits
(merged to 'next' on 2013-02-09 at 8f19ebe)
+ contrib/subtree: make the manual directory if needed
+ contrib/subtree: honor DESTDIR
+ contrib/subtree: fix synopsis
+ contrib/subtree: better error handling for 'subtree add'
+ contrib/subtree: use %B for split subject/body
+ contrib/subtree: remove test number comments
contrib/subtree updates, but here are only the ones that looked
ready.
* jc/extended-fake-ancestor-for-gitlink (2013-02-05) 1 commit
(merged to 'next' on 2013-02-09 at 2d3547b)
+ apply: verify submodule commit object name better
Instead of requiring the full 40-hex object names on the index
line, we can read submodule commit object names from the textual
diff when synthesizing a fake ancestore tree for "git am -3".
* jk/diff-graph-cleanup (2013-02-12) 6 commits
(merged to 'next' on 2013-02-12 at 6e764c1)
+ combine-diff.c: teach combined diffs about line prefix
+ diff.c: use diff_line_prefix() where applicable
+ diff: add diff_line_prefix function
+ diff.c: make constant string arguments const
+ diff: write prefix to the correct file
+ graph: output padding for merge subsequent parents
Refactors a lot of repetitive code sequence from the graph drawing
code and adds it to the combined diff output.
* jk/error-const-return (2013-02-08) 1 commit
(merged to 'next' on 2013-02-11 at ba8dba3)
+ Use __VA_ARGS__ for all of error's arguments
* jx/utf8-printf-width (2013-02-11) 1 commit
(merged to 'next' on 2013-02-11 at 968b4e2)
+ Add utf8_fprintf helper that returns correct number of columns
Use a new helper that prints a message and counts its display width
to align the help messages parse-options produces.
* mg/bisect-doc (2013-02-11) 1 commit
(merged to 'next' on 2013-02-11 at 6125304)
+ git-bisect.txt: clarify that reset quits bisect
* mm/remote-mediawiki-build (2013-02-08) 2 commits
(merged to 'next' on 2013-02-11 at 4ebb902)
+ git-remote-mediawiki: use toplevel's Makefile
+ Makefile: make script-related rules usable from subdirectories
* nd/status-show-in-progress (2013-02-05) 1 commit
(merged to 'next' on 2013-02-11 at 5ffcbc2)
+ status: show the branch name if possible in in-progress info
* tz/perl-styles (2013-02-06) 1 commit
(merged to 'next' on 2013-02-09 at c8cff17)
+ Update CodingGuidelines for Perl
Add coding guidelines for writing Perl scripts for Git.
--------------------------------------------------
[New Topics]
* mk/make-rm-depdirs-could-be-empty (2013-02-13) 1 commit
(merged to 'next' on 2013-02-14 at d966248)
+ Makefile: don't run "rm" without any files
"make COMPUTE_HEADER_DEPENDENCIES=no clean" would try to run "rm
-rf $(dep_dirs)" with an empty dep_dir, but some implementations of
"rm -rf" barf on an empty argument list.
Will merge to 'master'.
* mw/bash-prompt-show-untracked-config (2013-02-13) 3 commits
(merged to 'next' on 2013-02-14 at 809dbcf)
+ t9903: add extra tests for bash.showDirtyState
+ t9903: add tests for bash.showUntrackedFiles
+ shell prompt: add bash.showUntrackedFiles option
Allows skipping the untracked check GIT_PS1_SHOWUNTRACKEDFILES
asks for the git-prompt (in contrib/) per repository.
Will merge to 'master'.
* mg/gpg-interface-using-status (2013-02-14) 5 commits
- pretty: make %GK output the signing key for signed commits
- pretty: parse the gpg status lines rather than the output
- gpg_interface: allow to request status return
- log-tree: rely upon the check in the gpg_interface
- gpg-interface: check good signature in a reliable way
Call "gpg" using the right API when validating the signature on
tags.
* mm/config-intro-in-git-doc (2013-02-14) 1 commit
- git.txt: update description of the configuration mechanism
--------------------------------------------------
[Stalled]
* mb/gitweb-highlight-link-target (2012-12-20) 1 commit
- Highlight the link target line in Gitweb using CSS
Expecting a reroll.
$gmane/211935
* jk/lua-hackery (2012-10-07) 6 commits
- pretty: fix up one-off format_commit_message calls
- Minimum compilation fixup
- Makefile: make "lua" a bit more configurable
- add a "lua" pretty format
- add basic lua infrastructure
- pretty: make some commit-parsing helpers more public
Interesting exercise. When we do this for real, we probably would want
to wrap a commit to make it more like an "object" with methods like
"parents", etc.
* rc/maint-complete-git-p4 (2012-09-24) 1 commit
- Teach git-completion about git p4
Comment from Pete will need to be addressed ($gmane/206172).
* jc/maint-name-rev (2012-09-17) 7 commits
- describe --contains: use "name-rev --algorithm=weight"
- name-rev --algorithm=weight: tests and documentation
- name-rev --algorithm=weight: cache the computed weight in notes
- name-rev --algorithm=weight: trivial optimization
- name-rev: --algorithm option
- name_rev: clarify the logic to assign a new tip-name to a commit
- name-rev: lose unnecessary typedef
"git name-rev" names the given revision based on a ref that can be
reached in the smallest number of steps from the rev, but that is
not useful when the caller wants to know which tag is the oldest one
that contains the rev. This teaches a new mode to the command that
uses the oldest ref among those which contain the rev.
I am not sure if this is worth it; for one thing, even with the help
from notes-cache, it seems to make the "describe --contains" even
slower. Also the command will be unusably slow for a user who does
not have a write access (hence unable to create or update the
notes-cache).
Stalled mostly due to lack of responses.
* jc/xprm-generation (2012-09-14) 1 commit
- test-generation: compute generation numbers and clock skews
A toy to analyze how bad the clock skews are in histories of real
world projects.
Stalled mostly due to lack of responses.
* jc/add-delete-default (2012-08-13) 1 commit
- git add: notice removal of tracked paths by default
"git add dir/" updated modified files and added new files, but does
not notice removed files, which may be "Huh?" to some users. They
can of course use "git add -A dir/", but why should they?
Resurrected from graveyard, as I thought it was a worthwhile thing
to do in the longer term.
Stalled mostly due to lack of responses.
* mb/remote-default-nn-origin (2012-07-11) 6 commits
- Teach get_default_remote to respect remote.default.
- Test that plain "git fetch" uses remote.default when on a detached HEAD.
- Teach clone to set remote.default.
- Teach "git remote" about remote.default.
- Teach remote.c about the remote.default configuration setting.
- Rename remote.c's default_remote_name static variables.
When the user does not specify what remote to interact with, we
often attempt to use 'origin'. This can now be customized via a
configuration variable.
Expecting a reroll.
$gmane/210151
"The first remote becomes the default" bit is better done as a
separate step.
--------------------------------------------------
[Cooking]
* mp/diff-algo-config (2013-01-16) 3 commits
(merged to 'next' on 2013-02-14 at cd765dc)
+ diff: Introduce --diff-algorithm command line option
+ config: Introduce diff.algorithm variable
+ git-completion.bash: Autocomplete --minimal and --histogram for git-diff
Add diff.algorithm configuration so that the user does not type
"diff --histogram".
Will merge to 'master'.
* da/p4merge-mktemp-fix (2013-02-10) 1 commit
(merged to 'next' on 2013-02-14 at c5fc5ba)
+ p4merge: fix printf usage
Will merge to 'master'.
* jn/shell-disable-interactive (2013-02-11) 2 commits
- shell: pay attention to exit status from 'help' command
- shell doc: emphasize purpose and security model
Expecting a reroll.
$gmane/216229
* jk/read-commit-buffer-data-after-free (2013-02-11) 1 commit
(merged to 'next' on 2013-02-14 at 220e3a8)
+ log: re-encode commit messages before grepping
Will merge to 'master'.
* mk/old-expat (2013-02-11) 1 commit
(merged to 'next' on 2013-02-14 at 5fb47ce)
+ Allow building with xmlparse.h
Will merge to 'master'.
* ef/non-ascii-parse-options-error-diag (2013-02-11) 1 commit
(merged to 'next' on 2013-02-14 at 10cbdf8)
+ parse-options: report uncorrupted multi-byte options
Will merge to 'master'.
* jk/rebase-i-comment-char (2013-02-12) 1 commit
(merged to 'next' on 2013-02-14 at 0ed2f48)
+ rebase -i: respect core.commentchar
Will merge to 'master'.
* mm/config-local-completion (2013-02-12) 1 commit
(merged to 'next' on 2013-02-14 at 26bf6c2)
+ completion: support 'git config --local'
Will merge to 'master'.
* jc/fetch-raw-sha1 (2013-02-07) 4 commits
(merged to 'next' on 2013-02-14 at ffa3c65)
+ fetch: fetch objects by their exact SHA-1 object names
+ upload-pack: optionally allow fetching from the tips of hidden refs
+ fetch: use struct ref to represent refs to be fetched
+ parse_fetch_refspec(): clarify the codeflow a bit
(this branch uses jc/hidden-refs.)
Allows requests to fetch objects at any tip of refs (including
hidden ones). It seems that there may be use cases even outside
Gerrit (e.g. $gmane/215701).
* mn/send-email-works-with-credential (2013-02-12) 6 commits
- git-send-email: use git credential to obtain password
- Git.pm: add interface for git credential command
- Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
- Git.pm: refactor command_close_bidi_pipe to use _cmd_close
- Git.pm: fix example in command_close_bidi_pipe documentation
- Git.pm: allow command_close_bidi_pipe to be called as method
Hooks the credential system to send-email.
Rerolled.
Waiting for a review.
* nd/branch-show-rebase-bisect-state (2013-02-08) 1 commit
- branch: show rebase/bisect info when possible instead of "(no branch)"
Expecting a reroll.
$gmane/215771
* nd/count-garbage (2013-02-13) 4 commits
- count-objects: report how much disk space taken by garbage files
- count-objects: report garbage files in pack directory too
- sha1_file: reorder code in prepare_packed_git_one()
- git-count-objects.txt: describe each line in -v output
Looked good, but the handling of files with known-corrupt .idx
counterparts could be improved.
* wk/man-deny-current-branch-is-default-these-days (2013-02-14) 1 commit
(merged to 'next' on 2013-02-14 at 6fab9d4)
+ user-manual: Update for receive.denyCurrentBranch=refuse
Will merge to 'master'.
* tz/credential-authinfo (2013-02-05) 1 commit
- Add contrib/credentials/netrc with GPG support
A new read-only credential helper (in contrib/) to interact with
the .netrc/.authinfo files. Hopefully mn/send-email-authinfo topic
can rebuild on top of something like this.
Expecting a reroll.
$gmane/215556
* jl/submodule-deinit (2013-02-06) 1 commit
- submodule: add 'deinit' command
There was no Porcelain way to say "I no longer am interested in
this submodule", once you express your interest in a submodule with
"submodule init". "submodule deinit" is the way to do so.
Expecting another reroll.
$gmane/216276
* jc/remove-export-from-config-mak-in (2013-02-12) 2 commits
(merged to 'next' on 2013-02-12 at eb8af04)
+ Makefile: do not export mandir/htmldir/infodir
(merged to 'next' on 2013-02-07 at 33f7d4f)
+ config.mak.in: remove unused definitions
config.mak.in template had an "export" line to cause a few
common makefile variables to be exported; if they need to be
expoted for autoconf/configure users, they should also be exported
for people who write config.mak the same way. Move the "export" to
the main Makefile. Also, stop exporting mandir that used to be
exported (only) when config.mak.autogen was used. It would have
broken installation of manpages (but not other documentation
formats).
Will cook in 'next'.
* jc/mention-tracking-for-pull-default (2013-01-31) 1 commit
- doc: mention tracking for pull.default
We stopped mentioning `tracking` is a deprecated but supported
synonym for `upstream` in pull.default even though we have no
intention of removing the support for it.
This is my "don't list it to catch readers' eyes, but make sure it
can be found if the reader looks for it" version; I'm not married
to the layout and will be happy to take a replacement patch.
Will merge to 'next', unless a replacement materializes soonish.
* jc/hidden-refs (2013-02-07) 3 commits
(merged to 'next' on 2013-02-14 at b69f9cc)
+ upload/receive-pack: allow hiding ref hierarchies
+ upload-pack: simplify request validation
+ upload-pack: share more code
(this branch is used by jc/fetch-raw-sha1.)
Allow the server side to redact the refs/ namespace it shows to the
client.
Will merge to 'master'.
* jc/remove-treesame-parent-in-simplify-merges (2013-01-17) 1 commit
(merged to 'next' on 2013-01-30 at b639b47)
+ simplify-merges: drop merge from irrelevant side branch
The --simplify-merges logic did not cull irrelevant parents from a
merge that is otherwise not interesting with respect to the paths
we are following.
This touches a fairly core part of the revision traversal
infrastructure; even though I think this change is correct, please
report immediately if you find any unintended side effect.
Will cook in 'next'.
* jc/push-2.0-default-to-simple (2013-01-16) 14 commits
(merged to 'next' on 2013-01-16 at 23f5df2)
+ t5570: do not assume the "matching" push is the default
+ t5551: do not assume the "matching" push is the default
+ t5550: do not assume the "matching" push is the default
(merged to 'next' on 2013-01-09 at 74c3498)
+ doc: push.default is no longer "matching"
+ push: switch default from "matching" to "simple"
+ t9401: do not assume the "matching" push is the default
+ t9400: do not assume the "matching" push is the default
+ t7406: do not assume the "matching" push is the default
+ t5531: do not assume the "matching" push is the default
+ t5519: do not assume the "matching" push is the default
+ t5517: do not assume the "matching" push is the default
+ t5516: do not assume the "matching" push is the default
+ t5505: do not assume the "matching" push is the default
+ t5404: do not assume the "matching" push is the default
Will cook in 'next' until Git 2.0 ;-).
* bc/append-signed-off-by (2013-02-12) 12 commits
- Unify appending signoff in format-patch, commit and sequencer
- format-patch: update append_signoff prototype
- t4014: more tests about appending s-o-b lines
- sequencer.c: teach append_signoff to avoid adding a duplicate newline
- sequencer.c: teach append_signoff how to detect duplicate s-o-b
- sequencer.c: always separate "(cherry picked from" from commit body
- sequencer.c: require a conforming footer to be preceded by a blank line
- sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
- t/t3511: add some tests of 'cherry-pick -s' functionality
- t/test-lib-functions.sh: allow to specify the tag name to test_commit
- commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
- sequencer.c: rework search for start of footer to improve clarity
Waiting for further reviews.
$gmane/216327 may need to be addressed.
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
From: Junio C Hamano @ 2013-02-14 23:17 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git@vger.kernel.org
In-Reply-To: <7vd2w23k7k.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Andrew Ardill <andrew.ardill@gmail.com> writes:
>
>>> If that is the change we are going to make, and if you can guarantee
>>> that nobody who is used to the historical behaviour will complain,
>>> then I am fine with it, but I think the latter part of the condition
>>> will not hold.
>>
>> Does the impossibility of asserting that no-one will complain put this
>> in the 'too hard' bucket?
>
> Basically, yes. "Cannot be done without UI regression."
>
> It could be a Git 2.0 item, if you plan the transition right, though.
I have been staring at the patch again, but I do not think of an
easy way out without retraining the old timers to introduce this "if
we knew better, we would have done so from day one and the world
would have been a much better place" change. If we were to have
this in the longer term, we would need a proper transition plan,
similar to the one we devised to change the default used for a lazy
"git push" (and "git push $there") from the traditional "matching"
to "simple" at Git 2.0 boundary.
The transition would go like this:
* Introduce "git add --ignore-removal" option in the release after
the current cycle (a new feature is too late for this cycle):
- when "git add <pathspec>" is given without "--ignore-removal",
give a warning about upcoming default change, and advise people
to use either "--ignore-removal" or "--all" option, but behave
as if "--ignore-removal" were given.
- when "git add --ignore-removal <pathspec>" is given, only add
additions and modifications, just like the current behaviour.
- obviously, "-u", "-A", and "--ignore-removal" are mutually
exclusive.
* Run with the above for a few releases.
* Change the behaviour of "git add <pathspec>" without "-u", "-A"
nor "--ignore-removal" to error out with the same warning and
advise.
* Run with the above for a few releases.
* At Git 2.0, change "git add <pathspec>" without "-u", "-A" nor
"--ignore-removal" to behave as if "git add -A <pathspec>" were
given.
At any point during the above transtion, "git add" without any
pathspec will not change its meaning; it will stay a no-op.
^ permalink raw reply
* Re: [PATCH v3] add: warn when -u or -A is used without filepattern
From: Junio C Hamano @ 2013-02-14 23:36 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, Jonathan Nieder, Robin Rosenberg, Piotr Krukowiecki,
Eric James Michael Ritz, Tomas Carnecky, Michael J Gruber
In-Reply-To: <7v4ni1xjuc.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> We should probably update the documentation/help for "git add", but
> that is entirely a separate topic.
The documentation update in 0fa2eb530fb7 (add: warn when -u or -A is
used without pathspec, 2013-01-28) says:
If no <pathspec> is given, the current version of Git defaults to
"."; in other words, update all tracked files in the current directory
and its subdirectories. This default will change in a future version
of Git, hence the form without <filepattern> should not be used.
(oops, I just spotted a stray <filepattern> here, which came from a
semantic mismerge---I'll fix it locally).
The above text says that we currently add what you have in your
current directory and below, before it says this default will
change. That makes it easier to connect "the default will change"
and "form without pathspec should not be used" in readers' mind. It
does not take that much imagination and intelligence to infer "it
will change and will not limit to my current directory, so in the
future I will have to be explicit when I want to do what I just told
git to do".
But the warning text does not sound quite right. This is what I get:
warning: The behavior of 'git add --update (or -u)' with no path argument from a
subdirectory of the tree will change in Git 2.0 and should not be used anymore.
There is a logic gap between "will change" and "should not be used"
that is not filled like the text in the manual page does.
^ permalink raw reply
* Re: [PATCH v3] add: warn when -u or -A is used without filepattern
From: Junio C Hamano @ 2013-02-14 23:55 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, Jonathan Nieder, Robin Rosenberg, Piotr Krukowiecki,
Eric James Michael Ritz, Tomas Carnecky, Michael J Gruber
In-Reply-To: <7vr4kiwjqp.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> warning: The behavior of 'git add --update (or -u)' with no path argument from a
> subdirectory of the tree will change in Git 2.0 and should not be used anymore.
>
> There is a logic gap between "will change" and "should not be used"
> that is not filled like the text in the manual page does.
I guess it is not so bad after all, if you read the entire message,
not just the first two lines.
^ permalink raw reply
* Re: [RFC v2] git-multimail: a replacement for post-receive-email
From: Michael Haggerty @ 2013-02-15 5:07 UTC (permalink / raw)
To: Matthieu Moy
Cc: git discussion list, Andy Parkins, Sitaram Chamarty,
Junio C Hamano, Marc Branchaud,
Ævar Arnfjörð Bjarmason, Chris Hiestand
In-Reply-To: <vpq7gmbdpi2.fsf@grenoble-inp.fr>
On 02/14/2013 01:55 PM, Matthieu Moy wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> On 02/13/2013 03:56 PM, Matthieu Moy wrote:
>>
>>> Installation troubles:
>>>
>>> I had an old python installation (Red Hat package, and I'm not root),
>>> that did not include the email.utils package, so I couldn't use my
>>> system's python. I found no indication about python version in README,
>>> so I installed the latest python by hand, just to find out that
>>> git-multimail wasn't compatible with Python 3.x. 2to3 can fix
>>> automatically a number of 3.x compatibility issues, but not all of them
>>> so I gave up and installed Python 2.7.
>>
>> What version of Python was it that caused problems?
>
> Python 2.4.3, installed with RHEL 5.9.
>
>> I just discovered that the script wouldn't have worked with Python
>> 2.4, where "email.utils" used to be called "email.Utils".
>
> Indeed, "import email.Utils" works with this Python.
>
>> But I pushed a fix to GitHub:
>>
>> ddb1796660 Accommodate older versions of Python's email module.
>
> Not sufficient, but I added a pull request that works for me with 2.4.
>
>>> @@ -835,6 +837,17 @@ class ReferenceChange(Change):
>>> for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
>>> yield line
>>>
>>> + if adds and self.showlog:
>>> + yield '\n'
>>> + yield 'Detailed log of added commits:\n\n'
>>> + for line in read_lines(
>>> + ['git', 'log']
>>> + + self.logopts
>>> + + ['%s..%s' % (self.old.commit, self.new.commit,)],
>>> + keepends=True,
>>> + ):
>>> + yield line
>>> +
>>> # The diffstat is shown from the old revision to the new
>>> # revision. This is to show the truth of what happened in
>>> # this change. There's no point showing the stat from the
>>>
>>
>> Thanks for the patch. I like the idea, but I think the implementation
>> is incorrect. Your code will not only list new commits but will also
>> list commits that were already in the repository on another branch
>> (e.g., if an existing feature branch is merged into master, all of the
>> commits on the feature branch will be listed). (Or was that your
>> intention?)
>
> I did not think very carefully about this case, but the behavior of my
> code seems sensible (although not uncontroversial): it's just showing
> the detailed log for the same commits as the summary at the top of the
> email. I have no personnal preferences.
I guess it depends a lot on what logopts are used. If the user
configures logopts to emit full patches, then the repeated reporting of
the same commits would cause a big increase in the bulk of notification
emails. But if the logopts are set to just emit a brief summary (e.g.,
author and log message), then a bit of repetition might be acceptable.
But since I wouldn't use this feature, I don't personally have a preference.
>> But even worse, it will fail to list commits that were
>> added at the same time that a branch was created (e.g., if I create a
>> feature branch with a number of commits on it and then push it for the
>> first time).
>
> Right.
>
>> Probably the Push object has to negotiate with its constituent
>> ReferenceChange objects to figure out which one is responsible for
>> summarizing each of the commits newly added by the push (i.e., the ones
>> returned by push.get_new_commits(None)).
>
> I updated the pull request with a version that works for new branches,
> and takes the list of commits to display from the call to
> get_new_commits (which were already there for other purpose). Then, it
> essentially calls "git log --no-walk $list_of_sha1s".
>
> This should be better.
I will check it out.
Thanks!
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Michael J Gruber @ 2013-02-15 8:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, git
In-Reply-To: <7vvc9u22p6.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 14.02.2013 19:03:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> But the exact location of per-user and per-repository configuration
>>> files does not matter in this context and is best left to the
>>> git-config documentation.
>>
>> I'm OK with your version.
>
> I already queued your original with one s/not/now/; perhaps I will
> redo it then.
Yes, I think the new version improves upon Matthieu's which was a good
start to begin with :)
Michael
^ permalink raw reply
* Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way
From: Michael J Gruber @ 2013-02-15 8:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Mariusz Gronczewski
In-Reply-To: <7vzjz624kj.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 14.02.2013 18:22:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> Currently, verify_signed_buffer() only checks the return code of gpg,
>> and some callers implement additional unreliable checks for "Good
>> signature" in the gpg output meant for the user.
>>
>> Use the status output instead and parse for a line beinning with
>> "[GNUPG:] GOODSIG ". This is the only reliable way of checking for a
>> good gpg signature.
>>
>> If needed we can change this easily to "[GNUPG:] VALIDSIG " if we want
>> to take into account the trust model.
>
> Thanks. I didn't look beyond "man gpg" nor bother looking at
> DETAILS file in its source, which the manpage refers to.
>
> I think GOODSIG is a good starting point. Depending on the context
> (e.g. "%G?") we may also want to consider EXPSIG (but not EXPKEYSIG
> or REVKEYSIG) acceptable, while reading "log --show-signature" on
> ancient part of the history, no?
Yes, we could certainly return a more detailed status to the callers.
Currently, "0" is OK (GOODSIG) and everything else is a fail. We would
need to change the callers to allow more details on the "fail" as well
as the "OK" so that they can decide what is good enough, say:
-1: fail for technical reasons (no sig, can't run gpg etc.)
0: sig present bad (cryptographically) BAD
1: REVKEYSIG
2: EXPKEYSIG
3: EXPSIG
4: GOODSIG
5: VALIDSIG
I'd have to recheck whether a bitmask or ordered values make more sense.
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>> gpg-interface.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/gpg-interface.c b/gpg-interface.c
>> index 4559033..c582b2e 100644
>> --- a/gpg-interface.c
>> +++ b/gpg-interface.c
>> @@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>> /*
>> * Run "gpg" to see if the payload matches the detached signature.
>> * gpg_output, when set, receives the diagnostic output from GPG.
>> + * gpg_status, when set, receives the status output from GPG.
>> */
>> int verify_signed_buffer(const char *payload, size_t payload_size,
>> const char *signature, size_t signature_size,
>> struct strbuf *gpg_output)
>> {
>> struct child_process gpg;
>> - const char *args_gpg[] = {NULL, "--verify", "FILE", "-", NULL};
>> + const char *args_gpg[] = {NULL, "--status-fd=1", "--verify", "FILE", "-", NULL};
>> char path[PATH_MAX];
>> int fd, ret;
>> + struct strbuf buf = STRBUF_INIT;
>>
>> args_gpg[0] = gpg_program;
>> fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
>> @@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
>> memset(&gpg, 0, sizeof(gpg));
>> gpg.argv = args_gpg;
>> gpg.in = -1;
>> + gpg.out = -1;
>> if (gpg_output)
>> gpg.err = -1;
>> - args_gpg[2] = path;
>> + args_gpg[3] = path;
>> if (start_command(&gpg)) {
>> unlink(path);
>> return error(_("could not run gpg."));
>> @@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
>> strbuf_read(gpg_output, gpg.err, 0);
>> close(gpg.err);
>> }
>> + strbuf_read(&buf, gpg.out, 0);
>> + close(gpg.out);
>> +
>> ret = finish_command(&gpg);
>>
>> unlink_or_warn(path);
>>
>> + ret |= !strstr(buf.buf, "\n[GNUPG:] GOODSIG ");
>> + strbuf_release(&buf);
>> +
>> return ret;
>> }
^ permalink raw reply
* Re: [PATCH v3] add: warn when -u or -A is used without filepattern
From: Matthieu Moy @ 2013-02-15 10:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jonathan Nieder, Robin Rosenberg, Piotr Krukowiecki,
Eric James Michael Ritz, Tomas Carnecky, Michael J Gruber
In-Reply-To: <7vmwv6wivs.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> warning: The behavior of 'git add --update (or -u)' with no path argument from a
>> subdirectory of the tree will change in Git 2.0 and should not be used anymore.
>>
>> There is a logic gap between "will change" and "should not be used"
>> that is not filled like the text in the manual page does.
>
> I guess it is not so bad after all, if you read the entire message,
> not just the first two lines.
Also, the warning is meant to be read by a user who just typed
"git add -u", so it is expected that the user knows what it does in
current (or past) versions of Git.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [PATCH v4+ 3/4] count-objects: report garbage files in pack directory too
From: Nguyễn Thái Ngọc Duy @ 2013-02-15 12:07 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <7vehgkb43v.fsf@alter.siamese.dyndns.org>
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.
The garbage is reported with warning() instead of error() in packed
garbage case because it's not an error to have garbage. Loose garbage
is still reported as errors and will be converted to warnings later.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-count-objects.txt | 4 +-
builtin/count-objects.c | 12 +++++-
cache.h | 3 ++
sha1_file.c | 83 ++++++++++++++++++++++++++++++++++++-
t/t5304-prune.sh | 26 ++++++++++++
5 files changed, 124 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
+
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..1706c8b 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,14 @@
#include "builtin.h"
#include "parse-options.h"
+static unsigned long garbage;
+
+static void real_report_garbage(const char *desc, const char *path)
+{
+ warning("%s: %s", desc, path);
+ garbage++;
+}
+
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
@@ -76,7 +84,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+ unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -87,6 +95,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+ if (verbose)
+ report_garbage = real_report_garbage;
memcpy(path, objdir, len);
if (len && objdir[len-1] != '/')
path[len++] = '/';
diff --git a/cache.h b/cache.h
index 7339f21..73de68c 100644
--- a/cache.h
+++ b/cache.h
@@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char *feature_list, const char *fea
extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
+/* A hook for count-objects to report invalid files in pack directory */
+extern void (*report_garbage)(const char *desc, const char *path);
+
extern void prepare_packed_git(void);
extern void reprepare_packed_git(void);
extern void install_packed_git(struct packed_git *pack);
diff --git a/sha1_file.c b/sha1_file.c
index 239bee7..16967d3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
#include "sha1-lookup.h"
#include "bulk-checkin.h"
#include "streaming.h"
+#include "dir.h"
#ifndef O_NOATIME
#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -1000,6 +1001,63 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
}
+void (*report_garbage)(const char *desc, const char *path);
+
+static void report_helper(const struct string_list *list,
+ int seen_bits, int first, int last)
+{
+ const char *msg;
+ switch (seen_bits) {
+ case 0:
+ msg = "no corresponding .idx nor .pack";
+ break;
+ case 1:
+ msg = "no corresponding .idx";
+ break;
+ case 2:
+ msg = "no corresponding .pack";
+ break;
+ default:
+ return;
+ }
+ for (; first < last; first++)
+ report_garbage(msg, list->items[first].string);
+}
+
+static void report_pack_garbage(struct string_list *list)
+{
+ int i, baselen = -1, first = 0, seen_bits = 0;
+
+ if (!report_garbage)
+ return;
+
+ sort_string_list(list);
+
+ for (i = 0; i < list->nr; i++) {
+ const char *path = list->items[i].string;
+ if (baselen != -1 &&
+ strncmp(path, list->items[first].string, baselen)) {
+ report_helper(list, seen_bits, first, i);
+ baselen = -1;
+ seen_bits = 0;
+ }
+ if (baselen == -1) {
+ const char *dot = strrchr(path, '.');
+ if (!dot) {
+ report_garbage("garbage found", path);
+ continue;
+ }
+ baselen = dot - path + 1;
+ first = i;
+ }
+ if (!strcmp(path + baselen, "pack"))
+ seen_bits |= 1;
+ else if (!strcmp(path + baselen, "idx"))
+ seen_bits |= 2;
+ }
+ report_helper(list, seen_bits, first, list->nr);
+}
+
static void prepare_packed_git_one(char *objdir, int local)
{
/* Ensure that this buffer is large enough so that we can
@@ -1009,6 +1067,7 @@ static void prepare_packed_git_one(char *objdir, int local)
int len;
DIR *dir;
struct dirent *de;
+ struct string_list garbage = STRING_LIST_INIT_DUP;
sprintf(path, "%s/pack", objdir);
len = strlen(path);
@@ -1024,7 +1083,17 @@ static void prepare_packed_git_one(char *objdir, int local)
int namelen = strlen(de->d_name);
struct packed_git *p;
- if (len + namelen + 1 > sizeof(path))
+ if (len + namelen + 1 > sizeof(path)) {
+ if (report_garbage) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%.*s/%s", len - 1, path, de->d_name);
+ report_garbage("path too long", sb.buf);
+ strbuf_release(&sb);
+ }
+ continue;
+ }
+
+ if (is_dot_or_dotdot(de->d_name))
continue;
strcpy(path + len, de->d_name);
@@ -1043,8 +1112,20 @@ static void prepare_packed_git_one(char *objdir, int local)
(p = add_packed_git(path, len + namelen, local)) != NULL)
install_packed_git(p);
}
+
+ if (!report_garbage)
+ continue;
+
+ if (has_extension(de->d_name, ".idx") ||
+ has_extension(de->d_name, ".pack") ||
+ has_extension(de->d_name, ".keep"))
+ string_list_append(&garbage, path);
+ else
+ report_garbage("garbage found", path);
}
closedir(dir);
+ report_pack_garbage(&garbage);
+ string_list_clear(&garbage, 0);
}
static int sort_pack(const void *a_, const void *b_)
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index d645328..e4bb3a1 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -195,4 +195,30 @@ test_expect_success 'gc: prune old objects after local clone' '
)
'
+test_expect_success 'garbage report in count-objects -v' '
+ : >.git/objects/pack/foo &&
+ : >.git/objects/pack/foo.bar &&
+ : >.git/objects/pack/foo.keep &&
+ : >.git/objects/pack/foo.pack &&
+ : >.git/objects/pack/fake.bar &&
+ : >.git/objects/pack/fake.keep &&
+ : >.git/objects/pack/fake.pack &&
+ : >.git/objects/pack/fake.idx &&
+ : >.git/objects/pack/fake2.keep &&
+ : >.git/objects/pack/fake3.idx &&
+ git count-objects -v 2>stderr &&
+ grep "index file .git/objects/pack/fake.idx is too small" stderr &&
+ grep "^warning:" stderr | sort >actual &&
+ cat >expected <<\EOF &&
+warning: garbage found: .git/objects/pack/fake.bar
+warning: garbage found: .git/objects/pack/foo
+warning: garbage found: .git/objects/pack/foo.bar
+warning: no corresponding .idx nor .pack: .git/objects/pack/fake2.keep
+warning: no corresponding .idx: .git/objects/pack/foo.keep
+warning: no corresponding .idx: .git/objects/pack/foo.pack
+warning: no corresponding .pack: .git/objects/pack/fake3.idx
+EOF
+ test_cmp expected actual
+'
+
test_done
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* Re: [BUG] Veryfing signatures in git log fails when language is not english
From: Mariusz Gronczewski @ 2013-02-15 14:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7vliaq3kr0.fsf@alter.siamese.dyndns.org>
2013/2/14 Junio C Hamano <gitster@pobox.com>:
>
> - The "right" one you mention for %GS is easier than you might
> think. If you just verify against the accompanying "tagger"
> identity, that should be sufficient. It of course cannot be
> generally solved, as you could tag as person A while signing
> with key for person B, but a simple social convention would
> help us out there: if you tag as Mariusz Gronczewski, your
> signature should also say so.
unless there is someone else with same name, which happens more often
(so far i've seen it happen twice) than same GPG IDs. It's all fine if
you just have one keyring that you can use to validate against all
repos but when there are multiple projects each with different persons
responsible for deploying it can get messy ;].
my use-case is basically "allow only commits signed by person X Y or Z
to be deployed on production" and "allow only persons A, B, C, X, Y,
Z to commit", while latter case can be solved by software like
gitolite, credential validation is messy at best as you have to
validate:
- ssh key
- if ssh key owner matches commiter name
- if commiter name =! author name, if a given person can do that
(project architect or some other person accepting patches) or can't
and I'm trying to implement GPG signing so if someone does something
malicious i can say "OK that commit was signed by your key ID, why you
did it?"
--
Mariusz Gronczewski (XANi) <xani666@gmail.com>
GnuPG: 0xEA8ACE64
^ permalink raw reply
* [PATCH] read_directory: avoid invoking exclude machinery on tracked files
From: Nguyễn Thái Ngọc Duy @ 2013-02-15 14:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Karsten Blees, kusmabite, Ramkumar Ramachandra,
Robert Zeh, finnag, Nguyễn Thái Ngọc Duy
read_directory() (and its friendly wrapper fill_directory) collects
untracked/ignored files by traversing through the whole worktree (*),
feeding every entry to treat_one_path(), where each entry is checked
against .gitignore patterns.
One may see that tracked files can't be excluded and we do not need to
run them through exclude machinery. On repos where there are many
.gitignore patterns and/or a lot of tracked files, this unnecessary
processing can become expensive.
This patch avoids it mostly for normal cases. Directories are still
processed as before. DIR_SHOW_IGNORED and DIR_COLLECT_IGNORED are not
normally used unless some options are given (e.g. "checkout
--overwrite-ignore", "add -f"...) so people still need to pay penalty
in some cases, just not as often as before.
git status | webkit linux-2.6 libreoffice-core gentoo-x86
-------------+----------------------------------------------
before | 1.159s 0.226s 0.415s 0.597s
after | 0.778s 0.176s 0.266s 0.556s
nr. patterns | 89 376 19 0
nr. tracked | 182k 40k 63k 101k
(*) Not completely true. read_directory may skip recursing into a
directory if it's entirely excluded and DIR_SHOW_OTHER_DIRECTORIES
is not set.
Tracked-down-by: Karsten Blees <karsten.blees@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
For reference:
http://thread.gmane.org/gmane.comp.version-control.git/215820/focus=216195
dir.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/dir.c b/dir.c
index 57394e4..bdff256 100644
--- a/dir.c
+++ b/dir.c
@@ -1244,7 +1244,19 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
const struct path_simplify *simplify,
int dtype, struct dirent *de)
{
- int exclude = is_excluded(dir, path->buf, &dtype);
+ int exclude;
+
+ if (dtype == DT_UNKNOWN)
+ dtype = get_dtype(de, path->buf, path->len);
+
+ if (!(dir->flags & DIR_SHOW_IGNORED) &&
+ !(dir->flags & DIR_COLLECT_IGNORED) &&
+ dtype != DT_DIR &&
+ cache_name_exists(path->buf, path->len, ignore_case))
+ return path_ignored;
+
+ exclude = is_excluded(dir, path->buf, &dtype);
+
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
@@ -1256,9 +1268,6 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
if (exclude && !(dir->flags & DIR_SHOW_IGNORED))
return path_ignored;
- if (dtype == DT_UNKNOWN)
- dtype = get_dtype(de, path->buf, path->len);
-
switch (dtype) {
default:
return path_ignored;
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* Re: [BUG] Veryfing signatures in git log fails when language is not english
From: Junio C Hamano @ 2013-02-15 16:08 UTC (permalink / raw)
To: Mariusz Gronczewski; +Cc: Michael J Gruber, git
In-Reply-To: <CAJ9Ak2oF+SiYjStJndRZqLfnzBisn3TyXr3U_E_42BHrKbKPfQ@mail.gmail.com>
Mariusz Gronczewski <xani666@gmail.com> writes:
> 2013/2/14 Junio C Hamano <gitster@pobox.com>:
>>
>> - The "right" one you mention for %GS is easier than you might
>> think. If you just verify against the accompanying "tagger"
>> identity, that should be sufficient. It of course cannot be
>> generally solved, as you could tag as person A while signing
>> with key for person B, but a simple social convention would
>> help us out there: if you tag as Mariusz Gronczewski, your
>> signature should also say so.
> unless there is someone else with same name, which happens more often
> (so far i've seen it happen twice) than same GPG IDs.
Oh, I didn't mean to say "ignore email part", which of course will
make the result more likely to be ambiguous.
I thought you meant by "have to show right one" the following
scenario:
The tag v1.8.1 has a GPG signature. The key 96AFE6CB was used
to sign it. The key is associated with more than one identities.
One of them is "Junio C Hamano <gitster@pobox.com>", but that is
not the only one. I also have combinations of other e-mail
addresses and names spelled differently (e.g. "Junio Hamano
<jchXXXX@gmail.com>") that are _not_ associated with that key.
GPG may say "good signature from A aka B aka C"; which one of A,
B, or C should we choose?
I was suggesting that among the identities associated with the key
used to sign the tag, we should show the one that matches the
identity on the tagger field.
object 5d417842efeafb6e109db7574196901c4e95d273
type commit
tag v1.8.1
tagger Junio C Hamano <gitster@pobox.com> 1356992771 -0800
Git 1.8.1
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iQIc...
=v706
-----END PGP SIGNATURE-----
Because it is clear from the context where the signature appears
that that identity is what matters for me as a signer in the project
the tag appears in.
I may have other e-mail addresses that are not associated with that
key, but it would be insane to put that on the tagger field of the
tag, while GPG-signing with that key.
^ permalink raw reply
* Re: [PATCH] read_directory: avoid invoking exclude machinery on tracked files
From: Junio C Hamano @ 2013-02-15 16:52 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Karsten Blees, kusmabite, Ramkumar Ramachandra, Robert Zeh,
finnag
In-Reply-To: <1360937848-4426-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> read_directory() (and its friendly wrapper fill_directory) collects
> untracked/ignored files by traversing through the whole worktree (*),
> feeding every entry to treat_one_path(), where each entry is checked
> against .gitignore patterns.
>
> One may see that tracked files can't be excluded and we do not need to
> run them through exclude machinery. On repos where there are many
> .gitignore patterns and/or a lot of tracked files, this unnecessary
> processing can become expensive.
>
> This patch avoids it mostly for normal cases. Directories are still
> processed as before. DIR_SHOW_IGNORED and DIR_COLLECT_IGNORED are not
> normally used unless some options are given (e.g. "checkout
> --overwrite-ignore", "add -f"...) so people still need to pay penalty
> in some cases, just not as often as before.
>
> git status | webkit linux-2.6 libreoffice-core gentoo-x86
> -------------+----------------------------------------------
> before | 1.159s 0.226s 0.415s 0.597s
> after | 0.778s 0.176s 0.266s 0.556s
> nr. patterns | 89 376 19 0
> nr. tracked | 182k 40k 63k 101k
>
> (*) Not completely true. read_directory may skip recursing into a
> directory if it's entirely excluded and DIR_SHOW_OTHER_DIRECTORIES
> is not set.
>
> Tracked-down-by: Karsten Blees <karsten.blees@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> For reference:
> http://thread.gmane.org/gmane.comp.version-control.git/215820/focus=216195
>
> dir.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 57394e4..bdff256 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1244,7 +1244,19 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
> const struct path_simplify *simplify,
> int dtype, struct dirent *de)
> {
> - int exclude = is_excluded(dir, path->buf, &dtype);
> + int exclude;
> +
> + if (dtype == DT_UNKNOWN)
> + dtype = get_dtype(de, path->buf, path->len);
> +
> + if (!(dir->flags & DIR_SHOW_IGNORED) &&
> + !(dir->flags & DIR_COLLECT_IGNORED) &&
> + dtype != DT_DIR &&
> + cache_name_exists(path->buf, path->len, ignore_case))
> + return path_ignored;
> +
> + exclude = is_excluded(dir, path->buf, &dtype);
> +
> if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
> && exclude_matches_pathspec(path->buf, path->len, simplify))
> dir_add_ignored(dir, path->buf, path->len);
Interesting.
In the current code, we always check if a path is excluded, and when
dealing with DT_REG/DT_LNK, we call treat_file():
* When such a path is excluded, treat_file() returns true when we
are not showing ignored directories. This causes treat_one_path()
to return path_ignored, so for excluded DT_REG/DT_LNK paths when
no DIR_*_IGNORED is in effect, this change is a correct
optimization.
* When such a path is not excluded, on the ther hand, and when we
are not showing ignored directories, treat_file() just returns
the value of exclude_file, which is initialized to false and is
not changed in the function. This causes treat_one_path() to
return path_handled. However, the new code returns path_ignored
in this case.
What guarantees that this change is regression free? I do not seem
to be able to find anything that checks if the path is already known
to the index in the original code for the case you special cased
(i.e. DIR_*_IGNORED is not set and dtype is not DT_DIR). Do all the
callers that reach this function in their callgraph, when they get
path_ignored for a path in the index, behave as if the difference
between path_ignored and path_handled does not matter?
> @@ -1256,9 +1268,6 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
> if (exclude && !(dir->flags & DIR_SHOW_IGNORED))
> return path_ignored;
>
> - if (dtype == DT_UNKNOWN)
> - dtype = get_dtype(de, path->buf, path->len);
> -
> switch (dtype) {
> default:
> return path_ignored;
^ permalink raw reply
* Re: [PATCH] read_directory: avoid invoking exclude machinery on tracked files
From: Duy Nguyen @ 2013-02-15 18:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Karsten Blees, kusmabite, Ramkumar Ramachandra, Robert Zeh,
finnag
In-Reply-To: <7vd2w1wmdo.fsf@alter.siamese.dyndns.org>
On Fri, Feb 15, 2013 at 11:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> In the current code, we always check if a path is excluded, and when
> dealing with DT_REG/DT_LNK, we call treat_file():
>
> * When such a path is excluded, treat_file() returns true when we
> are not showing ignored directories. This causes treat_one_path()
> to return path_ignored, so for excluded DT_REG/DT_LNK paths when
> no DIR_*_IGNORED is in effect, this change is a correct
> optimization.
>
> * When such a path is not excluded, on the ther hand, and when we
> are not showing ignored directories, treat_file() just returns
> the value of exclude_file, which is initialized to false and is
> not changed in the function. This causes treat_one_path() to
> return path_handled. However, the new code returns path_ignored
> in this case.
>
> What guarantees that this change is regression free?
If you consider read_directory_recursive alone, there is a regression.
The return value of r_d_r depends on path_handled/path_ignored. With
this patch, the return value will be different. The return value is
only used by treat_directory() in two cases:
- when DIR_SHOW_IGNORED is set, which disables the optimization so no
regression
- when DIR_HIDE_EMPTY_DIRECTORIES is _not_ set (and neither is
DIR_SHOW_IGNORED), the optimization is still on and different r_d_r's
return value would lead to different behavior. However I don't think
it can happen.
treat_directory checks if the given directory can be found in index.
In that case the neither r_d_r calls in treat_directory is reachable.
If the given directory cannot be found in the index, the second r_d_r
is reachable. But then the cache_name_exists() in the patch should
always be false (parent not in index, children cannot), so the
optimization is off and r_d_r returns correctly.
It's a bit tricky. I'm not sure if I miss anything else.
> I do not seem
> to be able to find anything that checks if the path is already known
> to the index in the original code for the case you special cased
> (i.e. DIR_*_IGNORED is not set and dtype is not DT_DIR). Do all the
> callers that reach this function in their callgraph, when they get
> path_ignored for a path in the index, behave as if the difference
> between path_ignored and path_handled does not matter?
--
Duy
^ permalink raw reply
* Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-02-15 18:58 UTC (permalink / raw)
To: John Keeping; +Cc: git, gitster, pclouds, jrnieder, Brandon Casey
In-Reply-To: <20130214175849.GA27958@farnsworth.metanate.com>
On Thu, Feb 14, 2013 at 9:58 AM, John Keeping <john@keeping.me.uk> wrote:
> On Tue, Feb 12, 2013 at 02:33:42AM -0800, Brandon Casey wrote:
>> Teach append_signoff to detect whether a blank line exists at the position
>> that the signed-off-by line will be added, and refrain from adding an
>> additional one if one already exists. Or, add an additional line if one
>> is needed to make sure the new footer is separated from the message body
>> by a blank line.
>>
>> Signed-off-by: Brandon Casey <bcasey@nvidia.com>
>> ---
>
> As Jonathan Nieder wondered before [1], this changes the behaviour when
> the commit message is empty. Before this commit, there is an empty line
> followed by the S-O-B line; now the S-O-B is on the first line of the
> commit.
>
> The previous behaviour seems better to me since the empty line is
> hinting that the user should fill something in. It looks particularly
> strange if your editor has syntax highlighting for commit messages such
> that the first line is in a different colour.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/214796
>
>> diff --git a/sequencer.c b/sequencer.c
>> index 3364faa..084573b 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -1127,8 +1127,19 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
>> else
>> has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
>>
>> - if (!has_footer)
>> - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
>> + if (!has_footer) {
>> + const char *append_newlines = NULL;
>> + size_t len = msgbuf->len - ignore_footer;
>> +
>> + if (len && msgbuf->buf[len - 1] != '\n')
>> + append_newlines = "\n\n";
>> + else if (len > 1 && msgbuf->buf[len - 2] != '\n')
>> + append_newlines = "\n";
>
> To restore the old behaviour this needs something like this:
>
> else if (!len)
> append_newlines = "\n";
>
>> + if (append_newlines)
>> + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
>> + append_newlines, strlen(append_newlines));
>> + }
>>
>> if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
>> strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
Are you talking about the output produced by format-patch? Or are you
talking about what happens when you do 'commit --amend -s' for a
commit with an empty commit message. (The email that you referenced
was about the behavior of format-patch).
I'm thinking you must be talking about the 'commit --amend -s'
behavior since you mentioned your editor. Is there another case that
is affected by this? Normally, any extra blank lines that precede or
follow a commit message are removed before the commit object is
created. So, I guess it wouldn't hurt to insert a newline (or maybe
it should be two?) before the signoff in this case. Would this
provide an improvement or change for any other commands than 'commit
--amend -s'?
If we want to do this, then I'd probably do it like this:
- if (len && msgbuf->buf[len - 1] != '\n')
+ if (!len || msgbuf->buf[len - 1] != '\n')
append_newlines = "\n\n";
- else if (len > 1 && msgbuf->buf[len - 2] != '\n')
+ else if (len == 1 || msgbuf->buf[len - 2] != '\n')
append_newlines = "\n";
This would ensure there were two newlines preceding the sob. The
editor would place its cursor on the top line where the user should
begin typing in a commit message. If an editor was not opened up
(e.g. if 'git cherry-pick -s --allow-empty-message ...' was used) then
the normal mechanism that removes extra blank lines would trigger to
remove the extra blank lines.
I think that's reasonable.
It seems 'git cherry-pick -s --edit' follows a different code path,
and the commit message is stripped of newlines by 'git commit' before
it is passed to the editor. 'cherry-pick -s --edit' and 'commit
--amend -s' should probably have the same behavior and present the
same buffer to the user for editing when they encounter a commit with
an empty message.
Maybe something like this is enough(?):
diff --git a/builtin/commit.c b/builtin/commit.c
index 7b9e2ac..0796412 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -124,8 +124,10 @@ static int opt_parse_m(const struct option *opt, const char
if (unset)
strbuf_setlen(buf, 0);
else {
+ if (buf->len)
+ strbuf_addch(buf, '\n');
strbuf_addstr(buf, arg);
- strbuf_addstr(buf, "\n\n");
+ strbuf_complete_line(buf);
}
return 0;
}
@@ -673,9 +675,6 @@ static int prepare_to_commit(const char *index_file, const c
if (s->fp == NULL)
die_errno(_("could not open '%s'"), git_path(commit_editmsg));
- if (clean_message_contents)
- stripspace(&sb, 0);
-
if (signoff) {
/*
* See if we have a Conflicts: block at the end. If yes, count
@@ -703,6 +702,9 @@ static int prepare_to_commit(const char *index_file, const c
append_signoff(&sb, ignore_footer, 0);
}
+ if (clean_message_contents)
+ stripspace(&sb, 0);
+
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
die_errno(_("could not write commit template"));
I suspect we have a broken test at t7502.15 though that happened to
work because opt_parse_m() was appending two newlines that the test
expected to be there.
-Brandon
^ permalink raw reply related
* [BUG] Git clone of a bundle fails, but works (somewhat) when run with strace
From: Alain Kalker @ 2013-02-15 19:33 UTC (permalink / raw)
To: git
tl;dr:
- `git bundle create` without <git-rev-list-args> gives git rev-list
help, then dies.
Should point out missing <git-rev-list-args> instead.
- `git clone <bundle> <dir> gives "ERROR: Repository not found."
- `strace ... git clone <bundle> <dir>` (magically) appears to work but
cannot checkout files b/c of nonexistent ref.
- Heisenbug? Race condition?
- Zaphod Beeblebrox has left the building, sulking.
Full description:
When I try to clone from a bundle created from a local repository, `git
clone <bundle> <dir>` fails with: "ERROR: Repository not found. fatal:
Could not read from remote repository." unless I run it with strace.
OS: Arch Linux (rolling release)
Git versions: 1.8.1.3 and git://github.com/git.git master@02339dd
Steps to reproduce:
$ # Clone the Linux kernel repository
$ git clone git://github.com/torvalds/linux.git
Cloning into 'linux'...
remote: Counting objects: 2841147, done.
remote: Compressing objects: 100% (670736/670736), done.
remote: Total 2841147 (delta 2308339), reused 2657487 (delta 2143012)
Receiving objects: 100% (2841147/2841147), 797.62 MiB | 2.59 MiB/s, done.
Resolving deltas: 100% (2308339/2308339), done.
Checking out files: 100% (41521/41521), done.
$ cd linux
$ git branch -av
* master 323a72d Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
remotes/origin/HEAD -> origin/master
remotes/origin/master 323a72d Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
$ # Try to create a bundle
$ git bundle create ../linux.bundle
usage: git rev-list [OPTION] <commit-id>... [ -- paths... ]
limiting output:
--max-count=<n>
--max-age=<epoch>
--min-age=<epoch>
--sparse
--no-merges
--min-parents=<n>
--no-min-parents
--max-parents=<n>
--no-max-parents
--remove-empty
--all
--branches
--tags
--remotes
--stdin
--quiet
ordering output:
--topo-order
--date-order
--reverse
formatting output:
--parents
--children
--objects | --objects-edge
--unpacked
--header | --pretty
--abbrev=<n> | --no-abbrev
--abbrev-commit
--left-right
special purpose:
--bisect
--bisect-vars
--bisect-all
error: rev-list died
$ # IMHO the error should refer to the usage of `git bundle` with a
proper basis, not `git rev-list`.
$ # Also nothing should die loudly because of a missing parameter.
$ git bundle create ../linux.bundle master
Counting objects: 2836191, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (505627/505627), done.
Writing objects: 100% (2836191/2836191), 796.59 MiB | 16.23 MiB/s, done.
Total 2836191 (delta 2304454), reused 2834391 (delta 2303193)
$ # Try to clone a new repository from the bundle
$ cd ..
$ git clone linux.bundle linuxfrombundle
Cloning into 'linuxfrombundle'...
ERROR: Repository not found.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
$ git clone linux.bundle -b master linuxfrombundle
Cloning into 'linuxfrombundle'...
ERROR: Repository not found.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
# Try again using strace
$ # (Replace /dev/null with a filename if you really want to try and
debug this, or if you just want to torture your hard drive ;) )
$ strace -o /dev/null git clone linux.bundle linuxfrombundle
Cloning into 'linuxfrombundle'...
Receiving objects: 100% (2836191/2836191), 796.59 MiB | 24.64 MiB/s, done.
Resolving deltas: 100% (2304454/2304454), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.
$ # Let's have a look at what we cloned
$ cd linuxfrombundle
$ ls
$ git branch -av
remotes/origin/master 323a72d Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
$ git checkout master
Checking out files: 100% (41521/41521), done.
Branch master set up to track remote branch master from origin.
Already on 'master'
$ git branch -av
* master 323a72d Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
remotes/origin/master 323a72d Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
$ # Where's my HEAD?
Kind regards,
Alain Kalker
^ permalink raw reply
* Re: [PATCH] read_directory: avoid invoking exclude machinery on tracked files
From: Junio C Hamano @ 2013-02-15 19:32 UTC (permalink / raw)
To: Duy Nguyen
Cc: git, Karsten Blees, kusmabite, Ramkumar Ramachandra, Robert Zeh,
finnag
In-Reply-To: <CACsJy8A6oBjbaX=3iQcSxcwed28KLTk_tN+iuWDLsC512Z2V1Q@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Fri, Feb 15, 2013 at 11:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> In the current code, we always check if a path is excluded, and when
>> dealing with DT_REG/DT_LNK, we call treat_file():
>>
>> * When such a path is excluded, treat_file() returns true when we
>> are not showing ignored directories. This causes treat_one_path()
>> to return path_ignored, so for excluded DT_REG/DT_LNK paths when
>> no DIR_*_IGNORED is in effect, this change is a correct
>> optimization.
>>
>> * When such a path is not excluded, on the ther hand, and when we
>> are not showing ignored directories, treat_file() just returns
>> the value of exclude_file, which is initialized to false and is
>> not changed in the function. This causes treat_one_path() to
>> return path_handled. However, the new code returns path_ignored
>> in this case.
>>
>> What guarantees that this change is regression free?
>
> If you consider read_directory_recursive alone, there is a regression.
> The return value of r_d_r depends on path_handled/path_ignored. With
> this patch, the return value will be different.
That is exactly what was missing from the proposed log message, and
made me ask "Do all the callers that reach this function in their
callgraph, when they get path_ignored for a path in the index,
behave as if the difference between path_ignored and path_handled
does not matter?" Your answer seems to be
- r-d-r returns 'how many paths in this directory match the
criteria we are looking for', unless check_only is true. Now in
some cases we return path_ignored not path_handled, so we may
return a number that is greater than we used to return.
- treat_directory, the only user of that return value, cares if
r-d-r returned 0 or non-zero; and
- As long as we keep returning 0 from r-d-r in cases we used to
return 0 and non-zero in cases we used to return non-zero, exact
number does not matter. Overall we get the same result.
I think all of the above is true, but I have not convinced myself
that r-d-r with the new code never returns 0 when we used to return
non-zero.
> ...
> It's a bit tricky. I'm not sure if I miss anything else.
Hrm...
^ permalink raw reply
* Re: You have exceeded the email quota limit
From: Alain Kalker @ 2013-02-15 21:55 UTC (permalink / raw)
To: git
In-Reply-To: <20130215142253.189649c3k4kd08ow@webmail.latech.edu>
On Fri, 15 Feb 2013 14:22:53 -0600, wlewis wrote:
Spam, spam, beautiful SPAM.
'nuff said.
^ permalink raw reply
* [PATCH] contrib/subtree: remove contradicting use options on echo wrapper
From: Paul Campbell @ 2013-02-15 22:20 UTC (permalink / raw)
To: git
Cc: Adam Tkac, David A. Greene, Jesper L. Nielsen, Junio C Hamano,
Michael Schubert, Techlive Zheng
Remove redundant -n option and raw ^M in call to echo.
Call to 'say' function, a wrapper of 'echo', passed the parameter -n, then
included a raw ^M newline in the end of the last parameter. Yet the -n option
is meant to suppress the addition of new line by echo.
Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
---
contrib/subtree/git-subtree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..51146bd 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -592,7 +592,7 @@ cmd_split()
eval "$grl" |
while read rev parents; do
revcount=$(($revcount + 1))
- say -n "$revcount/$revmax ($createcount)
"
+ say "$revcount/$revmax ($createcount)"
debug "Processing commit: $rev"
exists=$(cache_get $rev)
if [ -n "$exists" ]; then
--
1.8.1.3.605.g02339dd
^ permalink raw reply related
* Re: [BUG] Git clone of a bundle fails, but works (somewhat) when run with strace
From: Alain Kalker @ 2013-02-15 22:25 UTC (permalink / raw)
To: git
In-Reply-To: <511E8D84.6060601@gmail.com>
On Fri, 15 Feb 2013 20:33:24 +0100, Alain Kalker wrote:
> tl;dr:
>
> - `git bundle create` without <git-rev-list-args> gives git rev-list
> help, then dies.
> Should point out missing <git-rev-list-args> instead.
> - `git clone <bundle> <dir> gives "ERROR: Repository not found."
> - `strace ... git clone <bundle> <dir>` (magically) appears to work but
> cannot checkout files b/c of nonexistent ref.
> - Heisenbug? Race condition?
> - Zaphod Beeblebrox has left the building, sulking.
>
> Full description:
>
> When I try to clone from a bundle created from a local repository, `git
> clone <bundle> <dir>` fails with: "ERROR: Repository not found. fatal:
> Could not read from remote repository." unless I run it with strace.
>
> OS: Arch Linux (rolling release)
> Git versions: 1.8.1.3 and git://github.com/git.git master@02339dd
>
> Steps to reproduce:
For those who like to "save the trees" (source code or otherwise), here
is a much simplified test case:
$ # Create test repository with a single commit in it
$ mkdir testrepo
$ cd testrepo
$ git init
$ echo Test > test.txt
$ git add test.txt
$ git commit -m "Add test.txt"
$ # Create bundle
$ git bundle create ../testrepo.bundle master
$ # Try to clone from bundle
$ cd ..
$ git clone testrepo.bundle testrepofrombundle
$ # Clone from bundle, wrapped with strace
$ strace -f -o /dev/null git clone testrepo.bundle testrepofrombundle
$ # Examine cloned repository
$ cd testrepofrombundle
$ ls
$ git branch -av
$ git checkout master
$ git branch -av
$ # Where's my HEAD?
> Kind regards,
>
> Alain Kalker
^ permalink raw reply
* Re: [PATCH] contrib/subtree: remove contradicting use options on echo wrapper
From: Junio C Hamano @ 2013-02-15 22:39 UTC (permalink / raw)
To: Paul Campbell
Cc: git, Adam Tkac, David A. Greene, Jesper L. Nielsen,
Michael Schubert, Techlive Zheng
In-Reply-To: <CALeLG_=p9k2B6AmTG0iKf9GpGB=_6kcECmCdDV1nmruJ4bdGcw@mail.gmail.com>
Paul Campbell <pcampbell@kemitix.net> writes:
> Remove redundant -n option and raw ^M in call to echo.
>
> Call to 'say' function, a wrapper of 'echo', passed the parameter -n, then
> included a raw ^M newline in the end of the last parameter. Yet the -n option
> is meant to suppress the addition of new line by echo.
>
> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
I generally do not comment on comment on contrib/ material, and I am
not familiar with subtree myself, but
for count in $(seq 0 $total)
do
echo -n "$count/$total^M"
... do heavy lifting ...
done
echo "Done "
is an idiomatic way to implement a progress meter without scrolling
more important message you gave earlier to the user before entering
the loop away. The message appears, carrige-return moves the cursor
to the beginning of the line without going to the next line, and the
next iteration overwrites the previous count. Finally, the progress
meter is overwritten with the "Done" message. Alternatively you can
wrap it up with
echo
echo Done
if you want to leave the final progress "100/100" before saying "Done."
Isn't that what this piece of code trying to do?
> ---
> contrib/subtree/git-subtree.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 8a23f58..51146bd 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -592,7 +592,7 @@ cmd_split()
> eval "$grl" |
> while read rev parents; do
> revcount=$(($revcount + 1))
> - say -n "$revcount/$revmax ($createcount)
> "
> + say "$revcount/$revmax ($createcount)"
> debug "Processing commit: $rev"
> exists=$(cache_get $rev)
> if [ -n "$exists" ]; then
^ permalink raw reply
* [PATCH v2 1/3] contrib/subtree: Store subtree sources in .gitsubtree and use for push/pull
From: Paul Campbell @ 2013-02-15 22:42 UTC (permalink / raw)
To: git
Cc: Adam Tkac, David A. Greene, Jesper L. Nielsen, Michael Schubert,
Techlive Zheng
Add the prefix, repository and refspec in the file .gitsubtree when
git subtree add is used. Then when a git subtree push or pull is needed
the repository and refspec from .gitsubtree are used as the default
values.
Having to remember what subtree came from what source is a waste of
developer memory and doesn't transfer easily to other developers.
git subtree push/pull operations would typically be to/from the same
source that the original subtree was cloned from with git subtree add.
The <repository> and <refspec>, or <commit>, used in the git subtree add
operation are stored in .gitsubtree. One line each, delimited by a space:
"<prefix> <repository> <refspec>" or "<prefix> . <commit>".
Subsequent git subtree push/pull operations now default to the values
stored in .gitsubtree, unless overridden from the command line.
The .gitsubtree file should be tracked as part of the repo as it
describes where parts of the tree came from and can be used to update
to and from that source.
Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
---
Reworked my previous patch paying closer attention to the coding style
documentation and renamed my new functions to make more sense.
contrib/subtree/git-subtree.sh | 75 +++++++++++++++++++++++++++++++++++-------
1 file changed, 64 insertions(+), 11 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 51146bd..6dc8999 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -11,8 +11,8 @@ OPTS_SPEC="\
git subtree add --prefix=<prefix> <commit>
git subtree add --prefix=<prefix> <repository> <commit>
git subtree merge --prefix=<prefix> <commit>
-git subtree pull --prefix=<prefix> <repository> <refspec...>
-git subtree push --prefix=<prefix> <repository> <refspec...>
+git subtree pull --prefix=<prefix> [<repository> <refspec...>]
+git subtree push --prefix=<prefix> [<repository> <refspec...>]
git subtree split --prefix=<prefix> <commit...>
--
h,help show the help
@@ -489,6 +489,28 @@ ensure_clean()
fi
}
+add_subtree () {
+ if ( grep "^$dir " .gitsubtree )
+ then
+ # remove $dir from .gitsubtree - there's probably a clever way to
do this with sed
+ grep -v "^$dir " .gitsubtree > .gitsubtree.temp
+ rm .gitsubtree
+ mv .gitsubtree.temp .gitsubtree
+ fi
+ if test $# -eq 1
+ then
+ # Only a commit provided, thus use the current repository
+ echo "$dir . $@" >> .gitsubtree
+ elif test $# -eq 2
+ then
+ echo "$dir $@" >> .gitsubtree
+ fi
+}
+
+get_subtree () {
+ grep "^$dir " .gitsubtree || die "Subtree not found in .gitsubtree: " "$dir"
+}
+
cmd_add()
{
if [ -e "$dir" ]; then
@@ -497,6 +519,8 @@ cmd_add()
ensure_clean
+ add_subtree "$@"
+
if [ $# -eq 1 ]; then
git rev-parse -q --verify "$1^{commit}" >/dev/null ||
die "'$1' does not refer to a commit"
@@ -700,7 +724,23 @@ cmd_merge()
cmd_pull()
{
ensure_clean
- git fetch "$@" || exit $?
+ if test $# -eq 0
+ then
+ subtree=($(get_subtree))
+ repository=${subtree[1]}
+ refspec=${subtree[2]}
+ if test "$repository" == "."
+ then
+ echo "Pulling into $dir from branch $refspec"
+ else
+ echo "Pulling into '$dir' from '$repository' '$refspec'"
+ fi
+ echo "git fetch using: " $repository $refspec
+ git fetch "$repository" "$refspec" || exit $?
+ else
+ echo "git fetch using: $@"
+ git fetch "$@" || exit $?
+ fi
revs=FETCH_HEAD
set -- $revs
cmd_merge "$@"
@@ -708,16 +748,29 @@ cmd_pull()
cmd_push()
{
- if [ $# -ne 2 ]; then
- die "You must provide <repository> <refspec>"
+ repository=$1
+ refspec=$2
+ if test $# -eq 0
+ then
+ subtree=($(get_subtree))
+ repository=${subtree[1]}
+ refspec=${subtree[2]}
+ if test "$repository" == "."
+ then
+ echo "Pushing from $dir into branch $refspec"
+ else
+ echo "Pushing from $dir into $repository $refspec"
+ fi
+ elif test $# -ne 2
+ then
+ die "You must provide <repository> <refspec>, or a <prefix> listed
in .gitsubtree"
fi
- if [ -e "$dir" ]; then
- repository=$1
- refspec=$2
- echo "git push using: " $repository $refspec
- git push $repository $(git subtree split
--prefix=$prefix):refs/heads/$refspec
+ if test -e "$dir"
+ then
+ echo "git push using: " $repository $refspec
+ git push $repository $(git subtree split
--prefix=$prefix):refs/heads/$refspec
else
- die "'$dir' must already exist. Try 'git subtree add'."
+ die "'$dir' must already exist. Try 'git subtree add'."
fi
}
--
1.8.1.3.605.g02339dd
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox