Git development
 help / color / mirror / Atom feed
* Re: Auto-repo-repair
From: Enrico Weigelt @ 2012-11-19 22:35 UTC (permalink / raw)
  To: Drew Northup; +Cc: Jeff King, git
In-Reply-To: <CAM9Z-nmu2MiE9vF9T6Aw8vFTR8mTkuR3akHgZX6+=n3uA4fmpA@mail.gmail.com>


> >> How would the broken repository be sure of what it is missing to
> >> request it from the other side?
> >
> > fsck will find missing objects.
> 
> And what about the objects referred to by objects that are missing?

Will be fetched after multiple iterations.
We could even introduce some 'fsck --autorepair' mode, which triggers
it to fetch any missing object from its remotes.

Maybe even introduce a concept of peer object stores, which (a bit like
alternates) are asked for objects that arent locally availabe - that
could be even a plain venti store.


cu
-- 
Mit freundlichen Grüßen / Kind regards 

Enrico Weigelt 
VNC - Virtual Network Consult GmbH 
Head Of Development 

Pariser Platz 4a, D-10117 Berlin
Tel.: +49 (30) 3464615-20
Fax: +49 (30) 3464615-59

enrico.weigelt@vnc.biz; www.vnc.de 

^ permalink raw reply

* Re: [PATCH v3 0/6] completion: test consolidations
From: Felipe Contreras @ 2012-11-19 22:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, git
In-Reply-To: <7v6251xrfb.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 9:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> These started from a discussion with SZEDER, but then I realized there were
>> many improvements possible.
>
> Thanks; I'd loop him in and wait for his acks/reviews.

I thought my series-cc-cmd would add him. Maybe I'm using a version of
'git send-email' that doesn't have that.

Either way, we already know what he will say, to the previous series
he said he was against consolidation of test scripts. So I don't see
how the situation would change.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-19 22:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <7vk3thxuj2.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
> Given that the problematic line
> 
> 	Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y
> 
> is not even a valid e-mail address, doesn't this new logic belong to
> sanitize_address() conceptually?

Yes, it's much better to do it in the sanitize_address().

Felipe, may you check it?

Krzysiek
-- >8 --
Subject: [PATCH] git-send-email: remove garbage after email address

In some cases it's very useful to add some additional information
after email in Cc-list, for instance:

"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6"

Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.

Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
 git-send-email.perl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..9840d0a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
 	my ($recipient) = @_;
