* Re: [PATCH 4/4] push: add 'prune' option
From: Felipe Contreras @ 2012-02-23 1:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7v1upm8jnq.fsf@alter.siamese.dyndns.org>
On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> +--prune::
>> + Remove remote branches that don't have a local counterpart. For example
>> + a remote branch `tmp` will be removed if a local branch with the same
>> + name doesn't exist any more. This also respects refspecs, e.g.
>> + `refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`
>> + will be removed if `refs/heads/foo` doesn't exist.
>
> I do not think it adds much useful information to mention `tmp` only once
> over what is already said by the first sentence. Also, the first sentence
> of the example does not make it clear that it is assuming a same-for-same
> mapping.
Sure, the first sentence doesn't make it clear, but it would be a
valid and obvious assumption. The second sentence makes it clear, and
the name `tmp` immediately evokes a branch that will probably be
removed.
> Coming up with a precise technical description is easy, but it is hard to
> explain to the end user in easy terms, and I commend you for attempting to
> add an example in a short single sentence, though.
>
> Perhaps spelling out the underlying assumption the example makes is the
> best we could do here without going too technical.
>
> ... For example, if you are pushing all your local branches to
> update the local branches of the remote,
Yeah, but that's 'git push --all', and that's not the common
operation--'git push' is. So that's what I presumed the reader would
assume, and it really doesn't make a difference as to what will
follow:
> `tmp` branch will be
> removed from the remote if you removed your `tmp` branch locally.
This reuses the name `tmp`, which seems to be your objective, but it
doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
the upstream branch, or is it because it has the same name?
> If you are pushing all your local branches on your laptop to a
> repository on your desktop machine under `refs/remotes/laptop/`
> hierarchy to back them up, `refs/remotes/laptop/tmp` is removed
> from the remote if you no longer have the branch `tmp` on your
> laptop.
Unfortunately, I as a reader have trouble understanding this. More
specifically I have trouble understanding where `refs/remotes/laptop/`
is coming from, and what it is meaning. I have always pictured
`refs/remotes` as something that 'git fetch' updates, and always from
the relevant repository. While eventually I could understand what this
thing is doing, it took me more than one read, and I had to read
slowly, and even then it seems completely non-standard to me.
I think a synthetic example, like `refs/heads/*:refs/tmp/*`, is much
easier to understand because it doesn't mess up with any established
refs, and also has the advantage that it shows the relevant refspec,
which is useful for people that are not familiar with refspecs, and in
fact, people could try it out without messing with their current refs.
>> diff --git a/remote.h b/remote.h
>> index b395598..341142c 100644
>> --- a/remote.h
>> +++ b/remote.h
>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>> enum match_refs_flags {
>> MATCH_REFS_NONE = 0,
>> MATCH_REFS_ALL = (1 << 0),
>> - MATCH_REFS_MIRROR = (1 << 1)
>> + MATCH_REFS_MIRROR = (1 << 1),
>> + MATCH_REFS_PRUNE = (1 << 2),
>> };
>
> Lose the ',' at the end, for the same reason why deleted line did not have
> one.
And why is that? Isn't git c99? That comma would only ensure that the
next patch that touches this would be two lines instead of one.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: how do I review gitk release notes
From: Junio C Hamano @ 2012-02-23 1:13 UTC (permalink / raw)
To: Neal Kreitzinger; +Cc: git
In-Reply-To: <ji42ab$71s$1@dough.gmane.org>
"Neal Kreitzinger" <neal@rsss.com> writes:
> What is the best way to get the corresponding gitk release notes for git
> releases (so I can see the detailed gitk release notes behind the git
> release note "various gitk enhancements and fixes")?
Unfortunately, the best way I can think of is that you write it yourself
by looking at the log; I do not think there is any selarate release notes
written by anybody.
$ git log -1 --oneline --first-parent master -- gitk-git
will show that 09bb4eb (Merge git://ozlabs.org/~paulus/gitk, 2011-12-16)
was the last merge, so
$ git log -p 09bb4eb^2
$ git shortlog 09bb4eb^2
would be the "raw" data source to study.
^ permalink raw reply
* Re: [PATCH 4/4] push: add 'prune' option
From: Junio C Hamano @ 2012-02-23 1:31 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <CAMP44s1s_NeXeUpxDR0cLoLGjSaf-0E_62MzAxeiHS=-6c045A@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Yeah, but that's 'git push --all', and that's not the common
> operation--'git push' is.
"git push" is common but that does not give you a solid base to guess what
the reader's assumption would be. Are you assuming "matching" semantics?
> So that's what I presumed the reader would
> assume,...
I do not want let us guess what the reader assumes, as many people seem to
suggest setting push.default to different values and that would change
what the reader would assume. That was the whole reason that I suggested
to spell the assumption out, so that the reader's assumption does not have
to get into the picture.
> This reuses the name `tmp`, which seems to be your objective, but it
> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
> the upstream branch, or is it because it has the same name?
The example is to clarify "local counterpart" in the main text. I
actually would prefer to get rid of `tmp` but I left it as-is as you
wrote. The exact name used in the example does not matter, whether it is
`tmp` or `xyzzy`.
> Unfortunately, I as a reader have trouble understanding this. More
> specifically I have trouble understanding where `refs/remotes/laptop/`
> is coming from, and what it is meaning. I have always pictured
> `refs/remotes` as something that 'git fetch' updates, and always from
> the relevant repository.
The layout is the recommended set-up to emulate a fetch with a push in the
reverse direction, which I thought anybody should notice. It is a failure
in our documentation that even an expert didn't.
>>> diff --git a/remote.h b/remote.h
>>> index b395598..341142c 100644
>>> --- a/remote.h
>>> +++ b/remote.h
>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>>> enum match_refs_flags {
>>> MATCH_REFS_NONE = 0,
>>> MATCH_REFS_ALL = (1 << 0),
>>> - MATCH_REFS_MIRROR = (1 << 1)
>>> + MATCH_REFS_MIRROR = (1 << 1),
>>> + MATCH_REFS_PRUNE = (1 << 2),
>>> };
>>
>> Lose the ',' at the end, for the same reason why deleted line did not have
>> one.
>
> And why is that?
Because I told you so ;-).
More seriously, we have had patches to accomodate other people's compilers
by dropping the last comma in enum {}. See c9b6782 (enums: omit trailing
comma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma
for portability, 2010-05-14) for examples.
^ permalink raw reply
* Re: [PATCH 4/4] push: add 'prune' option
From: Felipe Contreras @ 2012-02-23 2:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vlinu72ui.fsf@alter.siamese.dyndns.org>
On Thu, Feb 23, 2012 at 3:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Yeah, but that's 'git push --all', and that's not the common
>> operation--'git push' is.
>
> "git push" is common but that does not give you a solid base to guess what
> the reader's assumption would be. Are you assuming "matching" semantics?
But if you want to spell the assumption, why not spell the most common one?
>> So that's what I presumed the reader would
>> assume,...
>
> I do not want let us guess what the reader assumes, as many people seem to
> suggest setting push.default to different values and that would change
> what the reader would assume.
No, it wouldn't. Almost all readers are familiar about what 'git push'
does by default--I would even adventure to say all--the ones that set
a custom push.default would do it because they _know_ they don't want
the default.
And in fact, as I said, it doesn't matter what the reader has set in
push.default, or if the reader is assuming 'git push --all', or 'git
push :', or 'git push --tags', or 'git push refs/heads/*', the end
result is the same.
> That was the whole reason that I suggested
> to spell the assumption out, so that the reader's assumption does not have
> to get into the picture.
I don't think it matters what the reader assumes, because the end
result is the same, so there's no reason to spell any assumption.
>> This reuses the name `tmp`, which seems to be your objective, but it
>> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
>> the upstream branch, or is it because it has the same name?
>
> The example is to clarify "local counterpart" in the main text. I
> actually would prefer to get rid of `tmp` but I left it as-is as you
> wrote. The exact name used in the example does not matter, whether it is
> `tmp` or `xyzzy`.
I believe `tmp` is more useful than `xyzzy`, or even more than
`master`. I already explained why.
>> Unfortunately, I as a reader have trouble understanding this. More
>> specifically I have trouble understanding where `refs/remotes/laptop/`
>> is coming from, and what it is meaning. I have always pictured
>> `refs/remotes` as something that 'git fetch' updates, and always from
>> the relevant repository.
>
> The layout is the recommended set-up to emulate a fetch with a push in the
> reverse direction, which I thought anybody should notice. It is a failure
> in our documentation that even an expert didn't.
The problem is not figuring out the `refs/remotes/foo`, but picturing
the fact that there are two repositories, the user ran on one of them
'git remote add laptop', and then 'git remote add pc', and then 'git
push --prune refs/heads*:refs/remotes/laptop/*'. Lots of things to
keep in mind for this paragraph. And, as you say, it's the reverse
direction of a fetch, so it's immediately weird.
Moreover, I find it strange that you want to rely on the assumption
that the reader is familiar with the pattern `refs/remotes/foo`, and
the refspec syntax, which is pretty deep plumbing, and not on the
default action of 'git push', which is high-level porcelain, and as I
said, not even needed to understand my example.
>>>> diff --git a/remote.h b/remote.h
>>>> index b395598..341142c 100644
>>>> --- a/remote.h
>>>> +++ b/remote.h
>>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>>>> enum match_refs_flags {
>>>> MATCH_REFS_NONE = 0,
>>>> MATCH_REFS_ALL = (1 << 0),
>>>> - MATCH_REFS_MIRROR = (1 << 1)
>>>> + MATCH_REFS_MIRROR = (1 << 1),
>>>> + MATCH_REFS_PRUNE = (1 << 2),
>>>> };
>>>
>>> Lose the ',' at the end, for the same reason why deleted line did not have
>>> one.
>>
>> And why is that?
>
> Because I told you so ;-).
You didn't tell me anything about the previous line :)
> More seriously, we have had patches to accomodate other people's compilers
> by dropping the last comma in enum {}. See c9b6782 (enums: omit trailing
> comma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma
> for portability, 2010-05-14) for examples.
Yeah, I remembered those patches after sending the mail. Shame. Maybe
next decade =/
Cheers.
--
Felipe Contreras
^ permalink raw reply
* What's cooking in git.git (Feb 2012, #08; Wed, 22)
From: Junio C Hamano @ 2012-02-23 2:34 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'.
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
But I seem to be getting this from the github:
ERROR: Permission to git/git.git denied to gitster.
fatal: The remote end hung up unexpectedly
so https://github.com/git/git/ is not updated. All others are OK.
--------------------------------------------------
[New Topics]
* jb/filter-ignore-sigpipe (2012-02-21) 1 commit
- Ignore SIGPIPE when running a filter driver
Looked sane.
Will merge to "next".
* jc/pickaxe-ignore-case (2012-02-21) 1 commit
- pickaxe: allow -i to search in patch case-insensitively
* jc/doc-merge-options (2012-02-22) 1 commit
- Documentation/merge-options.txt: group "ff" related options together
Documentation for "git merge" had "--ff-only" far away from other options
related to the handling of fast-forward merges.
* ph/cherry-pick-advice-refinement (2012-02-22) 1 commit
- cherry-pick: No advice to commit if --no-commit
* pj/completion-remote-set-url-branches (2012-02-22) 2 commits
- completion: normalize increment/decrement style
- completion: remote set-* <name> and <branch>
* th/git-diffall (2012-02-22) 1 commit
- contrib: added git-diffall
--------------------------------------------------
[Graduated to "master"]
* jk/diff-highlight (2012-02-13) 5 commits
(merged to 'next' on 2012-02-20 at ba040ae)
+ diff-highlight: document some non-optimal cases
+ diff-highlight: match multi-line hunks
+ diff-highlight: refactor to prepare for multi-line hunks
+ diff-highlight: don't highlight whole lines
+ diff-highlight: make perl strict and warnings fatal
Updates diff-highlight (in contrib/).
* jn/gitweb-unborn-head (2012-02-17) 1 commit
(merged to 'next' on 2012-02-20 at 80e3ff2)
+ gitweb: Fix "heads" view when there is no current branch
"gitweb" compared non-existent value of HEAD with the names of commit
objects at tips of branches, triggering runtime warnings.
--------------------------------------------------
[Stalled]
* jl/maint-submodule-relative (2012-02-09) 2 commits
- submodules: always use a relative path from gitdir to work tree
- submodules: always use a relative path to gitdir
The second one looked iffy.
* hv/submodule-recurse-push (2012-02-13) 3 commits
- push: teach --recurse-submodules the on-demand option
- Refactor submodule push check to use string list instead of integer
- Teach revision walking machinery to walk multiple times sequencially
The bottom one was not clearly explained and needs a reroll.
* zj/diff-stat-dyncol (2012-02-15) 6 commits
. diff --stat: use less columns for change counts
- (squash to the previous -- replace the last line of the log with the following)
- diff --stat: use the full terminal width
- (squash to the previous -- replace the log message with this)
- diff --stat: tests for long filenames and big change counts
- Merge branches zj/decimal-width and zj/term-columns
I am beginning to think that the last one should wait until the dust from
the earlier part settles.
* jc/advise-push-default (2011-12-18) 1 commit
- push: hint to use push.default=upstream when appropriate
Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?
* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
- fixup! 15eaaf4
- git-svn, perl/Git.pm: extend Git::prompt helper for querying users
- perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS
The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.
* jc/split-blob (2012-01-24) 6 commits
- chunked-object: streaming checkout
- chunked-object: fallback checkout codepaths
- bulk-checkin: support chunked-object encoding
- bulk-checkin: allow the same data to be multiply hashed
- new representation types in the packstream
- varint-in-pack: refactor varint encoding/decoding
Not ready.
I finished the streaming checkout codepath, but as explained in 127b177
(bulk-checkin: support chunked-object encoding, 2011-11-30), these are
still early steps of a long and painful journey. At least pack-objects and
fsck need to learn the new encoding for the series to be usable locally,
and then index-pack/unpack-objects needs to learn it to be used remotely.
Given that I heard a lot of noise that people want large files, and that I
was asked by somebody at GitTogether'11 privately for an advice on how to
pay developers (not me) to help adding necessary support, I am somewhat
dissapointed that the original patch series that was sent almost two
months ago still remains here without much comments and updates from the
developer community. I even made the interface to the logic that decides
where to split chunks easily replaceable, and I deliberately made the
logic in the original patch extremely stupid to entice others, especially
the "bup" fanboys, to come up with a better logic, thinking that giving
people an easy target to shoot for, they may be encouraged to help
out. The plan is not working :-(.
* nd/columns (2012-02-08) 15 commits
. column: Fix some compiler and sparse warnings
. column: add a corner-case test to t3200
. columns: minimum coding style fixes
. tag: add --column
. column: support piping stdout to external git-column process
. status: add --column
. branch: add --column
. help: reuse print_columns() for help -a
. column: add column.ui for default column output settings
. column: support columns with different widths
. column: add columnar layout
. Stop starting pager recursively
. Add git-column and column mode parsing
. column: add API to print items in columns
. Save terminal width before setting up pager
Expecting a reroll on top of zj/term-columns topic.
--------------------------------------------------
[Cooking]
* fc/push-prune (2012-02-22) 4 commits
- push: add '--prune' option
- remote: refactor code into alloc_delete_ref()
- remote: reorganize check_pattern_match()
- remote: use a local variable in match_push_refs()
Rerolled and looked sane.
Will merge to "next".
* jc/add-refresh-unmerged (2012-02-17) 1 commit
(merged to 'next' on 2012-02-21 at 09f8721)
+ refresh_index: do not show unmerged path that is outside pathspec
"git add --refresh <pathspec>" warned about unmerged paths outside the
given pathspec.
* jc/diff-ignore-case (2012-02-19) 6 commits
- diff -i
- diff: --ignore-case
- xdiff: introduce XDF_IGNORE_CASE
- xdiff: introduce XDF_INEXACT_MATCH
- xdiff: PATIENCE/HISTOGRAM are not independent option bits
- xdiff: remove XDL_PATCH_* macros
"git diff" learns "--ignore-case" option.
* jn/gitweb-hilite-regions (2012-02-19) 5 commits
- gitweb: Use esc_html_match_hl() in 'grep' search
- gitweb: Highlight matched part of shortened project description
- gitweb: Highlight matched part of project description when searching projects
- gitweb: Highlight matched part of project name when searching projects
- gitweb: Introduce esc_html_hl_regions
(this branch uses jn/gitweb-search-optim.)
Not reviewed and do not know what this is about yet ;-).
* jn/gitweb-search-optim (2012-02-19) 3 commits
- gitweb: Faster project search
- gitweb: Option for filling only specified info in fill_project_list_info
- gitweb: Refactor checking if part of project info need filling
(this branch is used by jn/gitweb-hilite-regions.)
The API introduced in the second step still has yucky design, but at least
it is more clear than the previous rounds what this is trying to do.
* js/configure-libintl (2012-02-20) 1 commit
(merged to 'next' on 2012-02-21 at 79d7ccc)
+ configure: don't use -lintl when there is no gettext support
Build fix for autoconf, meant for 'maint' track.
* pj/remote-set-branches-usage-fix (2012-02-19) 1 commit
(merged to 'next' on 2012-02-21 at cb71d0e)
+ remote: fix set-branches usage and documentation
Documentation fix.
* tr/perftest (2012-02-17) 3 commits
(merged to 'next' on 2012-02-20 at 4c75ba9)
+ Add a performance test for git-grep
+ Introduce a performance testing framework
+ Move the user-facing test library to test-lib-functions.sh
* jb/required-filter (2012-02-17) 1 commit
- Add a setting to require a filter to be successful
A content filter used to be a way to make the recorded contents "more
useful", but this defines a way to optionally mark a filter "required".
Will merge to "next" after waiting for a few more days for comments.
* jk/config-include (2012-02-17) 10 commits
(merged to 'next' on 2012-02-20 at 7b150b7)
+ config: add include directive
+ config: eliminate config_exclusive_filename
+ config: stop using config_exclusive_filename
+ config: provide a version of git_config with more options
+ config: teach git_config_rename_section a file argument
+ config: teach git_config_set_multivar_in_file a default path
+ config: copy the return value of prefix_filename
+ t1300: add missing &&-chaining
+ docs/api-config: minor clarifications
+ docs: add a basic description of the config API
An assignment to the include.path pseudo-variable causes the named file
to be included in-place when Git looks up configuration variables.
Reverted the earlier round from 'next' and then fixed up further.
* ld/git-p4-expanded-keywords (2012-02-14) 1 commit
(merged to 'next' on 2012-02-16 at a9004c5)
+ git-p4: add initial support for RCS keywords
Teach git-p4 to unexpand $RCS$-like keywords that are embedded in
tracked contents in order to reduce unnecessary merge conflicts.
Waiting for follow-up fix-up patches.
^ permalink raw reply
* [ANNOUNCE] Git 1.7.9.2
From: Junio C Hamano @ 2012-02-23 2:38 UTC (permalink / raw)
To: git
The latest maintenance release Git 1.7.9.2 is now available at the
usual places.
The release tarballs are found at:
http://code.google.com/p/git-core/downloads/list
and their SHA-1 checksums are:
7aff1048480a8637de94e8d82744d312c0b5e060 git-1.7.9.2.tar.gz
3cf13b03b2f64d0458212232cc18983231f8251e git-htmldocs-1.7.9.2.tar.gz
d6992d899fb70e40983f94a2f96ad24b8ee93557 git-manpages-1.7.9.2.tar.gz
Also the following public repositories all have a copy of the v1.7.9.2
tag and the maint branch that the tag points at:
url = git://repo.or.cz/alt-git.git
url = https://code.google.com/p/git-core/
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
Git v1.7.9.2 Release Notes
==========================
Fixes since v1.7.9.1
--------------------
* Bash completion script (in contrib/) did not like a pattern that
begins with a dash to be passed to __git_ps1 helper function.
* Adaptation of the bash completion script (in contrib/) for zsh
incorrectly listed all subcommands when "git <TAB><TAB>" was given
to ask for list of porcelain subcommands.
* The build procedure for profile-directed optimized binary was not
working very well.
* Some systems need to explicitly link -lcharset to get locale_charset().
* t5541 ignored user-supplied port number used for HTTP server testing.
* The error message emitted when we see an empty loose object was
not phrased correctly.
* The code to ask for password did not fall back to the terminal
input when GIT_ASKPASS is set but does not work (e.g. lack of X
with GUI askpass helper).
* We failed to give the true terminal width to any subcommand when
they are invoked with the pager, i.e. "git -p cmd".
* map_user() was not rewriting its output correctly, which resulted
in the user visible symptom that "git blame -e" sometimes showed
excess '>' at the end of email addresses.
* "git checkout -b" did not allow switching out of an unborn branch.
* When you have both .../foo and .../foo.git, "git clone .../foo" did not
favor the former but the latter.
* "git commit" refused to create a commit when entries added with
"add -N" remained in the index, without telling Git what their content
in the next commit should be. We should have created the commit without
these paths.
* "git diff --stat" said "files", "insertions", and "deletions" even
when it is showing one "file", one "insertion" or one "deletion".
* The output from "git diff --stat" for two paths that have the same
amount of changes showed graph bars of different length due to the
way we handled rounding errors.
* "git grep" did not pay attention to -diff (hence -binary) attribute.
* The transport programs (fetch, push, clone)ignored --no-progress
and showed progress when sending their output to a terminal.
* Sometimes error status detected by a check in an earlier phase of
"git receive-pack" (the other end of "git push") was lost by later
checks, resulting in false indication of success.
* "git rev-list --verify" sometimes skipped verification depending on
the phase of the moon, which dates back to 1.7.8.x series.
* Search box in "gitweb" did not accept non-ASCII characters correctly.
* Search interface of "gitweb" did not show multiple matches in the same file
correctly.
Also contains minor fixes and documentation updates.
----------------------------------------------------------------
Changes since v1.7.9.1 are as follows:
Christian Hammerl (1):
completion: Allow dash as the first character for __git_ps1
Clemens Buchacher (4):
git rev-list: fix invalid typecast
push/fetch/clone --no-progress suppresses progress output
t5541: check error message against the real port number used
do not override receive-pack errors
Felipe Contreras (3):
completion: work around zsh option propagation bug
completion: use ls -1 instead of rolling a loop to do that ourselves
completion: simplify __gitcomp and __gitcomp_nl implementations
Jakub Narebski (2):
gitweb: Allow UTF-8 encoded CGI query parameters and path_info
gitweb: Fix 'grep' search for multiple matches in file
Jeff King (12):
grep: make locking flag global
grep: move sha1-reading mutex into low-level code
grep: refactor the concept of "grep source" into an object
convert git-grep to use grep_source interface
grep: drop grep_buffer's "name" parameter
grep: cache userdiff_driver in grep_source
grep: respect diff attributes for binary-ness
grep: load file data after checking binary-ness
grep: pre-load userdiff drivers when threaded
standardize and improve lookup rules for external local repos
prompt: clean up strbuf usage
prompt: fall back to terminal if askpass fails
Jiang Xin (2):
i18n: git-commit whence_s "merge/cherry-pick" message
i18n: format_tracking_info "Your branch is behind" message
Johannes Sixt (1):
Makefile: fix syntax for older make
Junio C Hamano (8):
mailmap: always return a plain mail address from map_user()
git checkout -b: allow switching out of an unborn branch
commit: ignore intent-to-add entries instead of refusing
diff --stat: show bars of same length for paths with same amount of changes
Update draft release notes to 1.7.9.2
Update draft release notes to 1.7.9.2
Update draft release notes to 1.7.9.2
Git 1.7.9.2
Matthieu Moy (1):
fsck: give accurate error message on empty loose object files
Namhyung Kim (2):
ctype.c only wants git-compat-util.h
ctype: implement islower/isupper macro
Nguyễn Thái Ngọc Duy (3):
sha1_file.c: move the core logic of find_pack_entry() into fill_pack_entry()
find_pack_entry(): do not keep packed_git pointer locally
Use correct grammar in diffstat summary line
Philip Jägenstedt (2):
completion: remove stale "to submit patches" documentation
completion: use tabs for indentation
Ralf Thielow (2):
completion: --edit-description option for git-branch
completion: --list option for git-branch
Theodore Ts'o (1):
Fix build problems related to profile-directed optimization
Zbigniew Jędrzejewski-Szmek (2):
pager: find out the terminal width before spawning the pager
man: rearrange git synopsis to fit in 80 lines
Дилян Палаузов (1):
Makefile: introduce CHARSET_LIB to link with -lcharset
^ permalink raw reply
* Re: [PATCH 2/2] bundle: use a strbuf to scan the log for boundary commits
From: Junio C Hamano @ 2012-02-23 3:20 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Jannis Pohlmann
In-Reply-To: <fa1553d59714fd89fdab1bf54af19ac631a30a8c.1329939233.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> +# If "ridiculous" is at least 1004 chars, this traps a bug in old
> +# versions where the resulting 1025-char line (with --pretty=oneline)
> +# was longer than a 1024-char buffer
> +test_expect_success 'ridiculously long subject in boundary' '
> +
> + : > file4 &&
> + test_tick &&
> + git add file4 &&
> + printf "abcdefghijkl %s\n" $(seq 1 100) | git commit -F - &&
> + test_commit fifth &&
> + git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
> + git fetch long-subject-bundle.bdl &&
> + sed -n "/^-/{p;q}" long-subject-bundle.bdl > boundary &&
> + grep "^-$_x40 " boundary
> +
> +'
Hmm, aside from the use of seq J6t mentioned, I am not sure what this is
testing.
I thought the point is to make "--oneline" output produce a long line
whose 1024th character is '-' and make the fgets() based parser to mistake
it as the beginning of a line that records a boundary commit, but you do
not seem to have any '-' there.
^ permalink raw reply
* Re: [PATCH 2/2] bundle: use a strbuf to scan the log for boundary commits
From: Junio C Hamano @ 2012-02-23 3:38 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Jannis Pohlmann
In-Reply-To: <fa1553d59714fd89fdab1bf54af19ac631a30a8c.1329939233.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> diff --git a/bundle.c b/bundle.c
> index 313de42..0dbd174 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path,
> const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
> const char **argv_pack = xmalloc(6 * sizeof(const char *));
> int i, ref_count = 0;
> - char buffer[1024];
> + struct strbuf buf = STRBUF_INIT;
> struct rev_info revs;
> struct child_process rls;
> FILE *rls_fout;
> @@ -266,16 +266,16 @@ int create_bundle(struct bundle_header *header, const char *path,
> if (start_command(&rls))
> return -1;
> rls_fout = xfdopen(rls.out, "r");
> - while (fgets(buffer, sizeof(buffer), rls_fout)) {
> + while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
I'd add strbuf_release(&buf) after this loop.
Perhaps we would want to squash something like this to the test to avoid
"seq", using J6t's idea. The issue is that we do not write the end of
line for the long boundary (because it is hidden from us), keep reading
and rejecting bogus letters, and then the tip is written without
terminating the boundary line. So checking boundary line is not a very
useful test, but "fetch" and "list-heads" are.
t/t5704-bundle.sh | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 7c2f307..5319b84 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -59,19 +59,20 @@ test_expect_success 'empty bundle file is rejected' '
'
-# If "ridiculous" is at least 1004 chars, this traps a bug in old
-# versions where the resulting 1025-char line (with --pretty=oneline)
-# was longer than a 1024-char buffer
+# This triggers a bug in older versions where the resulting line (with
+# --pretty=oneline) was longer than a 1024-char buffer.
test_expect_success 'ridiculously long subject in boundary' '
- : > file4 &&
+ : >file4 &&
test_tick &&
git add file4 &&
- printf "abcdefghijkl %s\n" $(seq 1 100) | git commit -F - &&
+ printf "%0982d\n" 0 | git commit -F - &&
test_commit fifth &&
git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
+ git bundle list-heads long-subject-bundle.bdl >heads &&
+ test -s heads &&
git fetch long-subject-bundle.bdl &&
- sed -n "/^-/{p;q}" long-subject-bundle.bdl > boundary &&
+ sed -n "/^-/{p;q}" long-subject-bundle.bdl >boundary &&
grep "^-$_x40 " boundary
'
--
1.7.9.2.258.g4407a
^ permalink raw reply related
* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Junio C Hamano @ 2012-02-23 5:08 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds, j.sixt
In-Reply-To: <1329775034-21551-1-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> this is v6, with a new approach.
When you reroll, please make sure you do not run format-patch with
nonstandard settings, e.g.
diff --git t/t4052-stat-output.sh t/t4052-stat-output.sh
^ permalink raw reply
* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Miles Bader @ 2012-02-23 7:29 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek
Cc: Nguyen Thai Ngoc Duy, git, gitster, Michael J Gruber, j.sixt
In-Reply-To: <4F43C23C.8070304@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> I use (or would like to be able to use) the --stat output to select with
> the mouse and paste into something like
> emacsclient -n <pathname>
> This would be harder with the grouping, because I'd have to select two
> parts and paste two times and type a slash. So for me this would be a
> minus.
Of course, git's diffstats often already violate that property (e.g.,
abbreviations and the renaming syntax)... In cases where such a
grouping syntax avoids information-losing abbreviations, it might even
be an improvement over the current situation (even if the info was
split up, at least it would be there).
If the heuristics were tuned so that the syntax only kicked in for
cases where deep nesting or long names would otherwise make
abbreviations likely, maybe it would be a general improvement.
However, it does seem like the additional inconsistency of
presentation might make diffstats harder to read or more ugly in some
cases...
-miles
--
Dawn, n. When men of reason go to bed.
^ permalink raw reply
* [PATCHv5] git-p4: RCS keyword handling
From: Luke Diamand @ 2012-02-23 7:51 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Eric Scouten, Junio C Hamano, Luke Diamand
Updated RCS keyword fix for git-p4, incorporating fixes to test cases
missed earlier.
Luke Diamand (1):
git-p4: add initial support for RCS keywords
Documentation/git-p4.txt | 5 +
contrib/fast-import/git-p4 | 118 ++++++++++++--
t/t9810-git-p4-rcs.sh | 388 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 501 insertions(+), 10 deletions(-)
create mode 100755 t/t9810-git-p4-rcs.sh
--
1.7.9.259.ga92e
^ permalink raw reply
* [PATCHv5] git-p4: add initial support for RCS keywords
From: Luke Diamand @ 2012-02-23 7:51 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Eric Scouten, Junio C Hamano, Luke Diamand
In-Reply-To: <1329983490-4033-1-git-send-email-luke@diamand.org>
RCS keywords cause problems for git-p4 as perforce always
expands them (if +k is set) and so when applying the patch,
git reports that the files have been modified by both sides,
when in fact they haven't.
This change means that when git-p4 detects a problem applying
a patch, it will check to see if keyword expansion could be
the culprit. If it is, it strips the keywords in the p4
repository so that they match what git is expecting. It then
has another go at applying the patch.
This behaviour is enabled with a new git-p4 configuration
option and is off by default.
Acked-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Luke Diamand <luke@diamand.org>
---
Documentation/git-p4.txt | 5 +
contrib/fast-import/git-p4 | 118 ++++++++++++--
t/t9810-git-p4-rcs.sh | 388 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 501 insertions(+), 10 deletions(-)
create mode 100755 t/t9810-git-p4-rcs.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 8b92cc0..3fecefa 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -483,6 +483,11 @@ git-p4.skipUserNameCheck::
user map, 'git p4' exits. This option can be used to force
submission regardless.
+git-p4.attemptRCSCleanup:
+ If enabled, 'git p4 submit' will attempt to cleanup RCS keywords
+ ($Header$, etc). These would otherwise cause merge conflicts and prevent
+ the submit going ahead. This option should be considered experimental at
+ present.
IMPLEMENTATION DETAILS
----------------------
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index a78d9c5..c8b6c8a 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -10,7 +10,7 @@
import optparse, sys, os, marshal, subprocess, shelve
import tempfile, getopt, os.path, time, platform
-import re
+import re, shutil
verbose = False
@@ -186,6 +186,47 @@ def split_p4_type(p4type):
mods = s[1]
return (base, mods)
+#
+# return the raw p4 type of a file (text, text+ko, etc)
+#
+def p4_type(file):
+ results = p4CmdList(["fstat", "-T", "headType", file])
+ return results[0]['headType']
+
+#
+# Given a type base and modifier, return a regexp matching
+# the keywords that can be expanded in the file
+#
+def p4_keywords_regexp_for_type(base, type_mods):
+ if base in ("text", "unicode", "binary"):
+ kwords = None
+ if "ko" in type_mods:
+ kwords = 'Id|Header'
+ elif "k" in type_mods:
+ kwords = 'Id|Header|Author|Date|DateTime|Change|File|Revision'
+ else:
+ return None
+ pattern = r"""
+ \$ # Starts with a dollar, followed by...
+ (%s) # one of the keywords, followed by...
+ (:[^$]+)? # possibly an old expansion, followed by...
+ \$ # another dollar
+ """ % kwords
+ return pattern
+ else:
+ return None
+
+#
+# Given a file, return a regexp matching the possible
+# RCS keywords that will be expanded, or None for files
+# with kw expansion turned off.
+#
+def p4_keywords_regexp_for_file(file):
+ if not os.path.exists(file):
+ return None
+ else:
+ (type_base, type_mods) = split_p4_type(p4_type(file))
+ return p4_keywords_regexp_for_type(type_base, type_mods)
def setP4ExecBit(file, mode):
# Reopens an already open file and changes the execute bit to match
@@ -753,6 +794,29 @@ class P4Submit(Command, P4UserMap):
return result
+ def patchRCSKeywords(self, file, pattern):
+ # Attempt to zap the RCS keywords in a p4 controlled file matching the given pattern
+ (handle, outFileName) = tempfile.mkstemp(dir='.')
+ try:
+ outFile = os.fdopen(handle, "w+")
+ inFile = open(file, "r")
+ regexp = re.compile(pattern, re.VERBOSE)
+ for line in inFile.readlines():
+ line = regexp.sub(r'$\1$', line)
+ outFile.write(line)
+ inFile.close()
+ outFile.close()
+ # Forcibly overwrite the original file
+ os.unlink(file)
+ shutil.move(outFileName, file)
+ except:
+ # cleanup our temporary file
+ os.unlink(outFileName)
+ print "Failed to strip RCS keywords in %s" % file
+ raise
+
+ print "Patched up RCS keywords in %s" % file
+
def p4UserForCommit(self,id):
# Return the tuple (perforce user,git email) for a given git commit id
self.getUserMapFromPerforceServer()
@@ -918,6 +982,7 @@ class P4Submit(Command, P4UserMap):
filesToDelete = set()
editedFiles = set()
filesToChangeExecBit = {}
+
for line in diff:
diff = parseDiffTreeEntry(line)
modifier = diff['status']
@@ -964,9 +1029,45 @@ class P4Submit(Command, P4UserMap):
patchcmd = diffcmd + " | git apply "
tryPatchCmd = patchcmd + "--check -"
applyPatchCmd = patchcmd + "--check --apply -"
+ patch_succeeded = True
if os.system(tryPatchCmd) != 0:
+ fixed_rcs_keywords = False
+ patch_succeeded = False
print "Unfortunately applying the change failed!"
+
+ # Patch failed, maybe it's just RCS keyword woes. Look through
+ # the patch to see if that's possible.
+ if gitConfig("git-p4.attemptRCSCleanup","--bool") == "true":
+ file = None
+ pattern = None
+ kwfiles = {}
+ for file in editedFiles | filesToDelete:
+ # did this file's delta contain RCS keywords?
+ pattern = p4_keywords_regexp_for_file(file)
+
+ if pattern:
+ # this file is a possibility...look for RCS keywords.
+ regexp = re.compile(pattern, re.VERBOSE)
+ for line in read_pipe_lines(["git", "diff", "%s^..%s" % (id, id), file]):
+ if regexp.search(line):
+ if verbose:
+ print "got keyword match on %s in %s in %s" % (pattern, line, file)
+ kwfiles[file] = pattern
+ break
+
+ for file in kwfiles:
+ if verbose:
+ print "zapping %s with %s" % (line,pattern)
+ self.patchRCSKeywords(file, kwfiles[file])
+ fixed_rcs_keywords = True
+
+ if fixed_rcs_keywords:
+ print "Retrying the patch with RCS keywords cleaned up"
+ if os.system(tryPatchCmd) == 0:
+ patch_succeeded = True
+
+ if not patch_succeeded:
print "What do you want to do?"
response = "x"
while response != "s" and response != "a" and response != "w":
@@ -1585,15 +1686,12 @@ class P4Sync(Command, P4UserMap):
# Note that we do not try to de-mangle keywords on utf16 files,
# even though in theory somebody may want that.
- if type_base in ("text", "unicode", "binary"):
- if "ko" in type_mods:
- text = ''.join(contents)
- text = re.sub(r'\$(Id|Header):[^$]*\$', r'$\1$', text)
- contents = [ text ]
- elif "k" in type_mods:
- text = ''.join(contents)
- text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', text)
- contents = [ text ]
+ pattern = p4_keywords_regexp_for_type(type_base, type_mods)
+ if pattern:
+ regexp = re.compile(pattern, re.VERBOSE)
+ text = ''.join(contents)
+ text = regexp.sub(r'$\1$', text)
+ contents = [ text ]
self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
new file mode 100755
index 0000000..49dfde0
--- /dev/null
+++ b/t/t9810-git-p4-rcs.sh
@@ -0,0 +1,388 @@
+#!/bin/sh
+
+test_description='git-p4 rcs keywords'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+#
+# Make one file with keyword lines at the top, and
+# enough plain text to be able to test modifications
+# far away from the keywords.
+#
+test_expect_success 'init depot' '
+ (
+ cd "$cli" &&
+ cat <<-\EOF >filek &&
+ $Id$
+ /* $Revision$ */
+ # $Change$
+ line4
+ line5
+ line6
+ line7
+ line8
+ EOF
+ cp filek fileko &&
+ sed -i "s/Revision/Revision: do not scrub me/" fileko
+ cp fileko file_text &&
+ sed -i "s/Id/Id: do not scrub me/" file_text
+ p4 add -t text+k filek &&
+ p4 submit -d "filek" &&
+ p4 add -t text+ko fileko &&
+ p4 submit -d "fileko" &&
+ p4 add -t text file_text &&
+ p4 submit -d "file_text"
+ )
+'
+
+#
+# Generate these in a function to make it easy to use single quote marks.
+#
+write_scrub_scripts () {
+ cat >"$TRASH_DIRECTORY/scrub_k.py" <<-\EOF &&
+ import re, sys
+ sys.stdout.write(re.sub(r'(?i)\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', sys.stdin.read()))
+ EOF
+ cat >"$TRASH_DIRECTORY/scrub_ko.py" <<-\EOF
+ import re, sys
+ sys.stdout.write(re.sub(r'(?i)\$(Id|Header):[^$]*\$', r'$\1$', sys.stdin.read()))
+ EOF
+}
+
+test_expect_success 'scrub scripts' '
+ write_scrub_scripts
+'
+
+#
+# Compare $cli/file to its scrubbed version, should be different.
+# Compare scrubbed $cli/file to $git/file, should be same.
+#
+scrub_k_check () {
+ file="$1" &&
+ scrub="$TRASH_DIRECTORY/$file" &&
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/scrub_k.py" <"$git/$file" >"$scrub" &&
+ ! test_cmp "$cli/$file" "$scrub" &&
+ test_cmp "$git/$file" "$scrub" &&
+ rm "$scrub"
+}
+scrub_ko_check () {
+ file="$1" &&
+ scrub="$TRASH_DIRECTORY/$file" &&
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/scrub_ko.py" <"$git/$file" >"$scrub" &&
+ ! test_cmp "$cli/$file" "$scrub" &&
+ test_cmp "$git/$file" "$scrub" &&
+ rm "$scrub"
+}
+
+#
+# Modify far away from keywords. If no RCS lines show up
+# in the diff, there is no conflict.
+#
+test_expect_success 'edit far away from RCS lines' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ sed -i "s/^line7/line7 edit/" filek &&
+ git commit -m "filek line7 edit" filek &&
+ "$GITP4" submit &&
+ scrub_k_check filek
+ )
+'
+
+#
+# Modify near the keywords. This will require RCS scrubbing.
+#
+test_expect_success 'edit near RCS lines' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "s/^line4/line4 edit/" filek &&
+ git commit -m "filek line4 edit" filek &&
+ "$GITP4" submit &&
+ scrub_k_check filek
+ )
+'
+
+#
+# Modify the keywords themselves. This also will require RCS scrubbing.
+#
+test_expect_success 'edit keyword lines' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "/Revision/d" filek &&
+ git commit -m "filek remove Revision line" filek &&
+ "$GITP4" submit &&
+ scrub_k_check filek
+ )
+'
+
+#
+# Scrubbing text+ko files should not alter all keywords, just Id, Header.
+#
+test_expect_success 'scrub ko files differently' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "s/^line4/line4 edit/" fileko &&
+ git commit -m "fileko line4 edit" fileko &&
+ "$GITP4" submit &&
+ scrub_ko_check fileko &&
+ ! scrub_k_check fileko
+ )
+'
+
+# hack; git-p4 submit should do it on its own
+test_expect_success 'cleanup after failure' '
+ (
+ cd "$cli" &&
+ p4 revert ...
+ )
+'
+
+#
+# Do not scrub anything but +k or +ko files. Sneak a change into
+# the cli file so that submit will get a conflict. Make sure that
+# scrubbing doesn't make a mess of things.
+#
+# Assumes that git-p4 exits leaving the p4 file open, with the
+# conflict-generating patch unapplied.
+#
+# This might happen only if the git repo is behind the p4 repo at
+# submit time, and there is a conflict.
+#
+test_expect_success 'do not scrub plain text' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "s/^line4/line4 edit/" file_text &&
+ git commit -m "file_text line4 edit" file_text &&
+ (
+ cd "$cli" &&
+ p4 open file_text &&
+ sed -i "s/^line5/line5 p4 edit/" file_text &&
+ p4 submit -d "file5 p4 edit"
+ ) &&
+ ! "$GITP4" submit &&
+ (
+ # exepct something like:
+ # file_text - file(s) not opened on this client
+ # but not copious diff output
+ cd "$cli" &&
+ p4 diff file_text >wc &&
+ test_line_count = 1 wc
+ )
+ )
+'
+
+# hack; git-p4 submit should do it on its own
+test_expect_success 'cleanup after failure 2' '
+ (
+ cd "$cli" &&
+ p4 revert ...
+ )
+'
+
+create_kw_file () {
+ cat <<\EOF >"$1"
+/* A file
+ Id: $Id$
+ Revision: $Revision$
+ File: $File$
+ */
+int main(int argc, const char **argv) {
+ return 0;
+}
+EOF
+}
+
+test_expect_success 'add kwfile' '
+ (
+ cd "$cli" &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "file 1" &&
+ create_kw_file kwfile1.c &&
+ p4 add kwfile1.c &&
+ p4 submit -d "Add rcw kw file" kwfile1.c
+ )
+'
+
+p4_append_to_file () {
+ f="$1" &&
+ p4 edit -t ktext "$f" &&
+ echo "/* $(date) */" >>"$f" &&
+ p4 submit -d "appending a line in p4"
+}
+
+# Create some files with RCS keywords. If they get modified
+# elsewhere then the version number gets bumped which then
+# results in a merge conflict if we touch the RCS kw lines,
+# even though the change itself would otherwise apply cleanly.
+test_expect_success 'cope with rcs keyword expansion damage' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ (cd ../cli && p4_append_to_file kwfile1.c) &&
+ old_lines=$(wc -l <kwfile1.c) &&
+ perl -n -i -e "print unless m/Revision:/" kwfile1.c &&
+ new_lines=$(wc -l <kwfile1.c) &&
+ test $new_lines = $(($old_lines - 1)) &&
+
+ git add kwfile1.c &&
+ git commit -m "Zap an RCS kw line" &&
+ "$GITP4" submit &&
+ "$GITP4" rebase &&
+ git diff p4/master &&
+ "$GITP4" commit &&
+ echo "try modifying in both" &&
+ cd "$cli" &&
+ p4 edit kwfile1.c &&
+ echo "line from p4" >>kwfile1.c &&
+ p4 submit -d "add a line in p4" kwfile1.c &&
+ cd "$git" &&
+ echo "line from git at the top" | cat - kwfile1.c >kwfile1.c.new &&
+ mv kwfile1.c.new kwfile1.c &&
+ git commit -m "Add line in git at the top" kwfile1.c &&
+ "$GITP4" rebase &&
+ "$GITP4" submit
+ )
+'
+
+test_expect_success 'cope with rcs keyword file deletion' '
+ test_when_finished cleanup_git &&
+ (
+ cd "$cli" &&
+ echo "\$Revision\$" >kwdelfile.c &&
+ p4 add -t ktext kwdelfile.c &&
+ p4 submit -d "Add file to be deleted" &&
+ cat kwdelfile.c &&
+ grep 1 kwdelfile.c
+ ) &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ grep Revision kwdelfile.c &&
+ git rm -f kwdelfile.c &&
+ git commit -m "Delete a file containing RCS keywords" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ ! test -f kwdelfile.c
+ )
+'
+
+# If you add keywords in git of the form $Header$ then everything should
+# work fine without any special handling.
+test_expect_success 'Add keywords in git which match the default p4 values' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ echo "NewKW: \$Revision\$" >>kwfile1.c &&
+ git add kwfile1.c &&
+ git commit -m "Adding RCS keywords in git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ test -f kwfile1.c &&
+ grep "NewKW.*Revision.*[0-9]" kwfile1.c
+
+ )
+'
+
+# If you add keywords in git of the form $Header:#1$ then things will fail
+# unless git-p4 takes steps to scrub the *git* commit.
+#
+test_expect_failure 'Add keywords in git which do not match the default p4 values' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ echo "NewKW2: \$Revision:1\$" >>kwfile1.c &&
+ git add kwfile1.c &&
+ git commit -m "Adding RCS keywords in git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ grep "NewKW2.*Revision.*[0-9]" kwfile1.c
+
+ )
+'
+
+# Check that the existing merge conflict handling still works.
+# Modify kwfile1.c in git, and delete in p4. We should be able
+# to skip the git commit.
+#
+test_expect_success 'merge conflict handling still works' '
+ test_when_finished cleanup_git &&
+ (
+ cd "$cli" &&
+ echo "Hello:\$Id\$" >merge2.c &&
+ echo "World" >>merge2.c &&
+ p4 add -t ktext merge2.c &&
+ p4 submit -d "add merge test file"
+ ) &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ sed -e "/Hello/d" merge2.c >merge2.c.tmp &&
+ mv merge2.c.tmp merge2.c &&
+ git add merge2.c &&
+ git commit -m "Modifying merge2.c"
+ ) &&
+ (
+ cd "$cli" &&
+ p4 delete merge2.c &&
+ p4 submit -d "remove merge test file"
+ ) &&
+ (
+ cd "$git" &&
+ test -f merge2.c &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ !(echo "s" | "$GITP4" submit) &&
+ git rebase --skip &&
+ ! test -f merge2.c
+ )
+'
+
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.9.259.ga92e
^ permalink raw reply related
* Re: [PATCH] cherry-pick -x: always insert an empty line
From: Beat Bolli @ 2012-02-23 8:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Raible
In-Reply-To: <7vhaynp605.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]
On 2012-02-19 09:44, Junio C Hamano wrote:
> Beat Bolli <bbolli@ewanet.ch> writes:
>
>> When cherry-picking a commit that has only a summary, the -x option
>> creates an invalid commit message because it puts the hash of the commit
>> being picked on the second line which should be left empty.
>>
>> This patch fixes this buglet by always inserting an empty line before
>> the added line.
>>
>> Aside from that, even with a non-trivial commit the generated note
>> "(cherry picked from commit 555c9864971744abb558796aea28e12a1ac20839)"
>> seems abrupt when appended directly.
>>
>> Cc: Eric Raible <raible@nextest.com>
>> Signed-off-by: Beat Bolli <bbolli@ewanet.ch>
>> ---
>> sequencer.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index 5fcbcb8..63fd589 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -382,7 +382,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
>> }
>>
>> if (opts->record_origin) {
>> - strbuf_addstr(&msgbuf, "(cherry picked from commit ");
>> + strbuf_addstr(&msgbuf, "\n(cherry picked from commit ");
>
> Doesn't this need to be conditional?
>
> If we cherry pick your commit somewhere else with "-x -s", the resulting
> commit log message would end like this:
>
> Aside from that, even with a non-trivial commit the generated note
> "(cherry picked from commit 555c9864971744abb558796aea28e12a1ac20839)"
> seems abrupt when appended directly.
>
> Cc: Eric Raible <raible@nextest.com>
> Signed-off-by: Beat Bolli <bbolli@ewanet.ch>
>
> (cherry picked from commit 555c9864971744abb558796aea28e12a1ac20839)
> Signed-off-by: Frotz Xyzzy <nitfol@example.xz>
>
> which clearly is worse-looking with the extra LF you added in this patch.
What should the condition then be? What is the canonical format for
cherry-picks with Signed-off-by: and Cc: footer lines?
>
>> strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
>> strbuf_addstr(&msgbuf, ")\n");
>> }
--
mail: echo '<bNbolOli@ewaSPnetAM.ch>' | tr -d '[A-S]'
pgp: 0x506A903A; 49D5 794A EA77 F907 764F D89E 304B 93CF 506A 903A
gsm: 4.7.7.6.0.7.7.9.7.1.4.e164.arpa
icbm: 47.0452 N, 7.2715 E
http: www.drbeat.li
"It takes love over gold, and mind over matter" -- Dire Straits
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply
* Re: [PATCH] cherry-pick -x: always insert an empty line
From: Sebastian Schuberth @ 2012-02-23 8:56 UTC (permalink / raw)
To: git; +Cc: git, Eric Raible
In-Reply-To: <1329599690-9152-1-git-send-email-bbolli@ewanet.ch>
On 18.02.2012 22:14, Beat Bolli wrote:
> This patch fixes this buglet by always inserting an empty line before
> the added line.
Thanks for this! Not having an empty line there has annoyed me, too,
several times.
--
Sebastian Schuberth
^ permalink raw reply
* git log -z doesn't separate commits with NULs
From: Nikolaj Shurkaev @ 2012-02-23 9:14 UTC (permalink / raw)
To: git
Hello all.
I wanted to generate several files with some statistics using "git log
-z" command.
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0
--max-chars=1000000 ~/1.sh
If I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of
multiple times.
I'm newbie to xargs, thus I tested with and that worked as I expected.
find . -type f -print0 | xargs -0 ./1.sh
That produced a lost of "started" lines.
Thus I suspect there is a but in git log -z command and that doesn't
"Separate the commits with NULs instead of with new newlines." as
promised in the documents.
Is my understanding correct or I don't understand the documentation or
somehow pass wrong parameters into git log?
Thank you.
Best regards,
Nikolaj
^ permalink raw reply
* Re: [PATCH] cherry-pick -x: always insert an empty line
From: Sebastian Schuberth @ 2012-02-23 9:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Beat Bolli, git, Eric Raible
In-Reply-To: <7vhaynp605.fsf@alter.siamese.dyndns.org>
On 19.02.2012 09:44, Junio C Hamano wrote:
> If we cherry pick your commit somewhere else with "-x -s", the resulting
> commit log message would end like this:
>
> Aside from that, even with a non-trivial commit the generated note
> "(cherry picked from commit 555c9864971744abb558796aea28e12a1ac20839)"
> seems abrupt when appended directly.
>
> Cc: Eric Raible<raible@nextest.com>
> Signed-off-by: Beat Bolli<bbolli@ewanet.ch>
>
> (cherry picked from commit 555c9864971744abb558796aea28e12a1ac20839)
> Signed-off-by: Frotz Xyzzy<nitfol@example.xz>
>
> which clearly is worse-looking with the extra LF you added in this patch.
Clearly? I would not say so, on the contrary. When using -s together
with -x, I'd interpret this as signing off the process of cherry-picking
itself, and as such, visually grouping the additional "signed-off" with
the "cherry picked from" makes sense to me.
--
Sebastian Schuberth
^ permalink raw reply
* [PATCH v2 1/4] strbuf: improve strbuf_get*line documentation
From: Thomas Rast @ 2012-02-23 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <cover.1329988335.git.trast@student.ethz.ch>
strbuf_getline() was not documented very clearly, though a reader
familiar with getline() would not have had any questions about it.
strbuf_getwholeline() was not documented at all.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/technical/api-strbuf.txt | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index afe2759..cc6db5e 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -255,8 +255,16 @@ same behaviour as well.
`strbuf_getline`::
- Read a line from a FILE* pointer. The second argument specifies the line
- terminator character, typically `'\n'`.
+ Read a line from a FILE*. The second argument specifies the
+ line terminator character, typically `'\n'`. Reading stops
+ after the terminator or at EOF. The terminator is removed
+ from the buffer before returning. Returns 0 unless there was
+ nothing left before EOF, in which case it returns `EOF`.
+
+`strbuf_getwholeline`::
+
+ Like `strbuf_getline`, but keeps the trailing terminator (if
+ any) in the buffer.
`stripspace`::
--
1.7.9.1.430.g4998543
^ permalink raw reply related
* [PATCH v2 2/4] bundle: put strbuf_readline_fd in strbuf.c with adjustments
From: Thomas Rast @ 2012-02-23 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <cover.1329988335.git.trast@student.ethz.ch>
The comment even said that it should eventually go there. While at
it, match the calling convention and name of the function to the
strbuf_get*line family. So it now is strbuf_getwholeline_fd.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/technical/api-strbuf.txt | 7 +++++++
bundle.c | 21 ++-------------------
strbuf.c | 16 ++++++++++++++++
strbuf.h | 1 +
4 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index cc6db5e..827c4bd 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -266,6 +266,13 @@ same behaviour as well.
Like `strbuf_getline`, but keeps the trailing terminator (if
any) in the buffer.
+`strbuf_getwholeline_fd`::
+
+ Like `strbuf_getwholeline`, but operates on a file descriptor.
+ It reads one character at a time, so it is very slow. Do not
+ use it unless you need the correct position in the file
+ descriptor!
+
`stripspace`::
Strip whitespace from a buffer. The second parameter controls if
diff --git a/bundle.c b/bundle.c
index b8acf3c..313de42 100644
--- a/bundle.c
+++ b/bundle.c
@@ -23,23 +23,6 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,
list->nr++;
}
-/* Eventually this should go to strbuf.[ch] */
-static int strbuf_readline_fd(struct strbuf *sb, int fd)
-{
- strbuf_reset(sb);
-
- while (1) {
- char ch;
- ssize_t len = xread(fd, &ch, 1);
- if (len <= 0)
- return len;
- strbuf_addch(sb, ch);
- if (ch == '\n')
- break;
- }
- return 0;
-}
-
static int parse_bundle_header(int fd, struct bundle_header *header,
const char *report_path)
{
@@ -47,7 +30,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header,
int status = 0;
/* The bundle header begins with the signature */
- if (strbuf_readline_fd(&buf, fd) ||
+ if (strbuf_getwholeline_fd(&buf, fd, '\n') ||
strcmp(buf.buf, bundle_signature)) {
if (report_path)
error("'%s' does not look like a v2 bundle file",
@@ -57,7 +40,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header,
}
/* The bundle header ends with an empty line */
- while (!strbuf_readline_fd(&buf, fd) &&
+ while (!strbuf_getwholeline_fd(&buf, fd, '\n') &&
buf.len && buf.buf[0] != '\n') {
unsigned char sha1[20];
int is_prereq = 0;
diff --git a/strbuf.c b/strbuf.c
index ff0b96b..5135d59 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -383,6 +383,22 @@ int strbuf_getline(struct strbuf *sb, FILE *fp, int term)
return 0;
}
+int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
+{
+ strbuf_reset(sb);
+
+ while (1) {
+ char ch;
+ ssize_t len = xread(fd, &ch, 1);
+ if (len <= 0)
+ return EOF;
+ strbuf_addch(sb, ch);
+ if (ch == term)
+ break;
+ }
+ return 0;
+}
+
int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
{
int fd, len;
diff --git a/strbuf.h b/strbuf.h
index fbf059f..3effaa8 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -116,6 +116,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)
extern int strbuf_getwholeline(struct strbuf *, FILE *, int);
extern int strbuf_getline(struct strbuf *, FILE *, int);
+extern int strbuf_getwholeline_fd(struct strbuf *, int, int);
extern void stripspace(struct strbuf *buf, int skip_comments);
extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
--
1.7.9.1.430.g4998543
^ permalink raw reply related
* [PATCH v2 4/4] bundle: use a strbuf to scan the log for boundary commits
From: Thomas Rast @ 2012-02-23 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <cover.1329988335.git.trast@student.ethz.ch>
The first part of the bundle header contains the boundary commits, and
could be approximated by
# v2 git bundle
$(git rev-list --pretty=oneline --boundary <ARGS> | grep ^-)
git-bundle actually spawns exactly this rev-list invocation, and does
the grepping internally.
There was a subtle bug in the latter step: it used fgets() with a
1024-byte buffer. If the user has sufficiently long subjects (e.g.,
by not adhering to the git oneline-subject convention in the first
place), the 'oneline' format can easily overflow the buffer. fgets()
then returns only the first part of the line _without a \n_, and it
would be printed as such, turning the next line into trailing garbage.
On top of that, fgets() returns the rest of the line in the next
call(s). If one of these remaining parts starts with '-', git-bundle
would mistakenly insert it into the boundary list.
Fix it by using strbuf_getwholeline() instead, which handles arbitrary
line lengths correctly.
Note that on the receiving side in parse_bundle_header() we were
already using strbuf_getwholeline_fd(), so that part is safe.
Thanks to Peff, Johannes Sixt and Junio for suggestions.
Reported-by: Jannis Pohlmann <jannis.pohlmann@codethink.co.uk>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
bundle.c | 15 ++++++++-------
t/t5704-bundle.sh | 16 ++++++++++++++++
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/bundle.c b/bundle.c
index 313de42..7a760db 100644
--- a/bundle.c
+++ b/bundle.c
@@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path,
const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
const char **argv_pack = xmalloc(6 * sizeof(const char *));
int i, ref_count = 0;
- char buffer[1024];
+ struct strbuf buf = STRBUF_INIT;
struct rev_info revs;
struct child_process rls;
FILE *rls_fout;
@@ -266,20 +266,21 @@ int create_bundle(struct bundle_header *header, const char *path,
if (start_command(&rls))
return -1;
rls_fout = xfdopen(rls.out, "r");
- while (fgets(buffer, sizeof(buffer), rls_fout)) {
+ while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
unsigned char sha1[20];
- if (buffer[0] == '-') {
- write_or_die(bundle_fd, buffer, strlen(buffer));
- if (!get_sha1_hex(buffer + 1, sha1)) {
+ if (buf.len > 0 && buf.buf[0] == '-') {
+ write_or_die(bundle_fd, buf.buf, buf.len);
+ if (!get_sha1_hex(buf.buf + 1, sha1)) {
struct object *object = parse_object(sha1);
object->flags |= UNINTERESTING;
- add_pending_object(&revs, object, buffer);
+ add_pending_object(&revs, object, buf.buf);
}
- } else if (!get_sha1_hex(buffer, sha1)) {
+ } else if (!get_sha1_hex(buf.buf, sha1)) {
struct object *object = parse_object(sha1);
object->flags |= SHOWN;
}
}
+ strbuf_release(&buf);
fclose(rls_fout);
if (finish_command(&rls))
return error("rev-list died");
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index f35f559..a51c8b0 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -42,4 +42,20 @@ test_expect_success 'empty bundle file is rejected' '
test_must_fail git fetch empty-bundle
'
+# This triggers a bug in older versions where the resulting line (with
+# --pretty=oneline) was longer than a 1024-char buffer.
+test_expect_success 'ridiculously long subject in boundary' '
+ : >file4 &&
+ test_tick &&
+ git add file4 &&
+ printf "%01200d\n" 0 | git commit -F - &&
+ test_commit fifth &&
+ git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
+ git bundle list-heads long-subject-bundle.bdl >heads &&
+ test -s heads &&
+ git fetch long-subject-bundle.bdl &&
+ sed -n "/^-/{p;q}" long-subject-bundle.bdl >boundary &&
+ grep "^-$_x40 " boundary
+'
+
test_done
--
1.7.9.1.430.g4998543
^ permalink raw reply related
* [PATCH v2 0/4] Making an elephant out of a getline() bug
From: Thomas Rast @ 2012-02-23 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <7vty2i5ie2.fsf@alter.siamese.dyndns.org>
So you all made very good points, and I don't want to repeat them.
Junio's analysis
> Perhaps we would want to squash something like this to the test to avoid
> "seq", using J6t's idea. The issue is that we do not write the end of
> line for the long boundary (because it is hidden from us), keep reading
> and rejecting bogus letters, and then the tip is written without
> terminating the boundary line. So checking boundary line is not a very
> useful test, but "fetch" and "list-heads" are.
is of course also correct for the case where the overlong line is in a
boundary commit. (The analysis about the accidentally-leading '-' is
also correct and can create a boundary line where there was none, like
in the perl case.)
The patches in this round are:
[1] new; following up on Peff's complaint that I should document
strbuf_getwholeline_fd, I found that the strbuf_get*line
documentation was also lacking.
[2] previously (1/2), now with documentation added
[3] new; Junio corrected me on the ': > file' spelling, which I added
because the start of the test used it too. So let's just fix it
outright to match the Git style all over.
[4] previously (2/2), now incorporating a strbuf_release and the
suggestions for the tests as sent by Junio. I upped the %0982 a
bit as it felt *too* tailored for the old bug, and this way
everyone can see immediately that it will exceed the 1024 char
limit (without thinking about the length of a sha and accounting
for \0 and \n).
Thomas Rast (4):
strbuf: improve strbuf_get*line documentation
bundle: put strbuf_readline_fd in strbuf.c with adjustments
t5704: match tests to modern style
bundle: use a strbuf to scan the log for boundary commits
Documentation/technical/api-strbuf.txt | 19 +++++++++++--
bundle.c | 36 +++++++-----------------
strbuf.c | 16 +++++++++++
strbuf.h | 1 +
t/t5704-bundle.sh | 47 ++++++++++++++++----------------
5 files changed, 67 insertions(+), 52 deletions(-)
--
1.7.9.1.430.g4998543
^ permalink raw reply
* [PATCH v2 3/4] t5704: match tests to modern style
From: Thomas Rast @ 2012-02-23 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <cover.1329988335.git.trast@student.ethz.ch>
The test did not adhere to the current style on several counts:
- empty lines around the test blocks, but within the test string
- ': > file' or even just '> file' with an extra space
- inconsistent indentation
- hand-rolled commits instead of using test_commit
Fix all of them. There's a catch to the last point: test_commit
creates a tag. We still change it to test_commit, and explicitly
delete the tags, so as to highlight that the test relies on not having
them.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
t/t5704-bundle.sh | 33 ++++++++-------------------------
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 4ae127d..f35f559 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -4,59 +4,42 @@ test_description='some bundle related tests'
. ./test-lib.sh
test_expect_success 'setup' '
-
- : > file &&
- git add file &&
- test_tick &&
- git commit -m initial &&
+ test_commit initial &&
test_tick &&
git tag -m tag tag &&
- : > file2 &&
- git add file2 &&
- : > file3 &&
- test_tick &&
- git commit -m second &&
- git add file3 &&
- test_tick &&
- git commit -m third
-
+ test_commit second &&
+ test_commit third &&
+ git tag -d initial &&
+ git tag -d second &&
+ git tag -d third
'
test_expect_success 'tags can be excluded by rev-list options' '
-
git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
git ls-remote bundle > output &&
! grep tag output
-
'
test_expect_success 'die if bundle file cannot be created' '
-
mkdir adir &&
test_must_fail git bundle create adir --all
-
'
test_expect_failure 'bundle --stdin' '
-
echo master | git bundle create stdin-bundle.bdl --stdin &&
git ls-remote stdin-bundle.bdl >output &&
grep master output
-
'
test_expect_failure 'bundle --stdin <rev-list options>' '
-
echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
git ls-remote hybrid-bundle.bdl >output &&
grep master output
-
'
test_expect_success 'empty bundle file is rejected' '
-
- >empty-bundle && test_must_fail git fetch empty-bundle
-
+ : >empty-bundle &&
+ test_must_fail git fetch empty-bundle
'
test_done
--
1.7.9.1.430.g4998543
^ permalink raw reply related
* Re: [PATCH v2] contrib: added git-diffall
From: Stefano Lattarini @ 2012-02-23 9:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tim Henigan, git, davvid
In-Reply-To: <7vipiy8m5q.fsf@alter.siamese.dyndns.org>
Hello everybody. Hope you don't mind 2 cents from an outsider ...
On 02/23/2012 12:48 AM, Junio C Hamano wrote:
>
> Tim Henigan <tim.henigan@gmail.com> writes:
>
>> +# mktemp is not available on all platforms (missing from msysgit)
>> +# Use a hard-coded tmp dir if it is not available
>> +tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || {
>> + tmp=/tmp/git-diffall-tmp
>> +}
>
> It would not withstand malicious attacks, but doing
>
> tmp=/tmp/git-diffall-tmp.$$
>
> would at least protect you from accidental name crashes better in the
> fallback codepath.
>
Maybe this would be enough to withstand malicious attacks (even if not
denial-of-service attacks):
# mktemp is not available on all platforms (missing from msysgit)
tmp=$(mktemp -d -t tmp.XXXXXX 2>/dev/null) || {
tmp=/tmp/git-diffall-tmp.$$
mkdir "$tmp" || fatal "couldn't create temporary directory"
}
>
>> +mkdir -p "$tmp"
>
At which point this should be removed, of course.
Regards,
Stefano
^ permalink raw reply
* Re: git log -z doesn't separate commits with NULs
From: Luke Diamand @ 2012-02-23 10:02 UTC (permalink / raw)
To: Nikolaj Shurkaev; +Cc: git
In-Reply-To: <4F46036F.3040406@gmail.com>
On 23/02/12 09:14, Nikolaj Shurkaev wrote:
> Hello all.
>
> I wanted to generate several files with some statistics using "git log
> -z" command.
> I did something like this:
> git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0
> --max-chars=1000000 ~/1.sh
>
> If I put
> echo "started"
> into the file ~/1.sh I see that the file is called only once instead of
> multiple times.
>
> I'm newbie to xargs, thus I tested with and that worked as I expected.
> find . -type f -print0 | xargs -0 ./1.sh
> That produced a lost of "started" lines.
>
> Thus I suspect there is a but in git log -z command and that doesn't
> "Separate the commits with NULs instead of with new newlines." as
> promised in the documents.
> Is my understanding correct or I don't understand the documentation or
> somehow pass wrong parameters into git log?
Just a guess, but doesn't the "--patch" option to git log ask it to
produce a patch output? Surely that will override the -z: patch will not
be expecting NULs.
>
> Thank you.
> Best regards,
> Nikolaj
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Jeff King @ 2012-02-23 10:04 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20120222204050.GB6781@sigill.intra.peff.net>
On Wed, Feb 22, 2012 at 03:40:50PM -0500, Jeff King wrote:
> I'll re-send the patch with a stand-alone commit message.
Here it is.
-- >8 --
Subject: [PATCH] disconnect from remote helpers more gently
When git spawns a remote helper program (like git-remote-http),
the last thing we do before closing the pipe to the child
process is to send a blank line, telling the helper that we
are done issuing commands. However, the helper may already
have exited, in which case the parent git process will
receive SIGPIPE and die.
In particular, this can happen with the remote-curl helper
when it encounters errors during a push. The helper reports
individual errors for each ref back to git-push, and then
exits with a non-zero exit code. Depending on the exact
timing of the write, the parent process may or may not
receive SIGPIPE.
This causes intermittent test failure in t5541.8, and is a
side effect of 5238cbf (remote-curl: Fix push status report
when all branches fail). Before that commit, remote-curl
would not send the final blank line to indicate that the
list of status lines was complete; it would just exit,
closing the pipe. The parent git-push would notice the
closed pipe while reading the status report and exit
immediately itself, propagating the failing exit code. But
post-5238cbf, remote-curl completes the status list before
exiting, git-push actually runs to completion, and then it
tries to cleanly disconnect the helper, leading to the
SIGPIPE race above.
This patch drops all error-checking when sending the final
"we are about to hang up" blank line to helpers. There is
nothing useful for the parent process to do about errors at
that point anyway, and certainly failing to send our "we are
done with commands" line to a helper that has already exited
is not a problem.
Signed-off-by: Jeff King <peff@peff.net>
---
transport-helper.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 6f227e2..f6b3b1f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -9,6 +9,7 @@
#include "remote.h"
#include "string-list.h"
#include "thread-utils.h"
+#include "sigchain.h"
static int debug;
@@ -220,15 +221,21 @@ static struct child_process *get_helper(struct transport *transport)
static int disconnect_helper(struct transport *transport)
{
struct helper_data *data = transport->data;
- struct strbuf buf = STRBUF_INIT;
int res = 0;
if (data->helper) {
if (debug)
fprintf(stderr, "Debug: Disconnecting.\n");
if (!data->no_disconnect_req) {
- strbuf_addf(&buf, "\n");
- sendline(data, &buf);
+ /*
+ * Ignore write errors; there's nothing we can do,
+ * since we're about to close the pipe anyway. And the
+ * most likely error is EPIPE due to the helper dying
+ * to report an error itself.
+ */
+ sigchain_push(SIGPIPE, SIG_IGN);
+ xwrite(data->helper->in, "\n", 1);
+ sigchain_pop(SIGPIPE);
}
close(data->helper->in);
close(data->helper->out);
--
1.7.8.4.8.g10fac
^ permalink raw reply related
* Re: [PATCH v2 1/4] strbuf: improve strbuf_get*line documentation
From: Jeff King @ 2012-02-23 10:08 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, Johannes Sixt, Jannis Pohlmann, git
In-Reply-To: <d5522f8d1154321804cb60fb4f53d538242ead88.1329988335.git.trast@student.ethz.ch>
On Thu, Feb 23, 2012 at 10:42:21AM +0100, Thomas Rast wrote:
> strbuf_getline() was not documented very clearly, though a reader
> familiar with getline() would not have had any questions about it.
> strbuf_getwholeline() was not documented at all.
Thanks for improving the existing docs. One suggestion:
> `strbuf_getline`::
>
> - Read a line from a FILE* pointer. The second argument specifies the line
> - terminator character, typically `'\n'`.
> + Read a line from a FILE*. The second argument specifies the
> + line terminator character, typically `'\n'`. Reading stops
> + after the terminator or at EOF. The terminator is removed
> + from the buffer before returning. Returns 0 unless there was
> + nothing left before EOF, in which case it returns `EOF`.
The get*line functions are unlike the rest of the strbuf API in that
they overwrite, rather than append to, the strbuf argument. Maybe:
s/from a FILE\*/&, overwriting the existing contents of the strbuf/
?
-Peff
^ permalink raw reply
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