* Re: git send-email should not allow 'y' for in-reply-to
From: Antoine Pelisse @ 2013-01-11 21:53 UTC (permalink / raw)
To: Jeff King; +Cc: Matt Seitz (matseitz), git@vger.kernel.org
In-Reply-To: <20130111212325.GA18193@sigill.intra.peff.net>
On Fri, Jan 11, 2013 at 10:23 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 11, 2013 at 08:13:57PM +0000, Matt Seitz (matseitz) wrote:
>
>> > > How about "What Message-ID to use as In-Reply-To for the first email?"
>> > > or "Provide the Message-ID to use as In-Reply-To for the first
>> > > email:".
>> >
>> > seem fine to me. Maybe somebody who has been confused by it can offer
>> > more. At any rate, patches welcome.
>>
>> Suggestion: "Message-ID to use as In-Reply-To for the first email:".
>>
>> Simple and unlikely to generate a "y" or "n" response. Putting
>> "Message-ID" first makes it more obvious what data is being asked for
>> by this prompt.
>
> You'd think. But the existing message that has been causing problems is:
>
> Message-ID to be used as In-Reply-To for the first email?
>
> which is more or less what you are proposing. I do think a colon rather
> than a question mark helps indicate that the response is not yes/no.
That is true.
I'm definitely not a wording person, but assuming people who make the
mistake probably don't read the whole sentence out of laziness (that
might be somehow extreme though ;), starting it with "what" makes it
obvious at first sight that you can't answer yes/no.
That is not true if the message starts with Message-ID .. which
doesn't look like a question. Now it feels like you have agree or not.
Antoine,
^ permalink raw reply
* Re: [PATCH v5] git-completion.bash: add support for path completion
From: Junio C Hamano @ 2013-01-11 22:02 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git, szeder, felipe.contreras, peff
In-Reply-To: <1357930123-26310-1-git-send-email-manlio.perillo@gmail.com>
Manlio Perillo <manlio.perillo@gmail.com> writes:
> +# Process path list returned by "ls-files" and "diff-index --name-only"
> +# commands, in order to list only file names relative to a specified
> +# directory, and append a slash to directory names.
> +__git_index_file_list_filter ()
> +{
> + # Default to Bash >= 4.x
> + __git_index_file_list_filter_bash
> +}
> +
> +# Execute git ls-files, returning paths relative to the directory
> +# specified in the first argument, and using the options specified in
> +# the second argument.
> +__git_ls_files_helper ()
> +{
> + # NOTE: $2 is not quoted in order to support multiple options
> + cd "$1" && git ls-files --exclude-standard $2
> +} 2>/dev/null
I think this redirection is correct but a bit tricky; it is in
effect during the execution of the { block } (in other words, it is
not about squelching errors during the function definition).
-- >8 --
#!/bin/sh
cat >t.sh <<\EOF &&
echo I am "$1"
t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
t
for sh in bash dash ksh zsh
do
$sh t.sh $sh
done
-- 8< --
Bash does (so do dash and real AT&T ksh) grok this correctly, but
zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
pretending to be ksh gets this wrong as well). Not that what ksh
does matters, as it won't be dot-sourcing bash completion script.
It however may affect zsh, which does seem to dot-source this file.
Perhaps zsh completion may have to be rewritten in a similar way as
tcsh completion is done (i.e. does not dot-source this file but ask
bash to do the heavy-lifting).
This function seems to be always called in an subshell (e.g. as an
upstream of a pipeline), so the "cd" may be harmless, but don't you
need to disable CDPATH while doing this?
> +# Execute git diff-index, returning paths relative to the directory
> +# specified in the first argument, and using the tree object id
> +# specified in the second argument.
> +__git_diff_index_helper ()
> +{
> + cd "$1" && git diff-index --name-only --relative "$2"
> +} 2>/dev/null
Ditto.
> @@ -722,6 +875,43 @@ __git_has_doubledash ()
> return 1
> }
>
> +# Try to count non option arguments passed on the command line for the
> +# specified git command.
> +# When options are used, it is necessary to use the special -- option to
> +# tell the implementation were non option arguments begin.
> +# XXX this can not be improved, since options can appear everywhere, as
> +# an example:
> +# git mv x -n y
If that is the case, it is a bug in the command line parser, I
think. We should reject it, and the command line completer
certainly should not encourage it.
^ permalink raw reply
* [PATCH] format_commit_message(): simplify calls to logmsg_reencode()
From: Junio C Hamano @ 2013-01-11 22:15 UTC (permalink / raw)
To: git
All the other callers of logmsg_reencode() pass return value of
get_commit_output_encoding() or get_log_output_encoding(). Teach
the function to optionally take NULL as a synonym to "" aka "no
conversion requested" so that we can simplify the only remaining
calling site.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pretty.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/pretty.c b/pretty.c
index e87fe9f..732e2a2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -500,7 +500,7 @@ char *logmsg_reencode(const struct commit *commit,
char *encoding;
char *out;
- if (!*output_encoding)
+ if (!output_encoding || !*output_encoding)
return NULL;
encoding = get_header(commit, "encoding");
use_encoding = encoding ? encoding : utf8;
@@ -1184,23 +1184,15 @@ void format_commit_message(const struct commit *commit,
const struct pretty_print_context *pretty_ctx)
{
struct format_commit_context context;
- static const char utf8[] = "UTF-8";
const char *output_enc = pretty_ctx->output_encoding;
memset(&context, 0, sizeof(context));
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb->len;
- context.message = commit->buffer;
- if (output_enc) {
- char *enc = get_header(commit, "encoding");
- if (strcmp(enc ? enc : utf8, output_enc)) {
- context.message = logmsg_reencode(commit, output_enc);
- if (!context.message)
- context.message = commit->buffer;
- }
- free(enc);
- }
+ context.message = logmsg_reencode(commit, output_enc);
+ if (!context.message)
+ context.message = commit->buffer;
strbuf_expand(sb, format, format_commit_item, &context);
rewrap_message_tail(sb, &context, 0, 0, 0);
--
1.8.1.407.g91cb4ac
^ permalink raw reply related
* Re: [PATCH] format_commit_message(): simplify calls to logmsg_reencode()
From: Junio C Hamano @ 2013-01-11 22:15 UTC (permalink / raw)
To: git
In-Reply-To: <1357942509-21431-1-git-send-email-gitster@pobox.com>
Please disregard this...
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Junio C Hamano @ 2013-01-11 22:18 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Jeff King, Matt Seitz (matseitz), git@vger.kernel.org
In-Reply-To: <CALWbr2xasF1y9j3G=-fQ9Wwg41Ymv=MMsWoqyuhweDov9KpRvg@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> On Fri, Jan 11, 2013 at 10:23 PM, Jeff King <peff@peff.net> wrote:
>> On Fri, Jan 11, 2013 at 08:13:57PM +0000, Matt Seitz (matseitz) wrote:
>>
>>> > > How about "What Message-ID to use as In-Reply-To for the first email?"
>>> > > or "Provide the Message-ID to use as In-Reply-To for the first
>>> > > email:".
>>> >
>>> > seem fine to me. Maybe somebody who has been confused by it can offer
>>> > more. At any rate, patches welcome.
>>>
>>> Suggestion: "Message-ID to use as In-Reply-To for the first email:".
>>>
>>> Simple and unlikely to generate a "y" or "n" response. Putting
>>> "Message-ID" first makes it more obvious what data is being asked for
>>> by this prompt.
>>
>> You'd think. But the existing message that has been causing problems is:
>>
>> Message-ID to be used as In-Reply-To for the first email?
>>
>> which is more or less what you are proposing. I do think a colon rather
>> than a question mark helps indicate that the response is not yes/no.
>
> That is true.
>
> I'm definitely not a wording person, but assuming people who make the
> mistake probably don't read the whole sentence out of laziness (that
> might be somehow extreme though ;), starting it with "what" makes it
> obvious at first sight that you can't answer yes/no.
> That is not true if the message starts with Message-ID .. which
> doesn't look like a question. Now it feels like you have agree or not.
The exchange, when you do not have a configuration, goes like this:
$ git send-email 0001-filename-of-the-patch.patch
0001-filename-of-the-patch.patch
Who should the emails be sent to (if any)? junio
Are you sure you want to use <junio> [y/N]? y
Message-ID to be used as In-Reply-To for the first email (if any)?
Why not do this instead?
$ git send-email 0001-filename-of-the-patch.patch
0001-filename-of-the-patch.patch
Who should the emails be sent to (if any)? junio
Are you sure you want to use <junio> [y/N]? y
Is this a response to an existing message [y/N]? y
What is the Message-ID of the message you are replying to?
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Antoine Pelisse @ 2013-01-11 22:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Matt Seitz (matseitz), git@vger.kernel.org
In-Reply-To: <7vy5fz9xdl.fsf@alter.siamese.dyndns.org>
On Fri, Jan 11, 2013 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The exchange, when you do not have a configuration, goes like this:
>
> $ git send-email 0001-filename-of-the-patch.patch
> 0001-filename-of-the-patch.patch
> Who should the emails be sent to (if any)? junio
> Are you sure you want to use <junio> [y/N]? y
> Message-ID to be used as In-Reply-To for the first email (if any)?
>
> Why not do this instead?
>
> $ git send-email 0001-filename-of-the-patch.patch
> 0001-filename-of-the-patch.patch
> Who should the emails be sent to (if any)? junio
> Are you sure you want to use <junio> [y/N]? y
> Is this a response to an existing message [y/N]? y
I'm not sure about the extra question. If the user doesn't care, he
will probably use the empty default, which will result in the same
number of steps. If the user cares, he probably knows what he's doing
and will give a sensible value.
> What is the Message-ID of the message you are replying to?
I would simply go for:
What Message-ID are you replying to (if any)?
If I don't know what to answer, I would definitely not say y/yes/n/no,
but press enter directly.
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Junio C Hamano @ 2013-01-11 23:54 UTC (permalink / raw)
To: Antoine Pelisse
Cc: Jeff King, Matt Seitz (matseitz), git@vger.kernel.org,
Hilco Wijbenga
In-Reply-To: <CALWbr2wtAzz7yWb_Z_V0LFt5ddZcRSs7_rea2w=ghdC847mEyQ@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> I would simply go for:
>
> What Message-ID are you replying to (if any)?
>
> If I don't know what to answer, I would definitely not say y/yes/n/no,
> but press enter directly.
Sounds sensible (even though technically you reply to a message
that has that message ID, and not to a message ID ;-)).
Any better phrasing from others? If not, I'd say we adopt this
text.
Thanks.
^ permalink raw reply
* What's cooking in git.git (Jan 2013, #05; Fri, 11)
From: Junio C Hamano @ 2013-01-11 23:56 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'.
As usual, this cycle is expected to last for 8 to 10 weeks, with a
preview -rc0 sometime in the middle of next month.
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"]
* as/api-allocation-doc (2013-01-06) 1 commit
(merged to 'next' on 2013-01-08 at c80b544)
+ api-allocation-growing.txt: encourage better variable naming
* as/dir-c-cleanup (2012-12-28) 10 commits
(merged to 'next' on 2013-01-08 at 5aee090)
+ dir.c: rename free_excludes() to clear_exclude_list()
+ dir.c: refactor is_path_excluded()
+ dir.c: refactor is_excluded()
+ dir.c: refactor is_excluded_from_list()
+ dir.c: rename excluded() to is_excluded()
+ dir.c: rename excluded_from_list() to is_excluded_from_list()
+ dir.c: rename path_excluded() to is_path_excluded()
+ dir.c: rename cryptic 'which' variable to more consistent name
+ Improve documentation and comments regarding directory traversal API
+ api-directory-listing.txt: update to match code
(this branch is used by as/check-ignore.)
Refactor and generally clean up the directory traversal API
implementation.
* aw/rebase-am-failure-detection (2012-10-11) 1 commit
(merged to 'next' on 2013-01-07 at 9e2ee43)
+ rebase: Handle cases where format-patch fails
Originally merged to 'next' on 2013-01-02
Save output from format-patch command in a temporary file, just in
case it aborts, to give a better failure-case behaviour.
* jc/comment-cygwin-win32api-in-makefile (2013-01-06) 1 commit
(merged to 'next' on 2013-01-08 at dea04e8)
+ Makefile: add comment on CYGWIN_V15_WIN32API
* jc/maint-fmt-merge-msg-no-edit-lose-credit (2012-12-28) 1 commit
(merged to 'next' on 2013-01-07 at 497bf10)
+ merge --no-edit: do not credit people involved in the side branch
Originally merged to 'next' on 2013-01-02
Stop spending cycles to compute information to be placed on
commented lines in "merge --no-edit".
* jk/config-uname (2013-01-03) 1 commit
(merged to 'next' on 2013-01-08 at f986500)
+ Makefile: hoist uname autodetection to config.mak.uname
Move the bits to set fallback default based on the platform from
the main Makefile to a separate file, so that it can be included in
Makefiles in subdirectories.
* jl/interrupt-clone-remove-separate-git-dir (2013-01-05) 1 commit
(merged to 'next' on 2013-01-08 at 568f874)
+ clone: support atomic operation with --separate-git-dir
When "git clone --separate-git-dir" is interrupted, we failed to
remove the real location we created the repository.
* mz/pick-unborn (2012-12-23) 2 commits
(merged to 'next' on 2013-01-07 at c6c062b)
+ learn to pick/revert into unborn branch
+ tests: move test_cmp_rev to test-lib-functions
Originally merged to 'next' on 2013-01-02
Allows "git cherry-pick $commit" when you do not have any history
behind HEAD yet.
* nd/wildmatch (2013-01-01) 18 commits
(merged to 'next' on 2013-01-07 at 2a39f7d)
+ wildmatch: replace variable 'special' with better named ones
+ compat/fnmatch: respect NO_FNMATCH* even on glibc
+ wildmatch: fix "**" special case
+ t3070: Disable some failing fnmatch tests
+ test-wildmatch: avoid Windows path mangling
+ Support "**" wildcard in .gitignore and .gitattributes
+ wildmatch: make /**/ match zero or more directories
+ wildmatch: adjust "**" behavior
+ wildmatch: fix case-insensitive matching
+ wildmatch: remove static variable force_lower_case
+ wildmatch: make wildmatch's return value compatible with fnmatch
+ t3070: disable unreliable fnmatch tests
+ Integrate wildmatch to git
+ wildmatch: follow Git's coding convention
+ wildmatch: remove unnecessary functions
+ Import wildmatch from rsync
+ ctype: support iscntrl, ispunct, isxdigit and isprint
+ ctype: make sane_ctype[] const array
(this branch is used by nd/retire-fnmatch.)
Originally merged to 'next' on 2013-01-01
Allows pathname patterns in .gitignore and .gitattributes files
with double-asterisks "foo/**/bar" to match any number of directory
hierarchies.
* rs/leave-base-name-in-name-field-of-tar (2013-01-05) 1 commit
(merged to 'next' on 2013-01-08 at 98f325e)
+ archive-tar: split long paths more carefully
Improve compatibility with implementations of "tar" that do not
like empty name field in header (with the additional prefix field
holding everything).
* tb/test-shell-lint (2013-01-02) 1 commit
(merged to 'next' on 2013-01-07 at 0bca54a)
+ test: Add check-non-portable-shell.pl
Originally merged to 'next' on 2013-01-04
Check for common mistakes in the test scripts, based on simple
pattern-matching.
--------------------------------------------------
[New Topics]
* jk/maint-fast-import-doc-reorder (2013-01-09) 2 commits
(merged to 'next' on 2013-01-10 at 9f3950d)
+ git-fast-import(1): reorganise options
+ git-fast-import(1): combine documentation of --[no-]relative-marks
Will merge to 'master'.
* jk/shortlog-no-wrap-doc (2013-01-09) 1 commit
(merged to 'next' on 2013-01-10 at c79898a)
+ git-shortlog(1): document behaviour of zero-width wrap
Will merge to 'master'.
* rt/commit-cleanup-config (2013-01-10) 1 commit
- commit: make default of "cleanup" option configurable
Add a configuration variable to set default clean-up mode other
than "strip".
Will merge to 'next'.
* jc/custom-comment-char (2013-01-10) 1 commit
- Allow custom "comment char"
An illustration to show codepaths that need to be touched to change
the hint lines in the edited text to begin with something other
than '#'.
* jn/maint-trim-vim-contrib (2013-01-10) 1 commit
- contrib/vim: simplify instructions for old vim support
Will merge to 'next'.
* mz/reset-misc (2013-01-10) 22 commits
- reset [--mixed]: use diff-based reset whether or not pathspec was given
- [SQUASH???] script portability fixes
- reset: allow reset on unborn branch
- reset $sha1 $pathspec: require $sha1 only to be treeish
- reset [--mixed] --quiet: don't refresh index
- reset.c: finish entire cmd_reset() whether or not pathspec is given
- reset [--mixed]: don't write index file twice
- reset.c: move lock, write and commit out of update_index_refresh()
- reset.c: move update_index_refresh() call out of read_from_tree()
- reset: avoid redundant error message
- reset --keep: only write index file once
- reset.c: replace switch by if-else
- reset.c: share call to die_if_unmerged_cache()
- [SQUASH???] style fixes
- reset.c: extract function for updating {ORIG,}HEAD
- reset.c: remove unnecessary variable 'i'
- [SQUASH???] style fix
- reset.c: extract function for parsing arguments
- reset: don't allow "git reset -- $pathspec" in bare repo
- reset.c: pass pathspec around instead of (prefix, argv) pair
- reset $pathspec: exit with code 0 if successful
- reset $pathspec: no need to discard index
Various 'reset' optimizations and clean-ups, followed by a change
to allow "git reset" to work even on an unborn branch.
* pe/doc-email-env-is-trumped-by-config (2013-01-10) 1 commit
- git-commit-tree(1): correct description of defaults
In the precedence order, the environment variable $EMAIL comes
between the built-in default (i.e. taking value by asking the
system's gethostname() etc.) and the user.email configuration
variable; the documentation implied that it is stronger than the
configuration like $GIT_COMMITTER_EMAIL is, which is wrong.
Will merge to 'next'.
* ds/completion-silence-in-tree-path-probe (2013-01-11) 1 commit
- git-completion.bash: silence "not a valid object" errors
An internal ls-tree call made by completion code only to probe if
a path exists in the tree recorded in a commit object leaked error
messages when the path is not there. It is not an error at all and
should not be shown to the end user.
Will merge to 'next'.
* er/replace-cvsimport (2013-01-11) 4 commits
- t9604: fixup for new cvsimport
- t9600: fixup for new cvsimport
- t/lib-cvs.sh: allow cvsps version 3.x.
- cvsimport: rewrite to use cvsps 3.x to fix major bugs
Rewrite of cvsimport to talk with cvsps 3.x; this negatively
affects existing users of cvsimport that only have cvsps 2.x and
use -o, -M and -m options (they cannot use the old cvsimport with
old cvsps 2.x in the fall-back mode, even though these options are
supported by the old one), but it is not known how common they are.
For people who work with CVS histories complex enough to need cvsps
3.x to import correctly, this version should be a definite
improvement, and I would love to see small wrinkles in the new
implementation straightened out sooner so that we can merge it to
'next' (and from there on, apply incremental updates).
Help from people with Python experience would be appreciated in
reviewing and patching.
* nd/fetch-depth-is-broken (2013-01-11) 3 commits
- fetch: elaborate --depth action
- upload-pack: fix off-by-one depth calculation in shallow clone
- fetch: add --unshallow for turning shallow repo into complete one
"git fetch --depth" was broken in at least three ways. The
resulting history was deeper than specified by one commit, it was
unclear how to wipe the shallowness of the repository with the
command, and documentation was misleading.
Will merge to 'next'.
* jc/no-git-config-in-clone (2013-01-11) 1 commit
- clone: do not export and unexport GIT_CONFIG
We stopped paying attention to $GIT_CONFIG environment that points
at a single configuration file from any command other than "git config"
quite a while ago, but "git clone" internally set, exported, and
then unexported the variable during its operation unnecessarily.
--------------------------------------------------
[Stalled]
* jl/submodule-deinit (2012-12-04) 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 a reroll.
$gmane/212884
* 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]
* nz/send-email-headers-are-case-insensitive (2013-01-06) 1 commit
(merged to 'next' on 2013-01-10 at cf4c9c9)
+ git-send-email: treat field names as case-insensitively
When user spells "cc:" in lowercase in the fake "header" in the
trailer part, send-email failed to pick up the addresses from
there. As e-mail headers field names are case insensitive, this
script should follow suit and treat "cc:" and "Cc:" the same way.
* mk/complete-tcsh (2013-01-07) 1 commit
(merged to 'next' on 2013-01-11 at b8b30b1)
+ Prevent space after directories in tcsh completion
Update tcsh command line completion so that an unwanted space is
not added to a single directory name.
* dg/subtree-fixes (2013-01-08) 7 commits
- contrib/subtree: mkdir the manual directory if needed
- contrib/subtree: honor $(DESTDIR)
- contrib/subtree: fix synopsis and command help
- contrib/subtree: better error handling for "add"
- contrib/subtree: add --unannotate option
- contrib/subtree: use %B for split Subject/Body
- t7900: remove test number comments
contrib/subtree updates.
Will merge to 'next'.
* ap/log-mailmap (2013-01-10) 11 commits
(merged to 'next' on 2013-01-10 at 8544084)
+ log --use-mailmap: optimize for cases without --author/--committer search
+ log: add log.mailmap configuration option
+ log: grep author/committer using mailmap
+ test: add test for --use-mailmap option
+ log: add --use-mailmap option
+ pretty: use mailmap to display username and email
+ mailmap: add mailmap structure to rev_info and pp
+ mailmap: simplify map_user() interface
+ mailmap: remove email copy and length limitation
+ Use split_ident_line to parse author and committer
+ string-list: allow case-insensitive string list
Teach commands in the "log" family to optionally pay attention to
the mailmap.
* nd/upload-pack-shallow-must-be-commit (2013-01-08) 1 commit
(merged to 'next' on 2013-01-10 at a8b3ba5)
+ upload-pack: only accept commits from "shallow" line
A minor consistency check patch that does not have much relevance
to the real world.
* jc/blame-no-follow (2012-09-21) 2 commits
(merged to 'next' on 2013-01-10 at 201c7f4)
+ blame: pay attention to --no-follow
+ diff: accept --no-follow option
Teaches "--no-follow" option to "git blame" to disable its
whole-file rename detection.
* jc/push-2.0-default-to-simple (2013-01-08) 11 commits
(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 ;-).
* jk/unify-exit-code-by-receiving-signal (2013-01-06) 1 commit
(merged to 'next' on 2013-01-08 at 5ebf940)
+ run-command: encode signal death as a positive integer
The internal logic had to deal with two representations of a death
of a child process by a signal.
Will merge to 'master'.
* jn/xml-depends-on-asciidoc-conf (2013-01-06) 1 commit
(merged to 'next' on 2013-01-08 at 4faf8d4)
+ docs: manpage XML depends on asciidoc.conf
Will merge to 'master'.
* nd/clone-no-separate-git-dir-with-bare (2013-01-10) 1 commit
- clone: forbid --bare --separate-git-dir <dir>
Will merge to 'next'.
* nd/parse-pathspec (2013-01-11) 20 commits
- Convert more init_pathspec() to parse_pathspec()
- Convert add_files_to_cache to take struct pathspec
- Convert {read,fill}_directory to take struct pathspec
- Convert refresh_index to take struct pathspec
- Convert report_path_error to take struct pathspec
- checkout: convert read_tree_some to take struct pathspec
- Convert unmerge_cache to take struct pathspec
- Convert read_cache_preload() to take struct pathspec
- add: convert to use parse_pathspec
- archive: convert to use parse_pathspec
- ls-files: convert to use parse_pathspec
- rm: convert to use parse_pathspec
- checkout: convert to use parse_pathspec
- rerere: convert to use parse_pathspec
- status: convert to use parse_pathspec
- commit: convert to use parse_pathspec
- clean: convert to use parse_pathspec
- Export parse_pathspec() and convert some get_pathspec() calls
- Add parse_pathspec() that converts cmdline args to struct pathspec
- pathspec: save the non-wildcard length part
Uses the parsed pathspec structure in more places where we used to
use the raw "array of strings" pathspec.
Unfortunately, this conflicts a couple of topics in flight. I tried
to be careful while resolving conflicts, though.
* rs/zip-tests (2013-01-07) 4 commits
(merged to 'next' on 2013-01-08 at 8e37423)
+ t5003: check if unzip supports symlinks
+ t5000, t5003: move ZIP tests into their own script
+ t0024, t5000: use test_lazy_prereq for UNZIP
+ t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead
Updates zip tests to skip some that cannot be handled on platform
unzip.
I've renamed the t5002 in the original to t5003 to avoid name
clashes with another topic in flight.
Will merge to 'master'.
* rs/zip-with-uncompressed-size-in-the-header (2013-01-06) 1 commit
(merged to 'next' on 2013-01-08 at d9ec30e)
+ archive-zip: write uncompressed size into header even with streaming
Improve compatibility of our zip output to fill uncompressed size
in the header, which we can do without seeking back (even though it
should not be necessary).
Will merge to 'master'.
* jc/doc-maintainer (2013-01-03) 2 commits
(merged to 'next' on 2013-01-11 at f35d582)
+ howto/maintain: mark titles for asciidoc
+ Documentation: update "howto maintain git"
Describe tools for automation that were invented since this
document was originally written.
* fc/remote-testgit-feature-done (2012-10-29) 1 commit
(merged to 'next' on 2013-01-10 at 3132a60)
+ remote-testgit: properly check for errors
In the longer term, tightening rules is a good thing to do, and
because nobody who has worked in the remote helper area seems to be
interested in reviewing this, I would assume they do not think
such a retroactive tightening will affect their remote helpers. So
let's advance this topic to see what happens.
* mo/cvs-server-updates (2012-12-09) 18 commits
(merged to 'next' on 2013-01-08 at 75e2d11)
+ t9402: Use TABs for indentation
+ t9402: Rename check.cvsCount and check.list
+ t9402: Simplify git ls-tree
+ t9402: Add missing &&; Code style
+ t9402: No space after IO-redirection
+ t9402: Dont use test_must_fail cvs
+ t9402: improve check_end_tree() and check_end_full_tree()
+ t9402: sed -i is not portable
+ cvsserver Documentation: new cvs ... -r support
+ cvsserver: add t9402 to test branch and tag refs
+ cvsserver: support -r and sticky tags for most operations
+ cvsserver: Add version awareness to argsfromdir
+ cvsserver: generalize getmeta() to recognize commit refs
+ cvsserver: implement req_Sticky and related utilities
+ cvsserver: add misc commit lookup, file meta data, and file listing functions
+ cvsserver: define a tag name character escape mechanism
+ cvsserver: cleanup extra slashes in filename arguments
+ cvsserver: factor out git-log parsing logic
Various git-cvsserver updates.
Will cook in 'next' for a while to see if anybody screams.
* ap/status-ignored-in-ignored-directory (2013-01-07) 3 commits
(merged to 'next' on 2013-01-10 at 20f7476)
+ status: always report ignored tracked directories
(merged to 'next' on 2013-01-07 at 2a20b19)
+ git-status: Test --ignored behavior
+ dir.c: Make git-status --ignored more consistent
Output from "git status --ignored" showed an unexpected interaction
with "--untracked".
* as/check-ignore (2013-01-10) 12 commits
- t0008: avoid brace expansion
- add git-check-ignore sub-command
- setup.c: document get_pathspec()
- add.c: extract new die_if_path_beyond_symlink() for reuse
- add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse
- pathspec.c: rename newly public functions for clarity
- add.c: move pathspec matchers into new pathspec.c for reuse
- add.c: remove unused argument from validate_pathspec()
- dir.c: improve docs for match_pathspec() and match_pathspec_depth()
- dir.c: provide clear_directory() for reclaiming dir_struct memory
- dir.c: keep track of where patterns came from
- dir.c: use a single struct exclude_list per source of excludes
Add a new command "git check-ignore" for debugging .gitignore
files.
Will merge to 'next'.
* jc/format-patch-reroll (2013-01-03) 9 commits
(merged to 'next' on 2013-01-07 at 0e007e6)
+ format-patch: give --reroll-count a short synonym -v
+ format-patch: document and test --reroll-count
+ format-patch: add --reroll-count=$N option
+ get_patch_filename(): split into two functions
+ get_patch_filename(): drop "just-numbers" hack
+ get_patch_filename(): simplify function signature
+ builtin/log.c: stop using global patch_suffix
+ builtin/log.c: drop redundant "numbered_files" parameter from make_cover_letter()
+ builtin/log.c: drop unused "numbered" parameter from make_cover_letter()
Originally merged to 'next' on 2013-01-04
Teach "format-patch" to prefix v4- to its output files for the
fourth iteration of a patch series, to make it easier for the
submitter to keep separate copies for iterations.
Will merge to 'master'.
* nd/retire-fnmatch (2013-01-01) 7 commits
(merged to 'next' on 2013-01-07 at ab31f9b)
+ Makefile: add USE_WILDMATCH to use wildmatch as fnmatch
+ wildmatch: advance faster in <asterisk> + <literal> patterns
+ wildmatch: make a special case for "*/" with FNM_PATHNAME
+ test-wildmatch: add "perf" command to compare wildmatch and fnmatch
+ wildmatch: support "no FNM_PATHNAME" mode
+ wildmatch: make dowild() take arbitrary flags
+ wildmatch: rename constants and update prototype
Originally merged to 'next' on 2013-01-04
Replace our use of fnmatch(3) with a more feature-rich wildmatch.
A handful patches at the bottom have been moved to nd/wildmatch to
graduate as part of that branch, before this series solidifies.
Will cook in 'next' a bit longer than other topics.
* jc/merge-blobs (2012-12-26) 5 commits
(merged to 'next' on 2013-01-08 at 582ca38)
+ merge-tree: fix d/f conflicts
+ merge-tree: add comments to clarify what these functions are doing
+ merge-tree: lose unused "resolve_directories"
+ merge-tree: lose unused "flags" from merge_list
+ Which merge_file() function do you mean?
Update the disused merge-tree proof-of-concept code.
Will merge to 'master'.
* mb/gitweb-highlight-link-target (2012-12-20) 1 commit
- Highlight the link target line in Gitweb using CSS
Expecting a reroll.
$gmane/211935
* zk/clean-report-failure (2013-01-10) 2 commits
- [SQUASH???] style fixes
- git-clean: Display more accurate delete messages
"git clean" states what it is going to remove and then goes on to
remove it, but sometimes it only discovers things that cannot be
removed after recursing into a directory, which makes the output
confusing and even wrong.
Will merge to 'next' after squashing the style fix in.
* mp/complete-paths (2013-01-11) 1 commit
- git-completion.bash: add support for path completion
The completion script used to let the default completer to suggest
pathnames, which gave too many irrelevant choices (e.g. "git add"
would not want to add an unmodified path). Teach it to use a more
git-aware logic to enumerate only relevant ones.
Waiting for area-experts' help and review.
* bc/append-signed-off-by (2013-01-01) 12 commits
- t4014: do not use echo -n
- Unify appending signoff in format-patch, commit and sequencer
- format-patch: update append_signoff prototype
- format-patch: stricter S-o-b detection
- 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: 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
- sequencer.c: remove broken support for rfc2822 continuation in footer
Expecting a reroll.
$gmane/212507
^ permalink raw reply
* Re: missing objects -- prevention
From: Sitaram Chamarty @ 2013-01-12 1:09 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20130111164202.GB5219@sigill.intra.peff.net>
Thanks for the very detailed answer.
On Fri, Jan 11, 2013 at 10:12 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 11, 2013 at 04:40:38PM +0530, Sitaram Chamarty wrote:
>
>> I find a lot of info on how to recover from and/or repair a repo that
>> has missing (or corrupted) objects.
>>
>> What I need is info on common reasons (other than disk errors -- we've
>> checked for those) for such errors to occur, any preventive measures
>> we can take, and so on.
>
> I don't think any race can cause corruption of the object or packfiles
> because of the way they are written. At GitHub, every case of file-level
> corruption we've seen has been a filesystem issue.
>
> So I think the main thing systemic/race issue to worry about is missing
> objects. And since git only deletes objects during a prune (assuming you
> are using git-gc or "repack -A" so that repack cannot drop objects), I
> think prune is the only thing to watch out for.
No one runs anything manually under normal conditions. If there's any
gc happening, it's gc --auto.
> The --expire time saves us from the obvious races where you write object
> X but have not yet referenced it, and a simultaneous prune wants to
> delete it. However, it's possible that you have an old object that is
> unreferenced, but would become referenced as a result of an in-progress
> operation. For example, commit X is unreferenced and ready to be pruned,
> you build commit Y on top of it, but before you write the ref, git-prune
> removes X.
>
> The server-side version of that would happen via receive-pack, and is
> even more unlikely, because X would have to be referenced initially for
> us to advertise it. So it's something like:
>
> 1. The repo has a ref R pointing at commit X.
>
> 2. A user starts a push to another ref, Q, of commit Y that builds on
> X. Git advertises ref R, so the sender knows they do not need to
> send X, but only Y. The user then proceeds to send the packfile
> (which might take a very long time).
>
> 3. Meanwhile, another user deletes ref R. X becomes unreferenced.
The gitolite logs show that no deletion of refs has happened.
> 4. After step 3 but before step 2 has finished, somebody runs prune
> (this might sound unlikely, but if you kick off a "gc" job after
> each push, or after N pushes, it's not so unlikely). It sees that
> X is unreferenced, and it may very well be older than the --expire
> setting. Prune deletes X.
>
> 5. The packfile in (2) arrives, and receive-pack attempts to update
> the refs.
>
> So it's even a bit more unlikely than the local case, because
> receive-pack would not otherwise build on dangling objects. You have
> to race steps (2) and (3) just to create the situation.
>
> Then we have an extra protection in the form of
> check_everything_connected, which receive-pack runs before writing the
> refs into place. So if step 4 happens while the packfile is being sent
> (which is the most likely case, since it is the longest stretch of
> receive-pack's time), we would still catch it there and reject the push
> (annoying to the user, but the repo remains consistent).
>
> However, that's not foolproof. We might hit step 4 after we've checked
> that everything is connected but right before we write the ref. In which
> case we drop X, which has just become referenced, and we have a missing
> object.
>
> So I think it's possible. But I have never actually seen it in practice,
> and come up with this scenario only by brainstorming "what could go
> wrong" scenarios.
>
> This could be mitigated if there was a "proposed refs" storage.
> Receive-pack would write a note saying "consider Y for pruning purposes,
> but it's not really referenced yet", check connectivity for Y against
> the current refs, and then eventually write Y to its real ref (or reject
> it if there are problems). Prune would either run before the "proposed"
> note is written, which would mean it deletes X, but the connectivity
> check fails. Or it would run after, in which case it would leave X
> alone.
>
>> For example, can *any* type of network error or race condition cause
>> this? (Say, can one push writes an object, then fails an update
>> check, and a later push succeeds and races against a gc that removes
>> the unreachable object?) Or... the repo is pretty large -- about 6-7
>> GB, so could size cause a race that would not show up on a smaller
>> repo?
>
> The above is the only open issue I know about. I don't think it is
> dependent on repo size, but the window is widened for a really large
> push, because rev-list takes longer to run. It does not widen if you
> have receive.fsckobjects set, because that happens before we do the
> connectivity check (and the connectivity check is run in a sub-process,
> so the race timer starts when we exec rev-list, which may open and mmap
> packfiles or otherwise cache the presence of X in memory).
>
>> Anything else I can watch out for or caution the team about?
>
> That's the only open issue I know about for missing objects.
>
> There is a race with simultaneously deleting and packing refs. It
> doesn't cause object db corruption, but it will cause refs to "rewind"
> back to their packed versions. I have seen that one in practice (though
> relatively rare). I fixed it in b3f1280, which is not yet in any
> released version.
This is for the packed-refs file right? And it could result in a ref
getting deleted right?
I said above that the gitolite logs say no ref was deleted. What if
the ref "deletion" happened because of this race, making the rest of
your 4-step scenario above possible?
>> The symptom is usually a disk space crunch caused by tmp_pack_* files
>> left around by auto-gc. Presumably the missing objects failed the gc
>> and so it left the files around, and they eventually accumulate enough
>> to cause disk full errors. (If a gc ever succeeded, I suspect these
>> files would go away, but that requires manual intervention).
>
> Gc shouldn't be leaving around tmp_pack_* unless it is actually dying
> during the pack-objects phase. In my experience, stale tmp_pack_*
> objects are more likely a sign of a failed push (e.g., the client hangs
Oh. I did not know that! That explains why I sometimes saw that even
when there were less than 6700 loose objects (i.e., auto-gc should not
have kicked in at all).
> up in the middle, or fsck rejects the pack). We have historically left
fsck... you mean if I had 'receive.fsckObjects' true, right? I don't.
Should I? Would it help this overall situation? As I understand it,
thats only about the internals of each object to check corruption, and
cannot detect a *missing* object on the local object store.
> them in place to facilitate analysis of the failure.
>
> At GitHub, we've taken to just cleaning them up aggressively (I think
> after an hour), though I am tempted to put in an optional signal/atexit
OK; I'll do the same then. I suppose a cron job is the best way; I
didn't find any config for expiring these files.
Thanks again for your help. I'm going to treat it (for now) as a
disk/fs error after hearing from you about the other possibility I
mentioned above, although I find it hard to believe one repo can be
hit buy *two* races occurring together!
> handler to clean them up when index-pack fails. The forensics are
> occasionally interesting (e.g., finding weird fsck problems), but large
> pushes can waste a lot of disk space in the interim.
>
> -Peff
--
Sitaram
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Ben Aveling @ 2013-01-12 1:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: Antoine Pelisse, Jeff King, Matt Seitz (matseitz),
git@vger.kernel.org, Hilco Wijbenga
In-Reply-To: <7vmwwf9sx9.fsf@alter.siamese.dyndns.org>
On 12/01/2013 10:54 AM, Junio C Hamano wrote:
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> I would simply go for:
>>
>> What Message-ID are you replying to (if any)?
>>
>> If I don't know what to answer, I would definitely not say y/yes/n/no,
>> but press enter directly.
> Sounds sensible (even though technically you reply to a message
> that has that message ID, and not to a message ID ;-)).
>
> Any better phrasing from others? If not, I'd say we adopt this
> text.
I guess it depends on how much we mind if people accidentally miss the
message ID.
If we don't mind much, we could say something like:
What Message-ID are you replying to [Default=None]?
If we are concerned that when a Message-ID exists, it should be
provided, we could split to 2 questions:
Are you replying to an existing Message [Y/n]?
And then, if the answer is Y,
What Message-ID are you replying to?
Regards, Ben
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Junio C Hamano @ 2013-01-12 2:56 UTC (permalink / raw)
To: Ben Aveling
Cc: Antoine Pelisse, Jeff King, Matt Seitz (matseitz),
git@vger.kernel.org, Hilco Wijbenga
In-Reply-To: <50F0B643.20201@optusnet.com.au>
Ben Aveling <bena.001@optusnet.com.au> writes:
> On 12/01/2013 10:54 AM, Junio C Hamano wrote:
>> Antoine Pelisse <apelisse@gmail.com> writes:
>>
>>> I would simply go for:
>>>
>>> What Message-ID are you replying to (if any)?
>>>
>>> If I don't know what to answer, I would definitely not say y/yes/n/no,
>>> but press enter directly.
>> Sounds sensible (even though technically you reply to a message
>> that has that message ID, and not to a message ID ;-)).
>>
>> Any better phrasing from others? If not, I'd say we adopt this
>> text.
>
> I guess it depends on how much we mind if people accidentally miss the
> message ID.
>
> If we don't mind much, we could say something like:
>
> What Message-ID are you replying to [Default=None]?
>
>
> If we are concerned that when a Message-ID exists, it should be
> provided, we could split to 2 questions:
>
> Are you replying to an existing Message [Y/n]?
>
> And then, if the answer is Y,
>
> What Message-ID are you replying to?
Eewww. Now we come back to full circles.
It sometimes helps to follow the in-reply-to chain to see what has
already been said in the thread, I guess ;-)
^ permalink raw reply
* [PATCH] t9605: test for cvsps commit ordering bug
From: Chris Rorvick @ 2013-01-12 4:13 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Chris Rorvick
Import of a trivial CVS repository fails due to a cvsps bug. Given the
following series of commits:
timestamp a b c message
------------------- --- --- --- -------
2012/12/12 21:09:39 1.1 changes are done
2012/12/12 21:09:44 1.1 changes
2012/12/12 21:09:46 1.2 changes
2012/12/12 21:09:50 1.1 1.3 changes are done
cvsps mangles the commit ordering (edited for brevity):
---------------------
PatchSet 1
Date: 2012/12/12 15:09:39
Log:
changes are done
Members:
a:INITIAL->1.1
b:INITIAL->1.1
c:1.2->1.3
---------------------
PatchSet 2
Date: 2012/12/12 15:09:44
Log:
changes
Members:
c:INITIAL->1.1
---------------------
PatchSet 3
Date: 2012/12/12 15:09:46
Log:
changes
Members:
c:1.1->1.2
This is seen in cvsps versions 2.x and up through at least 3.7.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
Ran into this recently. No branching and no "criss cross" timestamps,
just lazy commit messages. And it magically backed out a bug fix.
This applies on top of master. With minor modifications I've tested it
with Eric's latest code and confirmed the bug still exists.
Chris
t/t9605-cvsimport-commit-order.sh | 25 +++++++++++++++
t/t9605/cvsroot/.gitattributes | 1 +
t/t9605/cvsroot/CVSROOT/.gitignore | 2 ++
t/t9605/cvsroot/module/a,v | 24 +++++++++++++++
t/t9605/cvsroot/module/b,v | 24 +++++++++++++++
t/t9605/cvsroot/module/c,v | 62 ++++++++++++++++++++++++++++++++++++++
6 files changed, 138 insertions(+)
create mode 100755 t/t9605-cvsimport-commit-order.sh
create mode 100644 t/t9605/cvsroot/.gitattributes
create mode 100644 t/t9605/cvsroot/CVSROOT/.gitignore
create mode 100644 t/t9605/cvsroot/module/a,v
create mode 100644 t/t9605/cvsroot/module/b,v
create mode 100644 t/t9605/cvsroot/module/c,v
diff --git a/t/t9605-cvsimport-commit-order.sh b/t/t9605-cvsimport-commit-order.sh
new file mode 100755
index 0000000..ab4042e
--- /dev/null
+++ b/t/t9605-cvsimport-commit-order.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+test_description='git cvsimport commit order'
+. ./lib-cvs.sh
+
+setup_cvs_test_repository t9605
+
+test_expect_success 'checkout with CVS' '
+
+ echo CVSROOT=$CVSROOT &&
+ cvs checkout -d module-cvs module
+'
+
+test_expect_failure 'import into git (commit order mangled)' '
+
+ git cvsimport -R -a -p"-x" -C module-git module &&
+ (
+ cd module-git &&
+ git merge origin
+ ) &&
+ test_cmp module-cvs/c module-git/c &&
+false
+'
+
+test_done
diff --git a/t/t9605/cvsroot/.gitattributes b/t/t9605/cvsroot/.gitattributes
new file mode 100644
index 0000000..562b12e
--- /dev/null
+++ b/t/t9605/cvsroot/.gitattributes
@@ -0,0 +1 @@
+* -whitespace
diff --git a/t/t9605/cvsroot/CVSROOT/.gitignore b/t/t9605/cvsroot/CVSROOT/.gitignore
new file mode 100644
index 0000000..3bb9b34
--- /dev/null
+++ b/t/t9605/cvsroot/CVSROOT/.gitignore
@@ -0,0 +1,2 @@
+history
+val-tags
diff --git a/t/t9605/cvsroot/module/a,v b/t/t9605/cvsroot/module/a,v
new file mode 100644
index 0000000..6455911
--- /dev/null
+++ b/t/t9605/cvsroot/module/a,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.39; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file a
+@
diff --git a/t/t9605/cvsroot/module/b,v b/t/t9605/cvsroot/module/b,v
new file mode 100644
index 0000000..55545c8
--- /dev/null
+++ b/t/t9605/cvsroot/module/b,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file b
+@
diff --git a/t/t9605/cvsroot/module/c,v b/t/t9605/cvsroot/module/c,v
new file mode 100644
index 0000000..d3eac77
--- /dev/null
+++ b/t/t9605/cvsroot/module/c,v
@@ -0,0 +1,62 @@
+head 1.3;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.3
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next 1.2;
+
+1.2
+date 2012.12.12.21.09.46; author tester; state Exp;
+branches;
+next 1.1;
+
+1.1
+date 2012.12.12.21.09.44; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.3
+log
+@changes are done
+@
+text
+@file c
+line two
+line three
+line four
+line five
+@
+
+
+1.2
+log
+@changes
+@
+text
+@d2 4
+a5 4
+line 2
+line 3
+line 4
+line 5
+@
+
+
+1.1
+log
+@changes
+@
+text
+@d2 4
+@
+
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* [PATCH v2] t9605: test for cvsps commit ordering bug
From: Chris Rorvick @ 2013-01-12 4:39 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Chris Rorvick
Import of a trivial CVS repository fails due to a cvsps bug. Given the
following series of commits:
timestamp a b c message
------------------- --- --- --- -------
2012/12/12 21:09:39 1.1 changes are done
2012/12/12 21:09:44 1.1 changes
2012/12/12 21:09:46 1.2 changes
2012/12/12 21:09:50 1.1 1.3 changes are done
cvsps mangles the commit ordering (edited for brevity):
---------------------
PatchSet 1
Date: 2012/12/12 15:09:39
Log:
changes are done
Members:
a:INITIAL->1.1
b:INITIAL->1.1
c:1.2->1.3
---------------------
PatchSet 2
Date: 2012/12/12 15:09:44
Log:
changes
Members:
c:INITIAL->1.1
---------------------
PatchSet 3
Date: 2012/12/12 15:09:46
Log:
changes
Members:
c:1.1->1.2
This is seen in cvsps versions 2.x and up through at least 3.7.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
It actually does fail without the "&& false" at the end. :-P Sorry for
the noise.
t/t9605-cvsimport-commit-order.sh | 24 +++++++++++++++
t/t9605/cvsroot/.gitattributes | 1 +
t/t9605/cvsroot/CVSROOT/.gitignore | 2 ++
t/t9605/cvsroot/module/a,v | 24 +++++++++++++++
t/t9605/cvsroot/module/b,v | 24 +++++++++++++++
t/t9605/cvsroot/module/c,v | 62 ++++++++++++++++++++++++++++++++++++++
6 files changed, 137 insertions(+)
create mode 100755 t/t9605-cvsimport-commit-order.sh
create mode 100644 t/t9605/cvsroot/.gitattributes
create mode 100644 t/t9605/cvsroot/CVSROOT/.gitignore
create mode 100644 t/t9605/cvsroot/module/a,v
create mode 100644 t/t9605/cvsroot/module/b,v
create mode 100644 t/t9605/cvsroot/module/c,v
diff --git a/t/t9605-cvsimport-commit-order.sh b/t/t9605-cvsimport-commit-order.sh
new file mode 100755
index 0000000..86aafd1
--- /dev/null
+++ b/t/t9605-cvsimport-commit-order.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='git cvsimport commit order'
+. ./lib-cvs.sh
+
+setup_cvs_test_repository t9605
+
+test_expect_success 'checkout with CVS' '
+
+ echo CVSROOT=$CVSROOT &&
+ cvs checkout -d module-cvs module
+'
+
+test_expect_failure 'import into git (commit order mangled)' '
+
+ git cvsimport -R -a -p"-x" -C module-git module &&
+ (
+ cd module-git &&
+ git merge origin
+ ) &&
+ test_cmp module-cvs/c module-git/c
+'
+
+test_done
diff --git a/t/t9605/cvsroot/.gitattributes b/t/t9605/cvsroot/.gitattributes
new file mode 100644
index 0000000..562b12e
--- /dev/null
+++ b/t/t9605/cvsroot/.gitattributes
@@ -0,0 +1 @@
+* -whitespace
diff --git a/t/t9605/cvsroot/CVSROOT/.gitignore b/t/t9605/cvsroot/CVSROOT/.gitignore
new file mode 100644
index 0000000..3bb9b34
--- /dev/null
+++ b/t/t9605/cvsroot/CVSROOT/.gitignore
@@ -0,0 +1,2 @@
+history
+val-tags
diff --git a/t/t9605/cvsroot/module/a,v b/t/t9605/cvsroot/module/a,v
new file mode 100644
index 0000000..6455911
--- /dev/null
+++ b/t/t9605/cvsroot/module/a,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.39; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file a
+@
diff --git a/t/t9605/cvsroot/module/b,v b/t/t9605/cvsroot/module/b,v
new file mode 100644
index 0000000..55545c8
--- /dev/null
+++ b/t/t9605/cvsroot/module/b,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file b
+@
diff --git a/t/t9605/cvsroot/module/c,v b/t/t9605/cvsroot/module/c,v
new file mode 100644
index 0000000..d3eac77
--- /dev/null
+++ b/t/t9605/cvsroot/module/c,v
@@ -0,0 +1,62 @@
+head 1.3;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.3
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next 1.2;
+
+1.2
+date 2012.12.12.21.09.46; author tester; state Exp;
+branches;
+next 1.1;
+
+1.1
+date 2012.12.12.21.09.44; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.3
+log
+@changes are done
+@
+text
+@file c
+line two
+line three
+line four
+line five
+@
+
+
+1.2
+log
+@changes
+@
+text
+@d2 4
+a5 4
+line 2
+line 3
+line 4
+line 5
+@
+
+
+1.1
+log
+@changes
+@
+text
+@d2 4
+@
+
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Eric S. Raymond @ 2013-01-12 5:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwwfbjv3.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> Yeah, it is OK to _discourage_ its use, but to me it looks like that
> the above is a fairly subjective policy decision, not something I
> should let you impose on the users of the old cvsimport, which you
> do not seem to even treat as your users.
Er. You still don't seem to grasp the fundamentals of this
situation. I'm not imposing any damn thing on the users. What's
imposing is the fact that cvsps-2.x and the Perl cvsimport are both
individually and collectively *broken right now*, and within a few
months the Perl git-cvsimport is going to cease even pretending to
work. I'm trying to *fix that problem* as best I can, fixing it
required two radical rewrites, and criticizing me for not emulating
every last detail and misfeature immediately is every bit as pointless
and annoying as arguing about the fabric on the deck chairs while the
ship is sinking.
To put it bluntly, you should be grateful to be getting back any
functionality at all - because the alternative is that the Perl
git-cvsimport will hang out in your tree as a dead piece of cruft.
Your choice is between making it easy for me replace it with minimum
disruption now and hoping for someone else to replace it months from
now after you've had a bunch of unhappy users bitching at you.
So let me be more direct. I think the -M and -m options are
sufficiently bad ideas that I am *not willing* to put in the quite
large amount of effort that would be required to implement them in cvsps
or parsecvs. That would be a bad use of my time.
This is not the case with -o; that might be a good idea if I
understood it. This is also not like the 2.x fallback; I thought that
was a bad idea (because it would be better for users that the
combination break in an obvious way than continue breaking in a silent
one), but it was a small enough effort that I was willing to do it
anyway to keep the git maintainer happy. The effort to fix the quoting
bugs is even easier for me to justify; they are actual bugs.
Those are my engineering judgments; go ahead and call them
"subjective" if you like, but neither the facts nor my judgment will
change on that account.
> The "major" in my sentence was from your description (the bugs you
> fixed), and not about the new ones you still have in this draft. I
> did not mean to say that you are trading fixes to "major" bugs with
> different "major" bugs.
OK, thank you. In the future I will try to bear in mind that English
is not your primary language when I evaluate statements that seems a bit
offensive.
So what's your next bid? Note that you can't increase my friction and
hassle costs much more before I give up and let you deal with the
consequences without me. I want to do the right thing, but I have
more other projects clamoring for my attention than you could easily
guess. I need to get git-cvsimport *finished* and off my hands -
I may already have given it more time than I really should have.
So give me your minimum list of deliverables before you'll merge,
please, and then stick to it. I assume fixes for the quoting bugs
will be on that list.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Junio C Hamano @ 2013-01-12 5:20 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <1357875152-19899-1-git-send-email-gitster@pobox.com>
I cloned git://gitorious.org/cvsps/cvsps.git and installed cvsps-3.7
at c2ce6cc (More fun with test loads, sigh. Timezones suck.,
2013-01-09) earlier on my $PATH, and tried to run t96xx series with
this patch applied on top of Git 1.8.1.
The first thing I noticed was that all the tests were skipped.
A patch to t/lib-cvs.sh might be sufficient,
--------------------- >8 -------------------------
diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index 44263ad..423953f 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -17,6 +17,8 @@ cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
case "$cvsps_version" in
2.1 | 2.2*)
;;
+3.*)
+ ;;
'')
skip_all='skipping cvsimport tests, cvsps not found'
test_done
--------------------- 8< -------------------------
but I didn't check more than "now it seems not to skip them".
And here is what I got:
--------------------- >8 -------------------------
Test Summary Report
-------------------
t9600-cvsimport.sh (Wstat: 256 Tests: 15 Failed: 9)
Failed tests: 4-6, 8-9, 11-13, 15
Non-zero exit status: 1
t9601-cvsimport-vendor-branch.sh (Wstat: 256 Tests: 9 Failed: 8)
Failed tests: 1-4, 6-9
Non-zero exit status: 1
t9602-cvsimport-branches-tags.sh (Wstat: 256 Tests: 11 Failed: 5)
Failed tests: 1-3, 7, 9
Non-zero exit status: 1
t9604-cvsimport-timestamps.sh (Wstat: 256 Tests: 2 Failed: 2)
Failed tests: 1-2
Non-zero exit status: 1
Files=5, Tests=38, 5 wallclock secs ( 0.05 usr 0.01 sys + 0.49
cusr 0.16 csys = 0.71 CPU)
Result: FAIL
--------------------- 8< -------------------------
A funny thing was that without cvsps-3.7 on $PATH (which means I am
getting distro packaged cvsps 2.1), I got identical errors. Looking
at the log message, it seems that you meant to remove t960[123], so
perhaps the patch simply forgot to remove 9601 and 9602?
As neither test runs "git cvsimport" with -o/-m/-M options, ideally
we should be able to pass them with and without having cvsps-3.x.
Not passing them without cvsps-3.x would mean that the fallback mode
of rewritten cvsimport is not working as expected. Not passing them
with cvsps-3.x may mean the tests were expecting a wrong conversion
result, or they uncover bugs in the replacement cvsimport.
t9600 fails with "-a is no longer supported", even without having
cvsps-3.x on the $PATH (i.e. attempting to use the fallback). I
wonder if this is an option the updated cvsimport would want to
simply ignore?
It is a way to tell the old cvsps/cvsimport to disable its
heuristics to ignore commits made within the last 10 minutes (this
is done in the hope of waiting for the per-file nature of CVS
commits to stabilize, IIUC); the user tells the command that he
knows that the CVS repository is now quiescent and it is safe to
import the whole thing.
If the updated cvsps can identify the changeset more reliably and it
no longer needs "-a" option, it may be more helpful to the users to
migrate their script if it allowed, warned and then ignored the
option. It certainly would help sharing of this test script between
runs that use the old and new cvsps as backends.
t9601 (after resurrecting the t/t9601/cvsroot directory) fails in an
interesting way.
--------------------- >8 -------------------------
$ sh t9601-cvsimport-vendor-branch.sh -i -v
Initialized empty Git repository in /git/git.build/t/trash directory.t9601-cvsimport-vendor-branch/.git/
expecting success:
git cvsimport -C module-git module
Traceback (most recent call last):
File "/git/git.build/git-cvsimport", line 262, in <module>
subprocess.check_output("cvsps -V 2>/dev/null", shell=True)
AttributeError: 'module' object has no attribute 'check_output'
not ok - 1 import a module with a vendor branch
--------------------- 8< -------------------------
Apparently, the copy of "subprocess.py" I have does not give us the
check_output thing:
--------------------- >8 -------------------------
$ python
Python 2.6.6 (r266:84292, Dec 26 2010, 22:31:48)
[GCC 4.4.5] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> dir(subprocess)
['CalledProcessError', 'MAXFD', 'PIPE', 'Popen', 'STDOUT', '__all__', '__builtins__', '__doc__', '__file__', '__name__', '__package__', '_active', '_cleanup', '_demo_posix', '_demo_windows', '_eintr_retry_call', 'call', 'check_call', 'errno', 'fcntl', 'gc', 'list2cmdline', 'mswindows', 'os', 'pickle', 'select', 'signal', 'sys', 'traceback', 'types']
--------------------- 8< -------------------------
The story is the same for t9602 and t9603 (again after resurrecting
the necessary files).
http://docs.python.org/2/library/subprocess.html tells me that
check_output has first become available in 2.7.
So... does this mean that we now set the minimum required version of
Python to 2.7? I dunno.
^ permalink raw reply related
* [PATCH] t/t960[123]: remove leftover scripts
From: Junio C Hamano @ 2013-01-12 5:38 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git, Chris Rorvick
In-Reply-To: <7v62339du4.fsf@alter.siamese.dyndns.org>
The rewrite patch was supposed to remove these scripts, but somehow
we ended up removing only the supporting files for them but not the
test script themselves. Remove them for real.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* I'll queue this on top of your patch together with a few fix-up
patches from Chris Rorvick. This may have been caused by your
private patch e-mail mangled somewhere between us before I resent
your patch, or perhaps you simply may have forgot to remove them,
but at this point I do not really care where these deletions were
lost---the only thing I care about is to make sure that you
_meant_ to remove them in your patch (i.e. if you didn't mean to,
then I am further breaking the tests in a way you did not intend
to), so I'd appreciate either "Yup, these three files should be
removed", or "No, they should stay; removal of their supporting
data is no longer needed" from you (I and this patch expect the
former, of course).
By the way, Chris, we'll need your Sign-off on the three paches
(t/lib-cvs.sh fix to allow cvsps v3, t9600 fix and t9604 fix).
t/t9601-cvsimport-vendor-branch.sh | 85 --------------------------------------
t/t9602-cvsimport-branches-tags.sh | 78 ----------------------------------
t/t9603-cvsimport-patchsets.sh | 39 -----------------
3 files changed, 202 deletions(-)
delete mode 100755 t/t9601-cvsimport-vendor-branch.sh
delete mode 100755 t/t9602-cvsimport-branches-tags.sh
delete mode 100755 t/t9603-cvsimport-patchsets.sh
diff --git a/t/t9601-cvsimport-vendor-branch.sh b/t/t9601-cvsimport-vendor-branch.sh
deleted file mode 100755
index 827d39f..0000000
--- a/t/t9601-cvsimport-vendor-branch.sh
+++ /dev/null
@@ -1,85 +0,0 @@
-#!/bin/sh
-
-# Description of the files in the repository:
-#
-# imported-once.txt:
-#
-# Imported once. 1.1 and 1.1.1.1 should be identical.
-#
-# imported-twice.txt:
-#
-# Imported twice. HEAD should reflect the contents of the
-# second import (i.e., have the same contents as 1.1.1.2).
-#
-# imported-modified.txt:
-#
-# Imported, then modified on HEAD. HEAD should reflect the
-# modification.
-#
-# imported-modified-imported.txt:
-#
-# Imported, then modified on HEAD, then imported again.
-#
-# added-imported.txt,v:
-#
-# Added with 'cvs add' to create 1.1, then imported with
-# completely different contents to create 1.1.1.1, therefore the
-# vendor branch was never the default branch.
-#
-# imported-anonymously.txt:
-#
-# Like imported-twice.txt, but with a vendor branch whose branch
-# tag has been removed.
-
-test_description='git cvsimport handling of vendor branches'
-. ./lib-cvs.sh
-
-setup_cvs_test_repository t9601
-
-test_expect_success PERL 'import a module with a vendor branch' '
-
- git cvsimport -C module-git module
-
-'
-
-test_expect_success PERL 'check HEAD out of cvs repository' 'test_cvs_co master'
-
-test_expect_success PERL 'check master out of git repository' 'test_git_co master'
-
-test_expect_success PERL 'check a file that was imported once' '
-
- test_cmp_branch_file master imported-once.txt
-
-'
-
-test_expect_failure PERL 'check a file that was imported twice' '
-
- test_cmp_branch_file master imported-twice.txt
-
-'
-
-test_expect_success PERL 'check a file that was imported then modified on HEAD' '
-
- test_cmp_branch_file master imported-modified.txt
-
-'
-
-test_expect_success PERL 'check a file that was imported, modified, then imported again' '
-
- test_cmp_branch_file master imported-modified-imported.txt
-
-'
-
-test_expect_success PERL 'check a file that was added to HEAD then imported' '
-
- test_cmp_branch_file master added-imported.txt
-
-'
-
-test_expect_success PERL 'a vendor branch whose tag has been removed' '
-
- test_cmp_branch_file master imported-anonymously.txt
-
-'
-
-test_done
diff --git a/t/t9602-cvsimport-branches-tags.sh b/t/t9602-cvsimport-branches-tags.sh
deleted file mode 100755
index e1db323..0000000
--- a/t/t9602-cvsimport-branches-tags.sh
+++ /dev/null
@@ -1,78 +0,0 @@
-#!/bin/sh
-
-# A description of the repository used for this test can be found in
-# t9602/README.
-
-test_description='git cvsimport handling of branches and tags'
-. ./lib-cvs.sh
-
-setup_cvs_test_repository t9602
-
-test_expect_success PERL 'import module' '
-
- git cvsimport -C module-git module
-
-'
-
-test_expect_success PERL 'test branch master' '
-
- test_cmp_branch_tree master
-
-'
-
-test_expect_success PERL 'test branch vendorbranch' '
-
- test_cmp_branch_tree vendorbranch
-
-'
-
-test_expect_failure PERL 'test branch B_FROM_INITIALS' '
-
- test_cmp_branch_tree B_FROM_INITIALS
-
-'
-
-test_expect_failure PERL 'test branch B_FROM_INITIALS_BUT_ONE' '
-
- test_cmp_branch_tree B_FROM_INITIALS_BUT_ONE
-
-'
-
-test_expect_failure PERL 'test branch B_MIXED' '
-
- test_cmp_branch_tree B_MIXED
-
-'
-
-test_expect_success PERL 'test branch B_SPLIT' '
-
- test_cmp_branch_tree B_SPLIT
-
-'
-
-test_expect_failure PERL 'test tag vendortag' '
-
- test_cmp_branch_tree vendortag
-
-'
-
-test_expect_success PERL 'test tag T_ALL_INITIAL_FILES' '
-
- test_cmp_branch_tree T_ALL_INITIAL_FILES
-
-'
-
-test_expect_failure PERL 'test tag T_ALL_INITIAL_FILES_BUT_ONE' '
-
- test_cmp_branch_tree T_ALL_INITIAL_FILES_BUT_ONE
-
-'
-
-test_expect_failure PERL 'test tag T_MIXED' '
-
- test_cmp_branch_tree T_MIXED
-
-'
-
-
-test_done
diff --git a/t/t9603-cvsimport-patchsets.sh b/t/t9603-cvsimport-patchsets.sh
deleted file mode 100755
index 52034c8..0000000
--- a/t/t9603-cvsimport-patchsets.sh
+++ /dev/null
@@ -1,39 +0,0 @@
-#!/bin/sh
-
-# Structure of the test cvs repository
-#
-# Message File:Content Commit Time
-# Rev 1 a: 1.1 2009-02-21 19:11:43 +0100
-# Rev 2 a: 1.2 b: 1.1 2009-02-21 19:11:14 +0100
-# Rev 3 b: 1.2 2009-02-21 19:11:43 +0100
-#
-# As you can see the commit of Rev 3 has the same time as
-# Rev 1 this leads to a broken import because of a cvsps
-# bug.
-
-test_description='git cvsimport testing for correct patchset estimation'
-. ./lib-cvs.sh
-
-setup_cvs_test_repository t9603
-
-test_expect_failure 'import with criss cross times on revisions' '
-
- git cvsimport -p"-x" -C module-git module &&
- (cd module-git &&
- git log --pretty=format:%s > ../actual-master &&
- git log A~2..A --pretty="format:%s %ad" -- > ../actual-A &&
- echo "" >> ../actual-master &&
- echo "" >> ../actual-A
- ) &&
- echo "Rev 4
-Rev 3
-Rev 2
-Rev 1" > expect-master &&
- test_cmp actual-master expect-master &&
-
- echo "Rev 5 Branch A Wed Mar 11 19:09:10 2009 +0000
-Rev 4 Branch A Wed Mar 11 19:03:52 2009 +0000" > expect-A &&
- test_cmp actual-A expect-A
-'
-
-test_done
--
1.8.1.421.g6236851
^ permalink raw reply related
* [BUG] Possible bug in `remote set-url --add --push`
From: Jardel Weyrich @ 2013-01-12 5:44 UTC (permalink / raw)
To: git
Hi,
I believe `remote set-url --add --push` has a bug. Performed tests
with v1.8.0.1 and v1.8.1 (Mac OS X).
Quoting the relevant part of the documentation:
> set-url
> Changes URL remote points to. Sets first URL remote points to matching regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If <oldurl> doesn’t match any URL, error occurs and nothing is changed.
>
> With --push, push URLs are manipulated instead of fetch URLs.
> With --add, instead of changing some URL, new URL is added.
> With --delete, instead of changing some URL, all URLs matching regex <url> are deleted. Trying to delete all non-push URLs is an error.
Here are some steps to reproduce:
1. Show the remote URLs
jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test (push)
2. Add a new push URL for origin
jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \
/Volumes/sandbox/test_clone2
3. Check what happened
jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test_clone2 (push)
4. Missing an URL? Re-add the original one
jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \
/Volumes/sandbox/test
5. Check what happened, again
jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test_clone2 (push)
origin /Volumes/sandbox/test (push)
In step 2, Git replaced the original push URL instead of adding a new
one. But it seems to happen only the first time I use `remote set-url
--add --push`. Re-adding the original URL using the same command seems
to work properly.
And FWIW, if I delete (with "set-url --delete") both URLs push, Git
restores the original URL.
Please, could someone try to reproduce?
- jw
^ permalink raw reply
* [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-01-12 5:50 UTC (permalink / raw)
To: git; +Cc: tboegi
The test Makefile has a default set of lint tests which are run
as part of "make test".
The macro TEST_LINT defaults to "test-lint-duplicates test-lint-executable".
Add test-lint-shell-syntax here, to detect non-portable shell syntax early.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index 1923cc1..6fa2b80 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -13,7 +13,7 @@ TAR ?= $(TAR)
RM ?= rm -f
PROVE ?= prove
DEFAULT_TEST_TARGET ?= test
-TEST_LINT ?= test-lint-duplicates test-lint-executable
+TEST_LINT ?= test-lint-duplicates test-lint-executable test-lint-shell-syntax
# Shell quote;
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
--
1.8.0.197.g5a90748
^ permalink raw reply related
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-12 6:00 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <201301120650.46479.tboegi@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> The test Makefile has a default set of lint tests which are run
> as part of "make test".
>
> The macro TEST_LINT defaults to "test-lint-duplicates test-lint-executable".
>
> Add test-lint-shell-syntax here, to detect non-portable shell syntax early.
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
As I said already, I do not want to do this yet without further
reduction of false positives.
Addition of the shell script test was a good starting point, but as
it stands, it still is too brittle and will trigger on something
even trivially innouous, like this:
test_expect_success 'no issues' '
cat >test.file <<-\EOF &&
which is the right way?
EOF
'
> t/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/Makefile b/t/Makefile
> index 1923cc1..6fa2b80 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -13,7 +13,7 @@ TAR ?= $(TAR)
> RM ?= rm -f
> PROVE ?= prove
> DEFAULT_TEST_TARGET ?= test
> -TEST_LINT ?= test-lint-duplicates test-lint-executable
> +TEST_LINT ?= test-lint-duplicates test-lint-executable test-lint-shell-syntax
>
> # Shell quote;
> SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
^ permalink raw reply
* Re: [PATCH v2 03/21] Export parse_pathspec() and convert some get_pathspec() calls
From: Duy Nguyen @ 2013-01-12 6:00 UTC (permalink / raw)
To: Matt Kraai, git, Junio C Hamano
In-Reply-To: <20130111175644.GA12359@ftbfs.org>
On Sat, Jan 12, 2013 at 12:56 AM, Matt Kraai <kraai@ftbfs.org> wrote:
> On Fri, Jan 11, 2013 at 06:20:57PM +0700, Nguyễn Thái Ngọc Duy wrote:
>> +#define PATHSPEC_FROMTOP (1<<0)
>
> The previous commit introduces a use of this macro in get_pathspec.
> Should this be defined by that commit instead?
This macro is already defined in setup.c when parse_pathspec is
introduced. I wanted to move it from setup.c to cache.h but forgot to
remove the original definition. Will fix.
>> @@ -266,9 +266,9 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
>> * Given command line arguments and a prefix, convert the input to
>> * pathspec. die() if any magic other than ones in magic_mask.
>> */
>> -static void parse_pathspec(struct pathspec *pathspec,
>> - unsigned magic_mask, unsigned flags,
>> - const char *prefix, const char **argv)
>> +void parse_pathspec(struct pathspec *pathspec,
>> + unsigned magic_mask, unsigned flags,
>
> The prototype for this function uses just "magic" instead of
> "magic_mask". Should they be consistent?
Definitely. Will fix.
--
Duy
^ permalink raw reply
* Re: [PATCH] t/t960[123]: remove leftover scripts
From: Chris Rorvick @ 2013-01-12 6:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric S. Raymond, git
In-Reply-To: <7v1udr9d0g.fsf_-_@alter.siamese.dyndns.org>
On Fri, Jan 11, 2013 at 11:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> By the way, Chris, we'll need your Sign-off on the three paches
> (t/lib-cvs.sh fix to allow cvsps v3, t9600 fix and t9604 fix).
Sure. I was just maintaining them for myself but thought I'd share
when I saw the follow-up patch. Didn't think to amend them.
Chris
^ permalink raw reply
* [PATCH v2 0/3] fixup remaining cvsimport tests
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
Reroll w/ sign-off.
Chris Rorvick (3):
t/lib-cvs.sh: allow cvsps version 3.x.
t9600: fixup for new cvsimport
t9604: fixup for new cvsimport
t/lib-cvs.sh | 2 +-
t/t9600-cvsimport.sh | 10 ++++------
t/t9604-cvsimport-timestamps.sh | 5 ++---
3 files changed, 7 insertions(+), 10 deletions(-)
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply
* [PATCH v2 1/3] t/lib-cvs.sh: allow cvsps version 3.x.
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
In-Reply-To: <1357971703-28513-1-git-send-email-chris@rorvick.com>
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
t/lib-cvs.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index 44263ad..b55e861 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -15,7 +15,7 @@ export CVS
cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
case "$cvsps_version" in
-2.1 | 2.2*)
+2.1 | 2.2* | 3.*)
;;
'')
skip_all='skipping cvsimport tests, cvsps not found'
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* [PATCH v2 2/3] t9600: fixup for new cvsimport
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
In-Reply-To: <1357971703-28513-1-git-send-email-chris@rorvick.com>
cvsimport no longer supports -a (import all commits including recent ones)
and no longer uses the 'origin' branch by default for imports.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
t/t9600-cvsimport.sh | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 4c384ff..14f54d5 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -44,7 +44,7 @@ EOF
test_expect_success PERL 'import a trivial module' '
- git cvsimport -a -R -z 0 -C module-git module &&
+ git cvsimport -R -z 0 -C module-git module &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
'
@@ -90,8 +90,7 @@ test_expect_success PERL 'update git module' '
(cd module-git &&
git config cvsimport.trackRevisions true &&
- git cvsimport -a -z 0 module &&
- git merge origin
+ git cvsimport -z 0 module
) &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
@@ -119,8 +118,7 @@ test_expect_success PERL 'cvsimport.module config works' '
(cd module-git &&
git config cvsimport.module module &&
git config cvsimport.trackRevisions true &&
- git cvsimport -a -z0 &&
- git merge origin
+ git cvsimport -z0
) &&
test_cmp module-cvs/tick module-git/tick
@@ -140,7 +138,7 @@ test_expect_success PERL 'import from a CVS working tree' '
$CVS co -d import-from-wt module &&
(cd import-from-wt &&
git config cvsimport.trackRevisions false &&
- git cvsimport -a -z0 &&
+ git cvsimport -z0 &&
echo 1 >expect &&
git log -1 --pretty=format:%s%n >actual &&
test_cmp actual expect
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* [PATCH v2 3/3] t9604: fixup for new cvsimport
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
In-Reply-To: <1357971703-28513-1-git-send-email-chris@rorvick.com>
cvsps no longer writes a cache file and therefore no longer can be told
to ignore it with -x.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
t/t9604-cvsimport-timestamps.sh | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/t/t9604-cvsimport-timestamps.sh b/t/t9604-cvsimport-timestamps.sh
index 1fd5142..b1629b6 100755
--- a/t/t9604-cvsimport-timestamps.sh
+++ b/t/t9604-cvsimport-timestamps.sh
@@ -7,8 +7,7 @@ setup_cvs_test_repository t9604
test_expect_success 'check timestamps are UTC (TZ=CST6CDT)' '
- TZ=CST6CDT git cvsimport -p"-x" -C module-1 module &&
- git cvsimport -p"-x" -C module-1 module &&
+ TZ=CST6CDT git cvsimport -C module-1 module &&
(
cd module-1 &&
git log --format="%s %ai"
@@ -42,7 +41,7 @@ test_expect_success 'check timestamps with author-specific timezones' '
user3=User Three <user3@domain.org> EST5EDT
user4=User Four <user4@domain.org> MST7MDT
EOF
- git cvsimport -p"-x" -A cvs-authors -C module-2 module &&
+ git cvsimport -A cvs-authors -C module-2 module &&
(
cd module-2 &&
git log --format="%s %ai %an"
--
1.8.1.rc3.335.g88a67d6
^ 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