+
+	# remove garbage after email address
+	$recipient =~ s/(.*>).*$/$1/;
+
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
 
 	if (not $recipient_name) {
-- 
1.8.0.283.gc57d856

^ permalink raw reply related

* git config --git-all can return non-zero error code
From: Timur Tabi @ 2012-11-19 23:02 UTC (permalink / raw)
  To: git

According to the man page for git-config, the --git-all option is not
supposed to return an error code:

--get-all
	Like get, but does not fail if the number of values for the key is
	not exactly one.

IMHO, zero is also "not exactly one", but I still get a 1 exit code if the
key does not exist.

My .git/config says this:

[user "cq.branch"]
	mastr = upstream/master

git-config --get-all does this:

$ git config --get-all user.cq.branch.master
$ echo $?
1
$ git config --get-all user.cq.branch.mastr
upstream/master
$ echo $?
0

I just want git to return nothing if the key doesn't exist.  I don't want
it to return an exit code.  Is there a way to do this?  I think either the
code is broken or the documentation needs to be changed.

I'm running git version 1.7.3.4

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Felipe Contreras @ 2012-11-19 23:12 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: Junio C Hamano, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <20121119225838.GA23412@shrek.podlesie.net>

On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:

> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -924,6 +924,10 @@ sub quote_subject {
>  # use the simplest quoting being able to handle the recipient
>  sub sanitize_address {
>         my ($recipient) = @_;
> +
> +       # remove garbage after email address
> +       $recipient =~ s/(.*>).*$/$1/;
> +

Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test
<foo@bar.com> <#comment>' gets the second comment removed.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Junio C Hamano @ 2012-11-19 23:55 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'.

Bunch of topics have been merged to 'next'.

We are at the beginning of the 5th week of this release cycle
(cf. http://tinyurl.com/gitcal), and I've moved many topics to the
Stalled category, which will be discarded without prejudice soonish
unless there are some updates.  I am still a bit behind on some
topics and already posted rerolls may have to be pulled in.

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

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

* nd/pathspec-wildcard (2012-11-19) 4 commits
 - tree_entry_interesting: do basedir compare on wildcard patterns when possible
 - pathspec: apply "*.c" optimization from exclude
 - pathspec: do exact comparison on the leading non-wildcard part
 - pathspec: save the non-wildcard length part

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

* pf/editor-ignore-sigint (2012-11-11) 5 commits
 - launch_editor: propagate SIGINT from editor to git
 - run-command: do not warn about child death by SIGINT
 - run-command: drop silent_exec_failure arg from wait_or_whine
 - launch_editor: ignore SIGINT while the editor has control
 - launch_editor: refactor to use start/finish_command

 Avoid confusing cases where the user hits Ctrl-C while in the editor
 session, not realizing git will receive the signal. Since most editors
 will take over the terminal and will block SIGINT, this is not likely
 to confuse anyone.

 Some people raised issues with emacsclient, which are addressed by this
 re-roll. It should probably also handle SIGQUIT, and there were a
 handful of other review comments.

 Expecting a re-roll.


* pp/gitweb-config-underscore (2012-11-08) 1 commit
 - gitweb: make remote_heads config setting work

 The key "gitweb.remote_heads" is not legal git config; this maps it to
 "gitweb.remoteheads".

 Junio raised a good point about the implementation for three-level
 variables.

 Expecting a re-roll.


* mo/cvs-server-updates (2012-10-16) 10 commits
 - 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

 Needs review by folks interested in cvsserver.


* jc/apply-trailing-blank-removal (2012-10-12) 1 commit
 - apply.c:update_pre_post_images(): the preimage can be truncated

 Fix to update_pre_post_images() that did not take into account the
 possibility that whitespace fix could shrink the preimage and
 change the number of lines in it.

 Extra set of eyeballs appreciated.


* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
 - config: exit on error accessing any config file
 - doc: advertise GIT_CONFIG_NOSYSTEM
 - config: treat user and xdg config permission problems as errors
 - config, gitignore: failure to access with ENOTDIR is ok

 An RFC to deal with a situation where .config/git is a file and we
 notice .config/git/config is not readable due to ENOTDIR, not
 ENOENT; I think a bit more refactored approach to consistently
 address permission errors across config, exclude and attrs is
 desirable.  Don't we also need a check for an opposite situation
 where we open .config/git/config or .gitattributes for reading but
 they turn out to be directories?


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - 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

 Duy helped to reroll this.

 Expecting a re-roll.


* aw/rebase-am-failure-detection (2012-10-11) 1 commit
 - rebase: Handle cases where format-patch fails

 I am unhappy a bit about the possible performance implications of
 having to store the output in a temporary file only for a rare case
 of format-patch aborting.


* 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.


* fc/remote-testgit-feature-done (2012-10-29) 1 commit
 - remote-testgit: properly check for errors

 Is this still in "Needs review" state?  Are poeple involved in the
 remote interface happy with this change?


* jk/send-email-sender-prompt (2012-11-15) 8 commits
 - send-email: do not prompt for explicit repo ident
 - Git.pm: teach "ident" to query explicitness
 - var: provide explicit/implicit ident information
 - var: accept multiple variables on the command line
 - ident: keep separate "explicit" flags for author and committer
 - ident: make user_ident_explicitly_given static
 - t7502: factor out autoident prerequisite
 - test-lib: allow negation of prerequisites

 Avoid annoying sender prompt in git-send-email, but only when it is
 safe to do so.

 Perhaps keep only the first three patches, and replace the rest
 with the one from Felipe that takes a much simpler approach (the
 rationale of that patch needs to be cleaned up first, along the
 lines Jeff outlined, though).  Frozen until that happens.


* nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
 - Unify appending signoff in format-patch, commit and sequencer

 I am not sure if the logic to refrain from adding a sign-off based
 on the existing run of sign-offs is done correctly in this change.


* nd/pretty-placeholder-with-color-option (2012-09-30) 9 commits
 . pretty: support %>> that steal trailing spaces
 . pretty: support truncating in %>, %< and %><
 . pretty: support padding placeholders, %< %> and %><
 . pretty: two phase conversion for non utf-8 commits
 . utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
 . utf8.c: move display_mode_esc_sequence_len() for use by other functions
 . pretty: support %C(auto[,N]) to turn on coloring on next placeholder(s)
 . pretty: split parsing %C into a separate function
 . pretty: share code between format_decoration and show_decorations

 This causes warnings with -Wuninitialized, so I've ejected it from pu
 for the time being.


* rc/maint-complete-git-p4 (2012-09-24) 1 commit
  (merged to 'next' on 2012-10-29 at af52cef)
 + Teach git-completion about git p4

 Comment from Pete will need to be addressed in a follow-up patch.


* as/test-tweaks (2012-09-20) 7 commits
 - tests: paint unexpectedly fixed known breakages in bold red
 - tests: test the test framework more thoroughly
 - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
 - tests: refactor mechanics of testing in a sub test-lib
 - tests: paint skipped tests in bold blue
 - tests: test number comes first in 'not ok $count - $message'
 - tests: paint known breakages in bold yellow

 Various minor tweaks to the test framework to paint its output
 lines in colors that match what they mean better.

 Has the "is this really blue?" issue Peff raised resolved???


* 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/blame-no-follow (2012-09-21) 2 commits
 - 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.

 Stalled mostly due to lack of responses.


* jc/doc-default-format (2012-10-07) 2 commits
 - [SQAUSH] allow "cd Doc* && make DEFAULT_DOC_TARGET=..."
 - Allow generating a non-default set of documentation

 Need to address the installation half if this is to be any useful.


* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
 - graph.c: infinite loop in git whatchanged --graph -m

 The --graph code fell into infinite loop when asked to do what the
 code did not expect ;-)

 Anybody who worked on "--graph" wants to comment?
 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.

 Waiting for comments.


* 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 re-roll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* mh/ceiling (2012-10-29) 8 commits
 - string_list_longest_prefix(): remove function
 - setup_git_directory_gently_1(): resolve symlinks in ceiling paths
 - longest_ancestor_length(): require prefix list entries to be normalized
 - longest_ancestor_length(): take a string_list argument for prefixes
 - longest_ancestor_length(): use string_list_split()
 - Introduce new function real_path_if_valid()
 - real_path_internal(): add comment explaining use of cwd
 - Introduce new static function real_path_internal()

 Elements of GIT_CEILING_DIRECTORIES list may not match the real
 pathname we obtain from getcwd(), leading the GIT_DIR discovery
 logic to escape the ceilings the user thought to have specified.

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

* jl/submodule-rm (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at bf4525d)
 + docs: move submodule section

 Documentation correction for d21240f (Merge branch
 'jl/submodule-rm', 2012-10-29) that needs to be fast-tracked.

 Will merge to 'master' in the sixth batch.


* sg/complete-help-undup (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at eadd0f3)
 + completion: remove 'help' duplicate from porcelain commands

 Will merge to 'master' in the seventh batch.


* bc/do-not-recurse-in-die (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 79d62a8)
 + usage.c: detect recursion in die routines and bail out immediately

 Will merge to 'master' in the seventh batch.


* cn/config-missing-path (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at c08b73c)
 + config: don't segfault when given --path with a missing value

 Will merge to 'master' in the sixth batch.


* jk/checkout-out-of-unborn (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 7d2aa24)
 + checkout: print a message when switching unborn branches

 Will merge to 'master' in the sixth batch.


* mk/complete-tcsh (2012-11-16) 1 commit
  (merged to 'next' on 2012-11-19 at 8309029)
 + tcsh-completion re-using git-completion.bash

 Will merge to 'master' in the seventh batch.


* mm/status-push-pull-advise (2012-11-16) 1 commit
 - status: add advice on how to push/pull to tracking branch

 Will merge to 'next'.


* jk/maint-gitweb-xss (2012-11-12) 1 commit
  (merged to 'next' on 2012-11-14 at 7a667bc)
 + gitweb: escape html in rss title

 Fixes an XSS vulnerability in gitweb.

 Will merge to 'master' in the sixth batch.


* mg/replace-resolve-delete (2012-11-13) 1 commit
  (merged to 'next' on 2012-11-14 at fa785ae)
 + replace: parse revision argument for -d

 Be more user friendly to people using "git replace -d".

 Will merge to 'master' in the sixth batch.


* ml/cygwin-mingw-headers (2012-11-18) 2 commits
  (merged to 'next' on 2012-11-19 at f9964da)
 + USE CGYWIN_V15_WIN32API as macro to select api for cygwin
  (merged to 'next' on 2012-11-15 at 22e11b3)
 + Update cygwin.c for new mingw-64 win32 api headers

 Make git work on newer cygwin.

 Will merge to 'master' in the sixth batch.


* ta/doc-cleanup (2012-10-25) 6 commits
  (merged to 'next' on 2012-11-13 at e11fafd)
 + Documentation: build html for all files in technical and howto
 + Documentation/howto: convert plain text files to asciidoc
 + Documentation/technical: convert plain text files to asciidoc
 + Change headline of technical/send-pack-pipeline.txt to not confuse its content with content from git-send-pack.txt
 + Shorten two over-long lines in git-bisect-lk2009.txt by abbreviating some sha1
 + Split over-long synopsis in git-fetch-pack.txt into several lines

 Will merge to 'master' in the sixth batch.


* lt/diff-stat-show-0-lines (2012-10-17) 1 commit
  (merged to 'next' on 2012-11-19 at 0037290)
 + Fix "git diff --stat" for interesting - but empty - file changes

 We failed to mention a file without any content change but whose
 permission bit was modified, or (worse yet) a new file without any
 content in the "git diff --stat" output.

 Will merge to 'master' in the seventh batch.


* fc/zsh-completion (2012-11-19) 2 commits
 - completion: start moving to the new zsh completion
 - completion: add new zsh completion

 Replaced by shedding large changes to other independent topics.
 Any comments from zsh users?


* so/prompt-command (2012-10-17) 4 commits
  (merged to 'next' on 2012-10-25 at 79565a1)
 + coloured git-prompt: paint detached HEAD marker in red
 + Fix up colored git-prompt
 + show color hints based on state of the git tree
 + Allow __git_ps1 to be used in PROMPT_COMMAND

 Updates __git_ps1 so that it can be used as $PROMPT_COMMAND,
 instead of being used for command substitution in $PS1, to embed
 color escape sequences in its output.

 Will cook in 'next'.


* nd/wildmatch (2012-10-15) 13 commits
  (merged to 'next' on 2012-10-25 at 510e8df)
 + 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

 Allows pathname patterns in .gitignore and .gitattributes files
 with double-asterisks "foo/**/bar" to match any number of directory
 hierarchies.

 I suspect that this needs to be plugged to pathspec matching code;
 otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
 commits that touch Documentation/git.txt, which would be confusing
 to the users.

 Will cook in 'next'.


* jh/update-ref-d-through-symref (2012-10-21) 2 commits
  (merged to 'next' on 2012-11-19 at 6bcca4c)
 + Fix failure to delete a packed ref through a symref
 + t1400-update-ref: Add test verifying bug with symrefs in delete_ref()

 "update-ref -d --deref SYM" to delete a ref through a symbolic ref
 that points to it did not remove it correctly.

 Will merge to 'master' in the seventh batch.


* jk/config-ignore-duplicates (2012-10-29) 9 commits
  (merged to 'next' on 2012-10-29 at 67fa0a2)
 + builtin/config.c: Fix a sparse warning
  (merged to 'next' on 2012-10-25 at 233df08)
 + git-config: use git_config_with_options
 + git-config: do not complain about duplicate entries
 + git-config: collect values instead of immediately printing
 + git-config: fix regexp memory leaks on error conditions
 + git-config: remove memory leak of key regexp
 + t1300: test "git config --get-all" more thoroughly
 + t1300: remove redundant test
 + t1300: style updates

 Drop duplicate detection from git-config; this lets it
 better match the internal config callbacks, which clears up
 some corner cases with includes.

 Will merge to 'master' in the sixth batch.


* fc/completion-test-simplification (2012-11-16) 6 commits
 - completion: simplify __gitcomp() test helper
 - completion: refactor __gitcomp related tests
 - completion: consolidate test_completion*() tests
 - completion: simplify tests using test_completion_long()
 - completion: standardize final space marker in tests
 - completion: add comment for test_completion()

 Clean up completion tests.  Use of conslidated helper may make
 instrumenting one particular test during debugging of the test
 itself, but I think that issue should be addressed in some other
 way (e.g. making sure individual tests in 9902 can be skipped).


* jk/pickaxe-textconv (2012-10-28) 2 commits
 - pickaxe: use textconv for -S counting
 - pickaxe: hoist empty needle check

 Use textconv filters when searching with "log -S".

 It probably should lose "are the textconv on the two sides the
 same?" check.


* fc/remote-bzr (2012-11-08) 5 commits
  (merged to 'next' on 2012-11-18 at 86add07)
 + remote-bzr: update working tree
 + remote-bzr: add support for remote repositories
 + remote-bzr: add support for pushing
 + remote-bzr: add simple tests
 + Add new remote-bzr transport helper

 New remote helper for bzr.

 Will merge to 'master' in the seventh batch.


* fc/remote-hg (2012-11-12) 20 commits
  (merged to 'next' on 2012-11-18 at 4a4f2e4)
 + remote-hg: avoid bad refs
 + remote-hg: try the 'tip' if no checkout present
 + remote-hg: fix compatibility with older versions of hg
 + remote-hg: add missing config for basic tests
 + remote-hg: the author email can be null
 + remote-hg: add option to not track branches
 + remote-hg: add extra author test
 + remote-hg: add tests to compare with hg-git
 + remote-hg: add bidirectional tests
 + test-lib: avoid full path to store test results
 + remote-hg: add basic tests
 + remote-hg: fake bookmark when there's none
 + remote-hg: add compat for hg-git author fixes
 + remote-hg: add support for hg-git compat mode
 + remote-hg: match hg merge behavior
 + remote-hg: make sure the encoding is correct
 + remote-hg: add support to push URLs
 + remote-hg: add support for remote pushing
 + remote-hg: add support for pushing
 + Add new remote-hg transport helper

 New remote helper for hg.

 Will merge to 'master' in the seventh batch.


* jk/maint-http-half-auth-fetch (2012-10-31) 2 commits
  (merged to 'next' on 2012-11-09 at af69926)
 + remote-curl: retry failed requests for auth even with gzip
 + remote-curl: hoist gzip buffer size to top of post_rpc

 Fixes fetch from servers that ask for auth only during the actual
 packing phase. This is not really a recommended configuration, but it
 cleans up the code at the same time.

 Will merge to 'master' in the sixth batch.


* kb/preload-index-more (2012-11-02) 1 commit
  (merged to 'next' on 2012-11-09 at a750ebd)
 + update-index/diff-index: use core.preloadindex to improve performance

 Use preloadindex in more places, which has a nice speedup on systems
 with slow stat calls (and even on Linux).

 Will merge to 'master' in the sixth batch.


* cr/push-force-tag-update (2012-11-19) 5 commits
 - push: update remote tags only with force
 - push: flag updates that require force
 - push: keep track of "update" state separately
 - push: add advice for rejected tag reference
 - push: return reject reasons via a mask

 Require "-f" for push to update a tag, even if it is a fast-forward.


* fc/fast-export-fixes (2012-11-08) 14 commits
 - fast-export: don't handle uninteresting refs
 - fast-export: make sure updated refs get updated
 - fast-export: fix comparison in tests
 - fast-export: trivial cleanup
 - remote-testgit: make clear the 'done' feature
 - remote-testgit: report success after an import
 - remote-testgit: exercise more features
 - remote-testgit: cleanup tests
 - remote-testgit: remove irrelevant test
 - remote-testgit: get rid of non-local functionality
 - Add new simplified git-remote-testgit
 - Rename git-remote-testgit to git-remote-testpy
 - remote-testgit: fix direction of marks
 - fast-export: avoid importing blob marks

 Improvements to fix fast-export bugs, including how refs pointing to
 already-seen commits are handled. An earlier 4-commit version of this
 series looked good to me, but this much-expanded version has not seen
 any comments.

 Looks like it has been re-rolled, but I haven't checked it out yet.

 Needs review.


* mh/alt-odb-string-list-cleanup (2012-11-08) 2 commits
  (merged to 'next' on 2012-11-13 at 2bf41d9)
 + link_alt_odb_entries(): take (char *, len) rather than two pointers
 + link_alt_odb_entries(): use string_list_split_in_place()

 Cleanups in the alternates code. Fixes a potential bug and makes the
 code much cleaner.

 Will merge to 'master' in the sixth batch.


* pw/maint-p4-rcs-expansion-newline (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at e90cc7c)
 + git p4: RCS expansion should not span newlines

 I do not have p4 to play with, but looks obviously correct to me.

 Will merge to 'master' in the sixth batch.


* rh/maint-gitweb-highlight-ext (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at c57d856)
 + gitweb.perl: fix %highlight_ext mappings

 Fixes a clever misuse of perl's list interpretation.

 Will merge to 'master' in the sixth batch.


* rr/submodule-diff-config (2012-11-18) 4 commits
  (merged to 'next' on 2012-11-19 at 355319e)
 + submodule: display summary header in bold
 + diff: rename "set" variable
 + diff: introduce diff.submodule configuration variable
 + Documentation: move diff.wordRegex from config.txt to diff-config.txt

 Lets "git diff --submodule=log" become the default via configuration.

 Will merge to 'master' in the seventh batch.

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Junio C Hamano @ 2012-11-19 23:57 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Krzysztof Mazur, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <CAMP44s0f0zYa1FVf9RhNuwYJbkQ7zPwgJ6=ty3c5knjo5a2TNw@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
>
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -924,6 +924,10 @@ sub quote_subject {
>>  # use the simplest quoting being able to handle the recipient
>>  sub sanitize_address {
>>         my ($recipient) = @_;
>> +
>> +       # remove garbage after email address
>> +       $recipient =~ s/(.*>).*$/$1/;
>> +
>
> Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test
> <foo@bar.com> <#comment>' gets the second comment removed.

Yeah, but do you need to capture the second group?  IOW, like
"s/(.*?>).*$/$1/" perhaps?

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Junio C Hamano @ 2012-11-20  0:00 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <20121119225838.GA23412@shrek.podlesie.net>

Krzysztof Mazur <krzysiek@podlesie.net> writes:

> On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
>> Given that the problematic line
>> 
>> 	Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y
>> 
>> is not even a valid e-mail address, doesn't this new logic belong to
>> sanitize_address() conceptually?
>
> Yes, it's much better to do it in the sanitize_address().

Note that I did not check that all the addresses that are handled by
extract-valid-address came through sanitize-address function, so
unlike your original patch, this change alone may still pass some
garbage to Email::Valid->address().  I tend to think that is a
progress; we should make sure all the addresses are sanitized before
using them for sending messages out.

^ permalink raw reply

* Re: git config --git-all can return non-zero error code
From: Junio C Hamano @ 2012-11-20  0:10 UTC (permalink / raw)
  To: Timur Tabi; +Cc: git
In-Reply-To: <50AABA9B.6090007@freescale.com>

Timur Tabi <timur@freescale.com> writes:

> According to the man page for git-config, the --git-all option is not
> supposed to return an error code:
>
> --get-all
> 	Like get, but does not fail if the number of values for the key is
> 	not exactly one.
>
> IMHO, zero is also "not exactly one",...

It should have stated "like get, but unlike get, it does not fail
when there are multiple values for the key", but the documentation
was written with that specific knowledge that "--get" has a logic to
make it fail when there are ambiguities, and wanted to stress that
difference.  It forgot to mention their similarity explicitly and
relied on "Like get" part to mean (1) shows the value(s) given to
the key, and (2) it is an error if there is no such key defined.

^ permalink raw reply

* [PATCH] config --get-all: it is an error not to see any value
From: Junio C Hamano @ 2012-11-20  0:24 UTC (permalink / raw)
  To: git; +Cc: Timur Tabi
In-Reply-To: <7vd2z9uobn.fsf@alter.siamese.dyndns.org>

The wording "... is not exactly one." incorrectly hinted as if
having no value is not an error, but that was not what was
intended.  Clarify what similarity it wants to mention by
elaborating the "Like get" part of the description.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-config.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git i/Documentation/git-config.txt w/Documentation/git-config.txt
index eaea079..273239d 100644
--- i/Documentation/git-config.txt
+++ w/Documentation/git-config.txt
@@ -85,8 +85,9 @@ OPTIONS
 	found and error code 2 if multiple key values were found.
 
 --get-all::
-	Like get, but does not fail if the number of values for the key
-	is not exactly one.
+	Like `--get`, shows the value(s) given to the key, and signals an
+	error if there is no such key.  Unlike `--get`, does not
+	fail if there are multiple values defined for the key.
 
 --get-regexp::
 	Like --get-all, but interprets the name as a regular expression and

^ permalink raw reply related

* Re: [PATCH 2/2] pickaxe: use textconv for -S counting
From: Junio C Hamano @ 2012-11-20  0:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Peter Oberndorfer, git
In-Reply-To: <20121115012131.GA17894@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Nov 13, 2012 at 03:13:19PM -0800, Junio C Hamano wrote:
>
>> >  static int has_changes(struct diff_filepair *p, struct diff_options *o,
>> >  		       regex_t *regexp, kwset_t kws)
>> >  {
>> > +	struct userdiff_driver *textconv_one = get_textconv(p->one);
>> > +	struct userdiff_driver *textconv_two = get_textconv(p->two);
>> > +	mmfile_t mf1, mf2;
>> > +	int ret;
>> > +
>> >  	if (!o->pickaxe[0])
>> >  		return 0;
>> >  
>> > -	if (!DIFF_FILE_VALID(p->one)) {
>> > -		if (!DIFF_FILE_VALID(p->two))
>> > -			return 0; /* ignore unmerged */
>> 
>> What happened to this part that avoids showing nonsense for unmerged
>> paths?
>
> It's moved down. fill_one will return an empty mmfile if
> !DIFF_FILE_VALID, so we end up here:
>
>         fill_one(p->one, &mf1, &textconv_one);
>         fill_one(p->two, &mf2, &textconv_two);
>
>         if (!mf1.ptr) {
>                 if (!mf2.ptr)
>                         ret = 0; /* ignore unmerged */
>
> Prior to this change, we didn't use fill_one, so we had to check manually.
>
>> > +	/*
>> > +	 * If we have an unmodified pair, we know that the count will be the
>> > +	 * same and don't even have to load the blobs. Unless textconv is in
>> > +	 * play, _and_ we are using two different textconv filters (e.g.,
>> > +	 * because a pair is an exact rename with different textconv attributes
>> > +	 * for each side, which might generate different content).
>> > +	 */
>> > +	if (textconv_one == textconv_two && diff_unmodified_pair(p))
>> > +		return 0;
>> 
>> I am not sure about this part that cares about the textconv.
>> 
>> Wouldn't the normal "git diff A B" skip the filepair that are
>> unmodified in the first place at the object name level without even
>> looking at the contents (see e.g. diff_flush_patch())?
>
> Hmph. The point was to find the case when the paths are different (e.g.,
> in a rename), and therefore the textconvs might be different. But I
> think I missed the fact that diff_unmodified_pair will note the
> difference in paths. So just calling diff_unmodified_pair would be
> sufficient, as the code prior to my patch does.
>
> I thought the point was an optimization to avoid comparing contains() on
> the same data (which we can know will match without looking at it).

Yes.

> Exact renames are the obvious one, but they are not handled here.

That is half true.  Before this change, we will find the same number
of needles and this function would have said "no differences" in a
very inefficient way.  After this change, we may apply different
textconv filters and this function will say "there is a difference",
even though we wouldn't see such a difference at the content level
if there wasn't any rename.

^ permalink raw reply

* Re: [PATCH 2/2] pickaxe: use textconv for -S counting
From: Junio C Hamano @ 2012-11-20  0:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Peter Oberndorfer, git
In-Reply-To: <7v3905uncf.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> Exact renames are the obvious one, but they are not handled here.
>
> That is half true.  Before this change, we will find the same number
> of needles and this function would have said "no differences" in a
> very inefficient way.  After this change, we may apply different
> textconv filters and this function will say "there is a difference",
> even though we wouldn't see such a difference at the content level
> if there wasn't any rename.

... but I think that is a good thing anyway.

If you renamed foo.c to foo.cc with different conversions from C
code to the text that explain what the code does, if we special case
only the exact rename case but let pickaxe examine the converted
result in a case where blobs are modified only by one byte, we would
get drastically different results between the two cases.

Corollary to this is what should happen when you update the attributes
between two trees so that textconv for a path that did not change
between preimage and postimage are different.  Ideally, we should
notice that the two converted result are different, perhaps, but I
do not like the performance implications very much.

^ permalink raw reply

* Re: [PATCH v4 2/4] diff: introduce diff.submodule configuration variable
From: Junio C Hamano @ 2012-11-20  0:48 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1352821367-3611-3-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index 6c01d0c..e401814 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -33,6 +33,7 @@ test_create_repo sm1 &&
>  add_file . foo >/dev/null
>  
>  head1=$(add_file sm1 foo1 foo2)
> +fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)

That looks like quite a roundabout way to say

	$(cd sm1; git rev-parse --verify HEAD)

doesn't it?  I know this is just moved code from the original, so I
am not saying this should be fixed in this commit.

Existing code in t7401 may want to be cleaned up, perhaps after this
topic settles.  The add_file() function, for example, has at least
these points:

 - do we know 7 hexdigits is always the right precision?
 - what happens when it fails to create a commit in one of the steps
   while looping over "$@" in its loop?
 - the function uses the "cd there and then come back", not
   "go there in a subshell and do whatever it needs to" pattern.

> +test_expect_success 'added submodule, set diff.submodule' "

s/added/add/;

Shouldn't it test the base case where no diff.submodule is set?  For
those people, the patch should not change the output when they do or
do not pass --submodule option, right?

> +	git config diff.submodule log &&
> +	git add sm1 &&
> +	git diff --cached >actual &&
> +	cat >expected <<-EOF &&
> +Submodule sm1 0000000...$head1 (new submodule)
> +EOF
> +	git config --unset diff.submodule &&
> +	test_cmp expected actual
> +"
> +
> +test_expect_success '--submodule=short overrides diff.submodule' "
> +	git config diff.submodule log &&
> +	git add sm1 &&
> +	git diff --submodule=short --cached >actual &&
> +	cat >expected <<-EOF &&
> +diff --git a/sm1 b/sm1
> +new file mode 160000
> +index 0000000..a2c4dab
> +--- /dev/null
> ++++ b/sm1
> +@@ -0,0 +1 @@
> ++Subproject commit $fullhead1
> +EOF
> +	git config --unset diff.submodule &&
> +	test_cmp expected actual
> +"
> +
>  commit_file sm1 &&
>  head2=$(add_file sm1 foo3)
>  
> @@ -73,7 +102,6 @@ EOF
>  	test_cmp expected actual
>  "
>  
> -fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
>  fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
>  test_expect_success 'modified submodule(forward) --submodule=short' "
>  	git diff --submodule=short >actual &&

^ permalink raw reply

* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Junio C Hamano @ 2012-11-20  0:49 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <20121117192026.GI22234@odin.tremily.us>

"W. Trevor King" <wking@tremily.us> writes:

>> From what I have heard of projects using this: They usually still have
>> something that records the SHA1s on a regular basis. Thinking further,
>> why not record them in git? We could add an option to update which
>> creates such a commit.
>
> I think it's best to have users craft their own commit messages
> explaining why the branch was updated.  That said, an auto-generated
> hint (a la "git merge") would probably be a useful extra feature.

I am not quite sure I agree.  When the project says "Use the tip of
'bar' branch for the submodule 'foo'" at the top-level, does an
individual user who is not working on the submodule 'foo' but merely
is using it have any clue as to why the submodule's 'foo' branch
'foo' moved, or does he necessarily even care?

For such a user working at the top-level superproject, or working on
one part of the project, possibly on a submodule other than 'foo',
wouldn't the natural thing to do would be to run "git pull" at the
top-level, maybe with "--recursive" to update the top-level and all
the submodules to start the day.

Now, since somebody created the top-level commit you have just
pulled and checked out, other people may have worked on submodule
'foo' [*1*].  What should happen on "git submodule update foo"?  It
would notice that the submodule 'foo' is set to float, and would
check out the tip of the branch 'bar', not the commit recorded in
the top-level superproject, in the working tree for 'foo', no?

What should appear in "git diff"?  The working tree taken as a whole
is different from what the superproject's commit describes (which is
the state the person who created the superproject wanted to record)
even though this user does not have anything to do with the change
at 'foo' from the recorded commit to the current tip of 'bar'.  What
would his description for the reason why the branch was updated?

I think I would agree that "git diff" should not hide such changes
(after all, when this user records his change to the overall project
in the top-level supermodule, he will be recording the state with
the commit at the tip of 'bar' checked out in the working tree of
the submodule 'foo'), but I am not sure if the user can say anything
sensible, other than "tip of 'bar' branch in submodule 'foo' was
changed by others", in the resulting commit.


[Footnote]

*1* This may look like a non-issue if you assume that the person who
updates the 'bar' branch of submodule 'foo' always updates the
gitlink in the superproject's commit to point at that updated
commit, but that assumption is flawed; the submodule project is a
project on its own and can be worked on without what other projects
bind it as their submodules.

^ permalink raw reply

* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-20  1:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <7vd2z9t7y2.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]

On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> 
> >> From what I have heard of projects using this: They usually still have
> >> something that records the SHA1s on a regular basis. Thinking further,
> >> why not record them in git? We could add an option to update which
> >> creates such a commit.
> >
> > I think it's best to have users craft their own commit messages
> > explaining why the branch was updated.  That said, an auto-generated
> > hint (a la "git merge") would probably be a useful extra feature.
> 
> I am not quite sure I agree.  When the project says "Use the tip of
> 'bar' branch for the submodule 'foo'" at the top-level, does an
> individual user who is not working on the submodule 'foo' but merely
> is using it have any clue as to why the submodule's 'foo' branch
> 'foo' moved, or does he necessarily even care?

If he doesn't care, why is he updating the submodule gitlink?

> Now, since somebody created the top-level commit you have just
> pulled and checked out, other people may have worked on submodule
> 'foo' [*1*].  What should happen on "git submodule update foo"?

If the 'foo' checkout is not the one listed in the superproject's
.gitmodules, the update should bail with an appropriate error message,
and let the user sort things out.

  $ git submodule update --pull foo
  error: Your local changes to the following submodule would be
  overwritten by update:…

This is similar to how Git currently bails on dirty-tree branch
switches:

  $ git checkout my-branch
  error: Your local changes to the following files would be
  overwritten by checkout:…

Without "--pull", the update command is intended to checkout the hash
specified in .gitmodules.  If you've committed some local work in foo
and then explicitly ask for an update, I suppose you get clobbered.

> What should appear in "git diff"?  The working tree taken as a whole
> is different from what the superproject's commit describes (which is
> the state the person who created the superproject wanted to record)
> even though this user does not have anything to do with the change
> at 'foo' from the recorded commit to the current tip of 'bar'.  What
> would his description for the reason why the branch was updated?

The submodule content is not part of the superproject.  All the
superproject has is a gitlink.  If the gitlink hasn't changed, "git
diff" in the superproject shouldn't say anything.

I'll probably have time to write up v4 over the weekend.  Maybe having
a more explicit example will clear things up.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Felipe Contreras @ 2012-11-20  1:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Sverre Rabbelier
In-Reply-To: <7vpq39up0m.fsf@alter.siamese.dyndns.org>

On Tue, Nov 20, 2012 at 12:55 AM, Junio C Hamano <gitster@pobox.com> wrote:

> * fc/remote-testgit-feature-done (2012-10-29) 1 commit
>  - remote-testgit: properly check for errors

Pinging Sverre again.

I now think that we need a better solution with waitpid() to check the
status of the process, so that both import and export get proper error
checks. I found that out the hard way with a buggy remote helper.

I still think this patch is good regardless, but not so big of a
priority. I'm not going to purse it more, if it gets in, good, if not,
a waitpid() patch would take care of it.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v4.1 5/5] push: update remote tags only with force
From: Chris Rorvick @ 2012-11-20  4:43 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <7va9udxryf.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 2:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Rorvick <chris@rorvick.com> writes:
>>  The object referenced by <src> is used to update the <dst> reference
>> -on the remote side, but by default this is only allowed if the
>> -update can fast-forward <dst>.  By having the optional leading `+`,
>> -you can tell git to update the <dst> ref even when the update is not a
>> -fast-forward.  This does *not* attempt to merge <src> into <dst>.  See
>> -EXAMPLES below for details.
>> +on the remote side.  By default this is only allowed if the update is
>> +a branch, and then only if it can fast-forward <dst>.  By having the
>
> I can already see the next person asking "I can update refs/notes
> the same way.  The doc says it applies only to the branches.  Is
> this a bug?".

Sure, will fix.

>> diff --git a/cache.h b/cache.h
>> index f410d94..127e504 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -1004,13 +1004,14 @@ struct ref {
>>               requires_force:1,
>>               merge:1,
>>               nonfastforward:1,
>> -             is_a_tag:1,
>> +             forwardable:1,
>
> This is somewhat an unfortunate churn.  Perhaps is_a_tag could have
> started its life under its final name in the series?
>
> I am assuming that the struct members will be initialized to 0 (false),
> so everything by default is now not forwardable if somebody forgets
> to set this flag?

Yeah, this was one of a few stupid mistakes.  Previously I used
'forwardable' throughout, but that is awkward except in the last
commit since until then everything is allowed to fast-forward and the
flag is only used to output tag-specific advice.  But inverting the
meaning of the flag is dumb, and I didn't even do it right.

But, as I think you're suggesting, it probably makes more sense to use
a flag that prevents fast-forwarding when set.  So maybe
"not_forwardable", or "is_a_tag" => "not_forwardable" if you think the
renaming is a good idea.  Or maybe just "is_a_tag"?  I think the
rename is good because its meaning is more general in the last commit.

>> diff --git a/remote.c b/remote.c
>> index 44e72d6..a723f7a 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -1311,14 +1311,24 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>>                *     to overwrite it; you would not know what you are losing
>>                *     otherwise.
>>                *
>> -              * (3) if both new and old are commit-ish, and new is a
>> -              *     descendant of old, it is OK.
>> +              * (3) if new is commit-ish and old is a commit, new is a
>> +              *     descendant of old, and the reference is not of the
>> +              *     refs/tags/ hierarchy it is OK.
>>                *
>>                * (4) regardless of all of the above, removing :B is
>>                *     always allowed.
>>                */
>
> I think this is unreadable.  Isn't this more like
>
>     (1.5) if the destination is inside refs/tags/ and already
>           exists, you are not allowed to overwrite it without
>           --force or +A:B notation.
>
> early in the rule set?

Yes, something like that is much better.

>> -             ref->is_a_tag = !prefixcmp(ref->name, "refs/tags/");
>> +             if (prefixcmp(ref->name, "refs/tags/")) {
>> +                     /* Additionally, disallow fast-forwarding if
>> +                      * the old object is not a commit.  This kicks
>> +                      * out annotated tags that might pass the
>> +                      * is_newer() test but dangle if the reference
>> +                      * is updated.
>> +                      */
>
> Huh?  prefixcmp() excludes refs/tags/ hierarchy. What is an
> annotated tag doing there?  Is this comment outdated???

I think the comment is accurate, although inverting the meaning of the
flag probably doesn't help the clarity of this change.

What do you mean by "there"?  Annotated tags normally are referenced
via something under refs/tags/, but they could be elsewhere, right?
So what I meant to say was: if the reference is not under refs/tags/
then the starting point has to be a commit for it to be forwardable.

I interpreted parse_object(ref->old_sha1) == NULL as the old thing not
existing (rule 1) but I'm pretty sure that was a mistake.  I should
first test it with is_null_sha1, if true then it doesn't exist.
Otherwise parse_object() returning NULL means we just don't have the
object and therefore should follow rule 2.

> Regarding the other arm of this "if (not in refs/tags/ hierarchy)",
> what happens when refs/tags/foo is a reference to a blob or a tree?
> This series declares that the point of tag is not to let people to
> move it willy-nilly, and I think it is in line with its spirit if
> you just rejected non-creation events.

Nothing under refs/tags/ is updateable and only commits are updateable
outside of that due to the new logic.  The ref_newer() call will
reject updating to blobs and trees, although that probably isn't the
right thing to do.  Previously I was ensuring both old and new objects
were commits if the ref was not under refs/tags/, but it occurred to
me that fast-forwarding from a commit to a tag might be a reasonable
thing to want to do.  But a rejected update to a blob should get the
already-exists advice, not a message suggesting a merge (which is what
you get by relying on ref_newer() to fail.)

> Also, I suspect that you do not even need to have old object locally
> when looking at refs/tags/ hierarchy.  "Do not overwrite tags" can
> be enforced by only noticing that they already have something; you
> do not know what that something actually is to prevent yourself from
> overwriting it.  You may have to update the rule (2) in remote.c
> around l.1311 for this.

Isn't it already doing this?  I think I've either confounded things
with my mistakes or I'm missing this point.

>> +test_expect_success 'push requires --force to update lightweight tag' '
>> +     mk_test heads/master &&
>> +     mk_child child1 &&
>> +     mk_child child2 &&
>> +     (
>> +             cd child1 &&
>> +             git tag Tag &&
>> +             git push ../child2 Tag &&
>
> Don't you want to make sure that another "git push ../child2 Tag" at
> this point, i.e. attempt to update Tag with the same, should succeed
> without --force?

Yes, that is clearly a good addition.

Thanks for all the feedback.  Sorry I mucked up this series a bit,
tried to get an update out too quickly I guess.

Chris

^ permalink raw reply

* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Junio C Hamano @ 2012-11-20  5:39 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <20121120011628.GD321@odin.tremily.us>

"W. Trevor King" <wking@tremily.us> writes:

> On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> ...
>> > I think it's best to have users craft their own commit messages
>> > explaining why the branch was updated.  That said, an auto-generated
>> > hint (a la "git merge") would probably be a useful extra feature.
>> 
>> I am not quite sure I agree.  When the project says "Use the tip of
>> 'bar' branch for the submodule 'foo'" at the top-level, does an
>> individual user who is not working on the submodule 'foo' but merely
>> is using it have any clue as to why the submodule's 'foo' branch
>> 'foo' moved, or does he necessarily even care?
>
> If he doesn't care, why is he updating the submodule gitlink?

He may not be updating the gitlink with "git add foo" at the
top-level superproject level.  He is just using that submodule as
part of the larger whole as he is working on either the top-level or
some other submodule.  And checkout of 'foo' is necessary in the
working tree for him to work in the larger context of the project,
and 'foo' is set to float at the tip of its 'bar' branch.  And that
checkout results in a commit that is different from the commit the
gitlink suggests, perhaps because somebody worked in 'foo' submodule
and advanced the tip of branch 'bar'.

So:

 - at the top-level superproject level, entry 'foo' in the HEAD tree
   points at an older commit;

 - 'foo/.git/HEAD' points at refs/heads/bar, which matches the
   working tree of 'foo' and the index foo/.git/index..

I am not sure what should happen to the entry 'foo' in the index of
the top-level superproject after such a 'submodule floats at the
tip' checkout, but I imagine that it must match the contents of
foo/.git/HEAD's tree.  Otherwise, "git diff" at the top-level would
report local changes.

When committing his work at the top-level, he will see that 'foo'
gitlink is updated in that commit; after all that combination is the
context in which his work was done.

Or are you envisioning that such a check-out will and should show a
local difference at the submodule 'foo' by leaving the index of the
top-level superproject unchanged, and the user should refrain from
using "git commit -a" to avoid having to describe the changes made
on the 'bar' branch in the meantime in his top-level commit?  That
is certainly fine by me (I am no a heavy submodule user to begin
with), but I am not sure if that is useful and helpful to the
submodule users.

^ permalink raw reply

* Re: [PATCH v4.1 5/5] push: update remote tags only with force
From: Junio C Hamano @ 2012-11-20  5:44 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <CAEUsAPa9fiF9cuMqRHNsQSz_CsbbvdO-eGDS9HWW3MrAYZPA8w@mail.gmail.com>

Chris Rorvick <chris@rorvick.com> writes:

> On Mon, Nov 19, 2012 at 2:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Yeah, this was one of a few stupid mistakes.  Previously I used
> 'forwardable' throughout, but that is awkward except in the last
> commit since until then everything is allowed to fast-forward and the
> flag is only used to output tag-specific advice.  But inverting the
> meaning of the flag is dumb, and I didn't even do it right.
>
> But, as I think you're suggesting, it probably makes more sense to use
> a flag that prevents fast-forwarding when set.  So maybe
> "not_forwardable", or "is_a_tag" => "not_forwardable" if you think the
> renaming is a good idea.

Yeah, calling it not-forwardable from the beginning would be a
sensible approach, I would think.

Thanks.

^ permalink raw reply

* [PATCH 14/13] test-wildmatch: avoid Windows path mangling
From: Johannes Sixt @ 2012-11-20  7:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <7vvcdco1pf.fsf@alter.siamese.dyndns.org>

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The MSYS bash mangles arguments that begin with a forward slash
when they are passed to test-wildmatch. This causes tests to fail.
Avoid mangling by prepending "XXX", which is removed by
test-wildmatch before further processing.

[J6t: reworded commit message]

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Works well, and I'm fine with this work-around.

 -- Hannes

 t/t3070-wildmatch.sh | 10 +++++-----
 test-wildmatch.c     |  8 ++++++++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index e6ad6f4..3155eab 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -74,7 +74,7 @@ match 0 0 'foo/bar' 'foo[/]bar'
 match 0 0 'foo/bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
 match 1 1 'foo-bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
 match 1 0 'foo' '**/foo'
-match 1 x '/foo' '**/foo'
+match 1 x 'XXX/foo' '**/foo'
 match 1 0 'bar/baz/foo' '**/foo'
 match 0 0 'bar/baz/foo' '*/foo'
 match 0 0 'foo/bar/baz' '**/bar*'
@@ -95,8 +95,8 @@ match 0 0 ']' '[!]-]'
 match 1 x 'a' '[!]-]'
 match 0 0 '' '\'
 match 0 x '\' '\'
-match 0 x '/\' '*/\'
-match 1 x '/\' '*/\\'
+match 0 x 'XXX/\' '*/\'
+match 1 x 'XXX/\' '*/\\'
 match 1 1 'foo' 'foo'
 match 1 1 '@foo' '@foo'
 match 0 0 'foo' '@foo'
@@ -187,8 +187,8 @@ match 0 0 '-' '[[-\]]'
 match 1 1 '-adobe-courier-bold-o-normal--12-120-75-75-m-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
 match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-X-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
 match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-/-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
-match 1 1 '/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' '/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
-match 0 0 '/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' '/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
+match 1 1 'XXX/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' 'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
+match 0 0 'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
 match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
 match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
 diff --git a/test-wildmatch.c b/test-wildmatch.c
index 74c0864..e384c8e 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -3,6 +3,14 @@
  int main(int argc, char **argv)
 {
+	int i;
+	for (i = 2; i < argc; i++) {
+		if (argv[i][0] == '/')
+			die("Forward slash is not allowed at the beginning of the\n"
+			    "pattern because Windows does not like it. Use `XXX/' instead.");
+		else if (!strncmp(argv[i], "XXX/", 4))
+			argv[i] += 3;
+	}
 	if (!strcmp(argv[1], "wildmatch"))
 		return !!wildmatch(argv[3], argv[2], 0);
 	else if (!strcmp(argv[1], "iwildmatch"))

^ permalink raw reply related

* Re: [PATCH 15/13] compat/fnmatch: fix off-by-one character class's length check
From: Johannes Sixt @ 2012-11-20  7:06 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1352628837-5784-2-git-send-email-pclouds@gmail.com>

Am 11/11/2012 11:13, schrieb Nguyễn Thái Ngọc Duy:
> Character class "xdigit" is the only one that hits 6 character limit
> defined by CHAR_CLASS_MAX_LENGTH. All other character classes are 5
> character long and therefore never caught by this.
> 
> This should make xdigit tests in t3070 pass on Windows.
> 
> Reported-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  I tested with Linux (removing the ifdef __LIBC in fnmatch.c) but I
>  think this should get an ACK from someone who actually uses it on
>  Windows.

Works well here on Windows.

This does not affect Windows alone, but all platforms that fall back to
compat/fnmatch. It's perhaps worth its own topic branch.

>  We may want to tell upstream (who?) about this if they haven't fixed
>  it already.
> 
>  compat/fnmatch/fnmatch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
> index 9473aed..0ff1d27 100644
> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -345,7 +345,7 @@ internal_fnmatch (pattern, string, no_leading_period, flags)
>  
>  		    for (;;)
>  		      {
> -			if (c1 == CHAR_CLASS_MAX_LENGTH)
> +			if (c1 > CHAR_CLASS_MAX_LENGTH)
>  			  /* The name is too long and therefore the pattern
>  			     is ill-formed.  */
>  			  return FNM_NOMATCH;
> 

-- Hannes

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-20  7:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <7vhaoluos6.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 04:00:09PM -0800, Junio C Hamano wrote:
> Krzysztof Mazur <krzysiek@podlesie.net> writes:
> 
> > On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
> >> Given that the problematic line
> >> 
> >> 	Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y
> >> 
> >> is not even a valid e-mail address, doesn't this new logic belong to
> >> sanitize_address() conceptually?
> >
> > Yes, it's much better to do it in the sanitize_address().
> 
> Note that I did not check that all the addresses that are handled by
> extract-valid-address came through sanitize-address function, so

Before sending that patch, I checked that and tested with and without
Email::Valid.

> unlike your original patch, this change alone may still pass some
> garbage to Email::Valid->address().  I tend to think that is a
> progress; we should make sure all the addresses are sanitized before
> using them for sending messages out.

I will try to check that.

Krzysiek

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-20  7:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <7vlidxuowf.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> >
> >> --- a/git-send-email.perl
> >> +++ b/git-send-email.perl
> >> @@ -924,6 +924,10 @@ sub quote_subject {
> >>  # use the simplest quoting being able to handle the recipient
> >>  sub sanitize_address {
> >>         my ($recipient) = @_;
> >> +
> >> +       # remove garbage after email address
> >> +       $recipient =~ s/(.*>).*$/$1/;
> >> +
> >
> > Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test
> > <foo@bar.com> <#comment>' gets the second comment removed.
> 
> Yeah, but do you need to capture the second group?  IOW, like
> "s/(.*?>).*$/$1/" perhaps?

I also thought about removing everything after first ">", but I will
not work for addresses like:

Cc: "foo >" <stable@vger.kernel.org> #v3.4 v3.5 v3.6

What about:

	$recipient =~ s/(.*<[^@]*@[^]]*>).*$/$1/;

or even

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..b988c57 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -925,8 +925,11 @@ sub quote_subject {
 sub sanitize_address {
 	my ($recipient) = @_;
 
+	my $local_part_regexp = qr/[^<>"\s@]+/;
+	my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
+
 	# remove garbage after email address
-	$recipient =~ s/(.*>).*$/$1/;
+	$recipient =~ s/(.*<$local_part_regexp\@$domain_regexp>).*$/$1/;
 
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);

which uses regex used by 99% accurate version of extract_valid_address().

Krzysiek

^ permalink raw reply related

* git-svn: What is --follow-parent / --no-follow-parent for?
From: Sebastian Leske @ 2012-11-20  7:31 UTC (permalink / raw)
  To: git

Hi,

on reading the docs of "git-svn", I stumbled across this paragraph:

> --follow-parent
> This is especially helpful when we’re tracking a directory that has been
> moved around within the repository, or if we started tracking a branch
> and never tracked the trunk it was descended from.  This feature is
> enabled by default, use --no-follow-parent to disable it.

However, this does not make sense to me: This sounds like there is no
good reason *not* to enable this option.  So why is it there? And in
what situation might I want to use "--no-follow-parent"?

As a matter of fact, I'm not even sure what "--no-follow-parent" does
(and the docs don't really say). 

I tried it out with a small test repo with a single branch (produced by
copying the trunk, then later deleted). With --follow-parent git-svn
correctly detected the branch point, and modeled the branch deletion as
a merge. With --no-follow-parent it just acted as if branch and trunk
were completely unrelated.

Commit graph of git-svn result:

--follow-parent:               --no-follow-parent:


       |                               |
      /|                             | |
     / |                             | |
     | |                             | |
     | |                             | |
     | |                             | |
     \ |                             | |
      \|                             | |
       |                               | 


(please excuse cheap ASCII art)

Is that the only effect of --no-follow-parent? And again, why would I
want that?

I'd be grateful for any clarifications. If I manage to understand the
explanation, I'll volunteer to summarize it into doc patch (if there are
no objections).

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-20  7:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <20121120073100.GB7206@shrek.podlesie.net>

On Tue, Nov 20, 2012 at 08:31:00AM +0100, Krzysztof Mazur wrote:
> On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> > 
> > > On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> > >
> > >> --- a/git-send-email.perl
> > >> +++ b/git-send-email.perl
> > >> @@ -924,6 +924,10 @@ sub quote_subject {
> > >>  # use the simplest quoting being able to handle the recipient
> > >>  sub sanitize_address {
> > >>         my ($recipient) = @_;
> > >> +
> > >> +       # remove garbage after email address
> > >> +       $recipient =~ s/(.*>).*$/$1/;
> > >> +
> > >
> > > Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test
> > > <foo@bar.com> <#comment>' gets the second comment removed.
> > 
> > Yeah, but do you need to capture the second group?  IOW, like
> > "s/(.*?>).*$/$1/" perhaps?
> 
> I also thought about removing everything after first ">", but I will
> not work for addresses like:
> 
> Cc: "foo >" <stable@vger.kernel.org> #v3.4 v3.5 v3.6
> 
> What about:
> 
> 	$recipient =~ s/(.*<[^@]*@[^]]*>).*$/$1/;
> 
> or even
> 
> which uses regex used by 99% accurate version of extract_valid_address().
> 

Of course, as you suggested earier, only the first email address should
be used, so in both cases the first ".*" should be changed to ".*?".
The second version becomes:

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..dbe520c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -925,8 +925,11 @@ sub quote_subject {
 sub sanitize_address {
 	my ($recipient) = @_;
 
+	my $local_part_regexp = qr/[^<>"\s@]+/;
+	my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
+
 	# remove garbage after email address
-	$recipient =~ s/(.*>).*$/$1/;
+	$recipient =~ s/^(.*?<$local_part_regexp\@$domain_regexp>).*/$1/;
 
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
 
Krzysiek

^ permalink raw reply related


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