* Re: [RFC/PATCH] ls-files: adding support for submodules
From: Junio C Hamano @ 2016-09-09 23:01 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <1473458004-41460-1-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Allow ls-files to recognize submodules in order to retrieve a list of
> files from a repository's submodules. This is done by forking off a
> process to recursively call ls-files on all submodules.
While I see why "ls-files --recurse-submodules" sounds nice ("hey, I
can get list of _all_ the files here"), and I am quite happy with
the quality of implementation (not just the code but its
documentation and test) especially from a first-time contributor, I
am not quite sure what the utility of this new feature would be,
especially given that the command is a plumbing, i.e. meant to be a
useful building block for scripts.
If I get
$ git ls-files --recurse-submodules
Makefile
lib/Makefile
lib/hello.py
main.py
goodbye.py
out of the command, what can I do with it without knowing where the
submodule boundaries are? It's not like I can just do
git ls-files --recurse-submodule |
while read path
do
git update-index --add "$path"
done
when "lib/" is a submodule. Instead, I'd need to go to "lib/" and
then add "Makefile" and "hello.py" from there.
> diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
> new file mode 100644
> index 0000000..78deded
> --- /dev/null
> +++ b/t/t3007-ls-files-recurse-submodules.sh
> @@ -0,0 +1,103 @@
> +...
> +test_expect_success 'setup directory structure and submodules' '
> +...
> +'
> +
> +cat >expect <<EOF
> +.gitmodules
> +a
> +b/b
> +submodule/c
> +EOF
We used to do that when we didn't know better.
Please don't do things outside test_expect_* block, especially in a
new script.
^ permalink raw reply
* git clone http:// fails some times with "Request for d53.. aborted"
From: Yaroslav Halchenko @ 2016-09-09 22:19 UTC (permalink / raw)
To: git
even when (v 2.7.0) ran on the box where the server is, so
unlikely to be network issue
or from my laptop (v 2.9.3) with ok but wifi with a weakish signal to the
access point:
$> ( set -e; for s in {1..100}; do rm -rf fbirn_phaseIII ; git clone http://datasets.datalad.org/nidm/fbirn_phaseIII/.git; done; )
Cloning into 'fbirn_phaseIII'...
Checking connectivity... done.
Cloning into 'fbirn_phaseIII'...
error: Request for d53302dfc7ad13b786923927021039d21a10d5bd aborted
error: Unable to find d53302dfc7ad13b786923927021039d21a10d5bd under http://datasets.datalad.org/nidm/fbirn_phaseIII/.git
Cannot obtain needed tree d53302dfc7ad13b786923927021039d21a10d5bd
while processing commit 22dd4c49417cad6f4082ac2aebef45da8b6e473d.
error: fetch failed.
even if I build fresh 2.10.0, discovered that we can trace now CURL calls
$> export PATH=$HOME/proj/misc/git:$PATH
$> git version
git version 2.10.0
$> ( set -e; for s in {1..100}; do rm -rf fbirn_phaseIII ; GIT_TRACE_CURL=/tmp/git-trace-curl-failed.log git clone http://datasets.datalad.org/nidm/fbirn_phaseIII/.git && mv /tmp/git-trace-curl-failed.log /tmp/git-trace-curl-good.log || exit 1; done; )
Cloning into 'fbirn_phaseIII'...
warning: templates not found /home/yoh/share/git-core/templates
... a few more ...
Cloning into 'fbirn_phaseIII'...
warning: templates not found /home/yoh/share/git-core/templates
error: Request for d53302dfc7ad13b786923927021039d21a10d5bd aborted
error: Unable to find d53302dfc7ad13b786923927021039d21a10d5bd under http://datasets.datalad.org/nidm/fbirn_phaseIII/.git
Cannot obtain needed tree d53302dfc7ad13b786923927021039d21a10d5bd
while processing commit 22dd4c49417cad6f4082ac2aebef45da8b6e473d.
error: fetch failed.
( set -e; for s in {1..100}; do; rm -rf fbirn_phaseIII; GIT_TRACE_CURL= git ) 18.70s user 8.01s system 34% cpu 1:16.84 total
NB
- note that it is d53302dfc7ad13b786923927021039d21a10d5bd all the time!
- that one corresponds to a tree
$> git show d53302dfc7ad13b786923927021039d21a10d5bd
tree d53302dfc7ad13b786923927021039d21a10d5bd
1ec/
da9/
- I do not see that object being reported as failed to be found on the server:
$> grep d5/3302dfc7ad13b786923927021039d21a10d5bd /var/log/apache2/datasets.datalad.org-error.log
$> grep d5/3302dfc7ad13b786923927021039d21a10d5bd /var/log/apache2/datasets.datalad.org-access.log
129.170.31.34 - - [09/Sep/2016:14:14:25 -0400] "GET /nidm/fbirn_phaseIII/.git/objects/d5/3302dfc7ad13b786923927021039d21a10d5bd HTTP/1.1" 200 72
...
- and the logs say that only "good" run had it requested:
$> zgrep 3302dfc7ad13b786923927021039d21a10d5bd git-trace-curl-*.log.gz
git-trace-curl-good.log.gz:18:09:13.749268 http.c:517 => Send header: GET /nidm/fbirn_phaseIII/.git/objects/d5/3302dfc7ad13b786923927021039d21a10d5bd HTTP/1.1
and here are those two GIT_TRACE_CURL traces from good and bad runs -- may be
someone with a better clue could figure it out or any other information I
should provide to help pin point why that "request for d5... aborted" ;)
http://www.onerussian.com/tmp/git-trace-curl-good.log.gz
http://www.onerussian.com/tmp/git-trace-curl-failed.log.gz
Thanks in advance and please CC me in replies
--
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
^ permalink raw reply
* What's cooking in git.git (Sep 2016, #03; Fri, 9)
From: Junio C Hamano @ 2016-09-09 22:47 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
I've started merging topics that have been cooking in 'next' to
'master', and the next step will be to rewind and rebuild 'next',
and merge those that have been waiting in 'pu' to 'next'. There are
a few more topics in flight that may be ready to be picked up but I
haven't, and other topics in flight that may not be quite ready.
They will be picked up after topics that have already been in-tree
starts quieting down.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* bh/diff-highlight-graph (2016-08-31) 6 commits
(merged to 'next' on 2016-08-31 at 523a15f)
+ diff-highlight: avoid highlighting combined diffs
+ diff-highlight: add multi-byte tests
+ diff-highlight: ignore test cruft
+ diff-highlight: add support for --graph output
+ diff-highlight: add failing test for handling --graph output
+ diff-highlight: add some tests
"diff-highlight" script (in contrib/) learned to work better with
"git log -p --graph" output.
* cc/receive-pack-limit (2016-08-24) 3 commits
(merged to 'next' on 2016-08-25 at bc74b5b)
+ receive-pack: allow a maximum input size to be specified
+ unpack-objects: add --max-input-size=<size> option
+ index-pack: add --max-input-size=<size> option
An incoming "git push" that attempts to push too many bytes can now
be rejected by setting a new configuration variable at the receiving
end.
* hv/doc-commit-reference-style (2016-08-26) 1 commit
(merged to 'next' on 2016-08-31 at 68fb778)
+ SubmittingPatches: use gitk's "Copy commit summary" format
A small doc update.
* jh/status-v2-porcelain (2016-08-12) 9 commits
(merged to 'next' on 2016-08-31 at e71f595)
+ status: unit tests for --porcelain=v2
+ test-lib-functions.sh: add lf_to_nul helper
+ git-status.txt: describe --porcelain=v2 format
+ status: print branch info with --porcelain=v2 --branch
+ status: print per-file porcelain v2 status data
+ status: collect per-file data for --porcelain=v2
+ status: support --porcelain[=<version>]
+ status: cleanup API to wt_status_print
+ status: rename long-format print routines
Enhance "git status --porcelain" output by collecting more data on
the state of the index and the working tree files, which may
further be used to teach git-prompt (in contrib/) to make fewer
calls to git.
* jk/delta-base-cache (2016-08-23) 7 commits
(merged to 'next' on 2016-08-25 at f1c141a)
+ t/perf: add basic perf tests for delta base cache
+ delta_base_cache: use hashmap.h
+ delta_base_cache: drop special treatment of blobs
+ delta_base_cache: use list.h for LRU
+ release_delta_base_cache: reuse existing detach function
+ clear_delta_base_cache_entry: use a more descriptive name
+ cache_or_unpack_entry: drop keep_cache parameter
The delta-base-cache mechanism has been a key to the performance in
a repository with a tightly packed packfile, but it did not scale
well even with a larger value of core.deltaBaseCacheLimit.
* jk/format-patch-number-singleton-patch-with-cover (2016-08-23) 1 commit
(merged to 'next' on 2016-08-25 at a4737fb)
+ format-patch: show 0/1 and 1/1 for singleton patch with cover letter
"git format-patch --cover-letter HEAD^" to format a single patch
with a separate cover letter now numbers the output as [PATCH 0/1]
and [PATCH 1/1] by default.
* po/range-doc (2016-08-13) 12 commits
(merged to 'next' on 2016-08-31 at d29870b)
+ doc: revisions: sort examples and fix alignment of the unchanged
+ doc: revisions: show revision expansion in examples
+ doc: revisions - clarify reachability examples
+ doc: revisions - define `reachable`
+ doc: gitrevisions - clarify 'latter case' is revision walk
+ doc: gitrevisions - use 'reachable' in page description
+ doc: revisions: single vs multi-parent notation comparison
+ doc: revisions: extra clarification of <rev>^! notation effects
+ doc: revisions: give headings for the two and three dot notations
+ doc: show the actual left, right, and boundary marks
+ doc: revisions - name the left and right sides
+ doc: use 'symmetric difference' consistently
Clarify various ways to specify the "revision ranges" in the
documentation.
* rt/help-unknown (2016-08-30) 3 commits
(merged to 'next' on 2016-08-30 at db2a5b0)
+ help: make option --help open man pages only for Git commands
+ help: introduce option --exclude-guides
+ Merge branch 'js/no-html-bypass-on-windows' into rt/help-unknown
"git nosuchcommand --help" said "No manual entry for gitnosuchcommand",
which was not intuitive, given that "git nosuchcommand" said "git:
'nosuchcommand' is not a git command".
* sb/submodule-clone-rr (2016-08-17) 8 commits
(merged to 'next' on 2016-08-31 at 08b4b7d)
+ clone: recursive and reference option triggers submodule alternates
+ clone: implement optional references
+ clone: clarify option_reference as required
+ clone: factor out checking for an alternate path
+ submodule--helper update-clone: allow multiple references
+ submodule--helper module-clone: allow multiple references
+ t7408: merge short tests, factor out testing method
+ t7408: modernize style
"git clone --resurse-submodules --reference $path $URL" is a way to
reduce network transfer cost by borrowing objects in an existing
$path repository when cloning the superproject from $URL; it
learned to also peek into $path for presense of corresponding
repositories of submodules and borrow objects from there when able.
--------------------------------------------------
[New Topics]
* jk/patch-ids-no-merges (2016-09-09) 2 commits
- patch-ids: define patch-id of merge commits as "null"
- patch-ids: turn off rename detection
"git log --cherry-pick" used to include merge commits as candidates
to be matched up with other commits, resulting a lot of wasted time.
The patch-id generation logic has been updated to ignore merges to
avoid the wastage.
Will merge to 'next'.
* js/git-gui-commit-gpgsign (2016-09-09) 2 commits
- Merge branch 'js/commit-gpgsign' of ../git-gui into js/git-gui-commit-gpgsign
- git-gui: respect commit.gpgsign again
"git commit-tree" stopped reading commit.gpgsign configuration
variable that was meant for Porcelain "git commit" in Git 2.9; we
forgot to update "git gui" to look at the configuration to match
this change.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* jc/blame-reverse (2016-06-14) 2 commits
- blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
- blame: improve diagnosis for "--reverse NEW"
It is a common mistake to say "git blame --reverse OLD path",
expecting that the command line is dwimmed as if asking how lines
in path in an old revision OLD have survived up to the current
commit.
Has been waiting for positive responses without seeing any.
Will discard.
* jc/attr (2016-05-25) 18 commits
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_attr()
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
(this branch is used by jc/attr-more, sb/pathspec-label and sb/submodule-default-paths.)
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
I wanted to polish this topic further to make the attribute
subsystem thread-ready, but because other topics depend on this
topic and they do not (yet) need it to be thread-ready.
As the authors of topics that depend on this seem not in a hurry,
let's discard this and dependent topics and restart them some other
day.
Will discard.
* jc/attr-more (2016-06-09) 8 commits
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- fixup! d5ad6c13
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
(this branch uses jc/attr; is tangled with sb/pathspec-label and sb/submodule-default-paths.)
The beginning of long and tortuous journey to clean-up attribute
subsystem implementation.
Needs to be redone.
Will discard.
* sb/submodule-default-paths (2016-06-20) 5 commits
- completion: clone can recurse into submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- Merge branch 'sb/pathspec-label' into sb/submodule-default-paths
- Merge branch 'jc/attr' into sb/submodule-default-paths
(this branch uses jc/attr and sb/pathspec-label; is tangled with jc/attr-more.)
Allow specifying the set of submodules the user is interested in on
the command line of "git clone" that clones the superproject.
Will discard.
* sb/pathspec-label (2016-06-03) 6 commits
- pathspec: disable preload-index when attribute pathspec magic is in use
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
(this branch is used by sb/submodule-default-paths; uses jc/attr; is tangled with jc/attr-more.)
The pathspec mechanism learned ":(attr:X)$pattern" pathspec magic
to limit paths that match $pattern further by attribute settings.
The preload-index mechanism is disabled when the new pathspec magic
is in use (at least for now), because the attribute subsystem is
not thread-ready.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* sb/bisect (2016-04-15) 22 commits
. SQUASH???
. bisect: get back halfway shortcut
. bisect: compute best bisection in compute_relevant_weights()
. bisect: use a bottom-up traversal to find relevant weights
. bisect: prepare for different algorithms based on find_all
. bisect: rename count_distance() to compute_weight()
. bisect: make total number of commits global
. bisect: introduce distance_direction()
. bisect: extract get_distance() function from code duplication
. bisect: use commit instead of commit list as arguments when appropriate
. bisect: replace clear_distance() by unique markers
. bisect: use struct node_data array instead of int array
. bisect: get rid of recursion in count_distance()
. bisect: make algorithm behavior independent of DEBUG_BISECT
. bisect: make bisect compile if DEBUG_BISECT is set
. bisect: plug the biggest memory leak
. bisect: add test for the bisect algorithm
. t6030: generalize test to not rely on current implementation
. t: use test_cmp_rev() where appropriate
. t/test-lib-functions.sh: generalize test_cmp_rev
. bisect: allow 'bisect run' if no good commit is known
. bisect: write about `bisect next` in documentation
The internal algorithm used in "git bisect" to find the next commit
to check has been optimized greatly.
Was expecting a reroll, but now pb/bisect topic starts removinging
more and more parts from git-bisect.sh, this needs to see a fresh
reroll.
Will discard.
cf. <1460294354-7031-1-git-send-email-s-beyer@gmx.net>
* sg/completion-updates (2016-02-28) 21 commits
. completion: cache the path to the repository
. completion: extract repository discovery from __gitdir()
. completion: don't guard git executions with __gitdir()
. completion: consolidate silencing errors from git commands
. completion: don't use __gitdir() for git commands
. completion: respect 'git -C <path>'
. completion: fix completion after 'git -C <path>'
. completion: don't offer commands when 'git --opt' needs an argument
. rev-parse: add '--absolute-git-dir' option
. completion: list short refs from a remote given as a URL
. completion: don't list 'HEAD' when trying refs completion outside of a repo
. completion: list refs from remote when remote's name matches a directory
. completion: respect 'git --git-dir=<path>' when listing remote refs
. completion: fix most spots not respecting 'git --git-dir=<path>'
. completion: ensure that the repository path given on the command line exists
. completion tests: add tests for the __git_refs() helper function
. completion tests: check __gitdir()'s output in the error cases
. completion tests: consolidate getting path of current working directory
. completion tests: make the $cur variable local to the test helper functions
. completion tests: don't add test cruft to the test repository
. completion: improve __git_refs()'s in-code documentation
Has been waiting for a reroll for too long.
cf. <1456754714-25237-1-git-send-email-szeder@ira.uka.de>
Will discard.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* bc/object-id (2016-09-07) 20 commits
- builtin/reset: convert to use struct object_id
- builtin/commit-tree: convert to struct object_id
- builtin/am: convert to struct object_id
- refs: add an update_ref_oid function.
- sha1_name: convert get_sha1_mb to struct object_id
- builtin/update-index: convert file to struct object_id
- notes: convert init_notes to use struct object_id
- builtin/rm: convert to use struct object_id
- builtin/blame: convert file to use struct object_id
- Convert read_mmblob to take struct object_id.
- notes-merge: convert struct notes_merge_pair to struct object_id
- builtin/checkout: convert some static functions to struct object_id
- streaming: make stream_blob_to_fd take struct object_id
- builtin: convert textconv_object to use struct object_id
- builtin/cat-file: convert some static functions to struct object_id
- builtin/cat-file: convert struct expand_data to use struct object_id
- builtin/log: convert some static functions to use struct object_id
- builtin/blame: convert struct origin to use struct object_id
- builtin/apply: convert static functions to struct object_id
- cache: convert struct cache_entry to use struct object_id
The "unsigned char sha1[20]" to "struct object_id" conversion
continues. Notable changes in this round includes that ce->sha1,
i.e. the object name recorded in the cache_entry, turns into an
object_id.
It had merge conflicts with a few topics in flight (Christian's
"apply.c split", Dscho's "cat-file --filters" and Jeff Hostetler's
"status --porcelain-v2"). Extra sets of eyes double-checking for
mismerges are highly appreciated.
* ep/use-git-trace-curl-in-tests (2016-09-07) 4 commits
(merged to 'next' on 2016-09-08 at 04372de)
+ t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var
+ t5550-http-fetch-dumb.sh: use the GIT_TRACE_CURL environment var
+ test-lib.sh: preserve GIT_TRACE_CURL from the environment
+ t5541-http-push-smart.sh: use the GIT_TRACE_CURL environment var
Update a few tests that used to use GIT_CURL_VERBOSE to use the
newer GIT_TRACE_CURL.
Will merge to 'master'.
* jk/pack-tag-of-tag (2016-09-07) 5 commits
- pack-objects: walk tag chains for --include-tag
- t5305: simplify packname handling
- t5305: use "git -C"
- t5305: drop "dry-run" of unpack-objects
- t5305: move cleanup into test block
"git pack-objects --include-tag" was taught that when we know that
we are sending an object C, we want a tag B that directly points at
C but also a tag A that points at the tag B. We used to miss the
intermediate tag B in some cases.
Will merge to 'next'.
* js/t6026-clean-up (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 5ad2fc1)
+ t6026-merge-attr: clean up background process at end of test case
A test spawned a short-lived background process, which sometimes
prevented the test directory from getting removed at the end of the
script on some platforms.
Will merge to 'master'.
* js/t9903-chaining (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 162a3c9)
+ t9903: fix broken && chain
Will merge to 'master'.
* jt/accept-capability-advertisement-when-fetching-from-void (2016-09-09) 3 commits
- connect: advertized capability is not a ref
- connect: tighten check for unexpected early hang up
- tests: move test_lazy_prereq JGIT to test-lib.sh
JGit can show a fake ref "capabilities^{}" to "git fetch" when it
does not advertise any refs, but "git fetch" was not prepared to
see such an advertisement. When the other side disconnects without
giving any ref advertisement, we used to say "there may not be a
repository at that URL", but we may have seen other advertisement
like "shallow" and ".have" in which case we definitely know that a
repository is there. The code to detect this case has also been
updated.
Will merge to 'next'.
* rs/compat-strdup (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 46acfdf)
+ compat: move strdup(3) replacement to its own file
Will merge to 'master'.
* rs/hex2chr (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 7266d5b)
+ introduce hex2chr() for converting two hexadecimal digits to a character
Will merge to 'master'.
* rt/rebase-i-broken-insn-advise (2016-09-07) 1 commit
- rebase -i: improve advice on bad instruction lines
When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").
Will hold.
Dscho's "rebase -i" hopefully will become available in 'pu', by
which time an equivalent of this fix would be ported to C. This is
queued merely as a reminder.
* sb/xdiff-remove-unused-static-decl (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 39e41dd)
+ xdiff: remove unneeded declarations
Code cleanup.
Will merge to 'master'.
* sy/git-gui-i18n-ja (2016-09-07) 7 commits
- Merge branch 'sy/i18n' of git-gui
- git-gui: update Japanese information
- git-gui: update Japanese translation
- git-gui: add Japanese language code
- git-gui: apply po template to Japanese translation
- git-gui: consistently use the same word for "blame" in Japanese
- git-gui: consistently use the same word for "remote" in Japanese
Update Japanese translation for "git-gui".
Will merge to 'next'.
* ah/misc-message-fixes (2016-09-08) 5 commits
- unpack-trees: do not capitalize "working"
- git-merge-octopus: do not capitalize "octopus"
- git-rebase--interactive: fix English grammar
- cat-file: put spaces around pipes in usage string
- am: put spaces around pipe in usage string
Message cleanup.
Will merge to 'next'.
* jk/fix-remote-curl-url-wo-proto (2016-09-08) 1 commit
- remote-curl: handle URLs without protocol
"git fetch http::/site/path" did not die correctly and segfaulted
instead.
Will merge to 'next'.
* jt/format-patch-base-info-above-sig (2016-09-09) 1 commit
- format-patch: show base info before email signature
"git format-patch --base=..." feature that was recently added
showed the base commit information after "-- " e-mail signature
line, which turned out to be inconvenient. The base information
has been moved above the signature line.
Needs an extra blank line before the baseinfo.
* nd/checkout-disambiguation (2016-09-09) 4 commits
- fixup! checkout.txt: document a common case that ignores ambiguation rules
- checkout: fix ambiguity check in subdir
- checkout.txt: document a common case that ignores ambiguation rules
- checkout: add some spaces between code and comment
"git checkout <word>" does not follow the usual disambiguation
rules when the <word> can be both a rev and a path, to allow
checking out a branch 'foo' in a project that happens to have a
file 'foo' in the working tree without having to disambiguate.
This was poorly documented and the check was incorrect when the
command was run from a subdirectory.
Waiting for an Ack for fixup!
* sb/diff-cleanup (2016-09-08) 3 commits
- diff: remove dead code
- diff: omit found pointer from emit_callback
- diff.c: use diff_options directly
Code cleanup.
Will merge to 'next'.
* sb/transport-report-missing-submodule-on-stderr (2016-09-08) 1 commit
(merged to 'next' on 2016-09-08 at 3550831)
+ transport: report missing submodule pushes consistently on stderr
Message cleanup.
Will merge to 'master'.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* va/i18n (2016-09-09) 10 commits
- i18n: update-index: mark warning for translation
- i18n: show-branch: mark error messages for translation
- i18n: receive-pack: mark messages for translation
- notes: downcase the first word of error messages
- i18n: notes: mark error messages for translation
- i18n: merge-recursive: mark verbose message for translation
- i18n: merge-recursive: mark error messages for translation
- i18n: config: mark error message for translation
- i18n: branch: mark option description for translation
- i18n: blame: mark error messages for translation
More i18n.
Will merge to 'next'.
* jc/submodule-anchor-git-dir (2016-09-01) 1 commit
(merged to 'next' on 2016-09-08 at b6f20cf)
+ submodule: avoid auto-discovery in prepare_submodule_repo_env()
Having a submodule whose ".git" repository is somehow corrupt
caused a few commands that recurse into submodules loop forever.
Will merge to 'master'.
* jc/forbid-symbolic-ref-d-HEAD (2016-09-02) 1 commit
(merged to 'next' on 2016-09-08 at cd8c1b3)
+ symbolic-ref -d: do not allow removal of HEAD
"git symbolic-ref -d HEAD" happily removes the symbolic ref, but
the resulting repository becomes an invalid one. Teach the command
to forbid removal of HEAD.
Will merge to 'master'.
* jc/am-read-author-file (2016-08-30) 1 commit
(merged to 'next' on 2016-09-08 at d2db42f)
+ am: refactor read_author_script()
Extract a small helper out of the function that reads the authors
script file "git am" internally uses.
Will merge to 'master'.
This by itself is not useful until a second caller appears in the
future for "rebase -i" helper.
* jk/test-lib-drop-pid-from-results (2016-08-30) 1 commit
(merged to 'next' on 2016-09-08 at 0967b0b)
+ test-lib: drop PID from test-results/*.count
The test framework left the number of tests and success/failure
count in the t/test-results directory, keyed by the name of the
test script plus the process ID. The latter however turned out not
to serve any useful purpose. The process ID part of the filename
has been removed.
Will merge to 'master'.
* js/sequencer-wo-die (2016-09-09) 17 commits
- sequencer: ensure to release the lock when we could not read the index
- sequencer: lib'ify checkout_fast_forward()
- sequencer: lib'ify fast_forward_to()
- sequencer: lib'ify save_opts()
- sequencer: lib'ify save_todo()
- sequencer: lib'ify save_head()
- sequencer: lib'ify create_seq_dir()
- sequencer: lib'ify read_populate_opts()
- sequencer: lib'ify read_populate_todo()
- sequencer: lib'ify read_and_refresh_cache()
- sequencer: lib'ify prepare_revs()
- sequencer: lib'ify walk_revs_populate_todo()
- sequencer: lib'ify do_pick_commit()
- sequencer: lib'ify do_recursive_merge()
- sequencer: lib'ify write_message()
- sequencer: do not die() in do_pick_commit()
- sequencer: lib'ify sequencer_pick_revisions()
Lifts calls to exit(2) and die() higher in the callchain in
sequencer.c files so that more helper functions in it can be used
by callers that want to handle error conditions themselves.
Will merge to 'next' after waiting for a few days for further comments.
* jk/squelch-false-warning-from-gcc-o3 (2016-08-31) 2 commits
(merged to 'next' on 2016-09-08 at c9a2af6)
+ color_parse_mem: initialize "struct color" temporary
+ error_errno: use constant return similar to error()
Will merge to 'master'.
* cp/completion-negative-refs (2016-08-24) 1 commit
- completion: support excluding refs
The command-line completion script (in contrib/) learned to
complete "git cmd ^mas<HT>" to complete the negative end of
reference to "git cmd ^master".
Waiting for a review.
* js/cat-file-filters (2016-09-09) 5 commits
- SQUASH??? "cat-file --filters" passes data as-is by default
- cat-file: support --textconv/--filters in batch mode
- cat-file --textconv/--filters: allow specifying the path separately
- cat-file: introduce the --filters option
- cat-file: fix a grammo in the man page
Even though "git hash-objects", which is a tool to take an
on-filesystem data stream and put it into the Git object store,
allowed to perform the "outside-world-to-Git" conversions (e.g.
end-of-line conversions and application of the clean-filter), and
it had the feature on by default from very early days, its reverse
operation "git cat-file", which takes an object from the Git object
store and externalize for the consumption by the outside world,
lacked an equivalent mechanism to run the "Git-to-outside-world"
conversion. The command learned the "--filters" option to do so.
Waiting for an ack to SQUASH???
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Alas, this reveals that the "check" mode is too inefficient to use
in real projects, even in ones as small as git itself.
cf. <xmqqh9aaot49.fsf@gitster.mtv.corp.google.com>
* ak/curl-imap-send-explicit-scheme (2016-08-17) 1 commit
- imap-send: Tell cURL to use imap:// or imaps://
When we started cURL to talk to imap server when a new enough
version of cURL library is available, we forgot to explicitly add
imap(s):// before the destination. To some folks, that didn't work
and the library tried to make HTTP(s) requests instead.
Needs review and testing.
* jk/diff-submodule-diff-inline (2016-08-31) 8 commits
(merged to 'next' on 2016-09-02 at 734e42c)
+ diff: teach diff to display submodule difference with an inline diff
+ submodule: refactor show_submodule_summary with helper function
+ submodule: convert show_submodule_summary to use struct object_id *
+ allow do_submodule_path to work even if submodule isn't checked out
+ diff: prepare for additional submodule formats
+ graph: add support for --line-prefix on all graph-aware output
+ diff.c: remove output_prefix_length field
+ cache: add empty_tree_oid object and helper function
The "git diff --submodule={short,log}" mechanism has been enhanced
to allow "--submodule=diff" to show the patch between the submodule
commits bound to the superproject.
Will merge to 'master'.
* jk/reduce-gc-aggressive-depth (2016-08-11) 1 commit
(merged to 'next' on 2016-08-11 at 6810c6f)
+ gc: default aggressive depth to 50
"git gc --aggressive" used to limit the delta-chain length to 250,
which is way too deep for gaining additional space savings and is
detrimental for runtime performance. The limit has been reduced to
50.
Will hold to see if people scream.
* ks/pack-objects-bitmap (2016-08-09) 2 commits
- pack-objects: use reachability bitmap index when generating non-stdout pack
- pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
What is the state of this thing?
Waiting for the review discussion to settle.
cf. <20160818175222.bmm3ivjheokf2qzl@sigill.intra.peff.net>
cf. <20160818180615.q25p57v35m2xxtww@sigill.intra.peff.net>
* mh/diff-indent-heuristic (2016-09-07) 9 commits
- SQAUSH???
- blame: honor the diff heuristic options and config
- parse-options: add parse_opt_unknown_cb()
- diff: improve positioning of add/delete blocks in diffs
- xdl_change_compact(): introduce the concept of a change group
- recs_match(): take two xrecord_t pointers as arguments
- is_blank_line(): take a single xrecord_t as argument
- xdl_change_compact(): only use heuristic if group can't be matched
- xdl_change_compact(): fix compaction heuristic to adjust ixo
Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
Needs adjustment of test numbers. Otherwise looked OK.
Waiting for an ack to SQUASH???
* cc/apply-am (2016-09-07) 41 commits
- builtin/am: use apply API in run_apply()
- apply: learn to use a different index file
- apply: pass apply state to build_fake_ancestor()
- apply: refactor `git apply` option parsing
- apply: change error_routine when silent
- usage: add get_error_routine() and get_warn_routine()
- usage: add set_warn_routine()
- apply: don't print on stdout in verbosity_silent mode
- apply: make it possible to silently apply
- apply: use error_errno() where possible
- apply: make some parsing functions static again
- apply: move libified code from builtin/apply.c to apply.{c,h}
- apply: rename and move opt constants to apply.h
- builtin/apply: rename option parsing functions
- builtin/apply: make create_one_file() return -1 on error
- builtin/apply: make try_create_file() return -1 on error
- builtin/apply: make write_out_results() return -1 on error
- builtin/apply: make write_out_one_result() return -1 on error
- builtin/apply: make create_file() return -1 on error
- builtin/apply: make add_index_file() return -1 on error
- builtin/apply: make add_conflicted_stages_file() return -1 on error
- builtin/apply: make remove_file() return -1 on error
- builtin/apply: make build_fake_ancestor() return -1 on error
- builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
- builtin/apply: make gitdiff_*() return -1 on error
- builtin/apply: make gitdiff_*() return 1 at end of header
- builtin/apply: make parse_traditional_patch() return -1 on error
- builtin/apply: make apply_all_patches() return 128 or 1 on error
- builtin/apply: move check_apply_state() to apply.c
- builtin/apply: make check_apply_state() return -1 instead of die()ing
- apply: make init_apply_state() return -1 instead of exit()ing
- builtin/apply: move init_apply_state() to apply.c
- builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing
- builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
- builtin/apply: make parse_single_patch() return -1 on error
- builtin/apply: make parse_chunk() return a negative integer on error
- builtin/apply: make find_header() return -128 instead of die()ing
- builtin/apply: read_patch_file() return -1 instead of die()ing
- builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
- apply: move 'struct apply_state' to apply.h
- apply: make some names more specific
"git am" has been taught to make an internal call to "git apply"'s
innards without spawning the latter as a separate process.
Will merge to 'next' after waiting for a few days for further comments.
* jk/pack-objects-optim-mru (2016-08-11) 4 commits
(merged to 'next' on 2016-08-11 at c0a7dae)
+ pack-objects: use mru list when iterating over packs
+ pack-objects: break delta cycles before delta-search phase
+ sha1_file: make packed_object_info public
+ provide an initializer for "struct object_info"
"git pack-objects" in a repository with many packfiles used to
spend a lot of time looking for/at objects in them; the accesses to
the packfiles are now optimized by checking the most-recently-used
packfile first.
Will hold to see if people scream.
* jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
(merged to 'next' on 2016-08-14 at 6891bcd)
+ rebase-interactive: drop early check for valid ident
Even when "git pull --rebase=preserve" (and the underlying "git
rebase --preserve") can complete without creating any new commit
(i.e. fast-forwards), it still insisted on having a usable ident
information (read: user.email is set correctly), which was less
than nice. As the underlying commands used inside "git rebase"
would fail with a more meaningful error message and advice text
when the bogus ident matters, this extra check was removed.
Will hold to see if people scream.
cf. <20160729224944.GA23242@sigill.intra.peff.net>
* dp/autoconf-curl-ssl (2016-06-28) 1 commit
- ./configure.ac: detect SSL in libcurl using curl-config
The ./configure script generated from configure.ac was taught how
to detect support of SSL by libcurl better.
Needs review.
* jc/pull-rebase-ff (2016-07-28) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
(merged to 'next' on 2016-07-13 at d9ca7fb)
+ pathspec: warn on empty strings as pathspec
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
Will hold to see if people scream.
* mh/ref-store (2016-09-09) 38 commits
- refs: implement iteration over only per-worktree refs
- refs: make lock generic
- refs: add method to rename refs
- refs: add methods to init refs db
- refs: make delete_refs() virtual
- refs: add method for initial ref transaction commit
- refs: add methods for reflog
- refs: add method iterator_begin
- files_ref_iterator_begin(): take a ref_store argument
- split_symref_update(): add a files_ref_store argument
- lock_ref_sha1_basic(): add a files_ref_store argument
- lock_ref_for_update(): add a files_ref_store argument
- commit_ref_update(): add a files_ref_store argument
- lock_raw_ref(): add a files_ref_store argument
- repack_without_refs(): add a files_ref_store argument
- refs: make peel_ref() virtual
- refs: make create_symref() virtual
- refs: make pack_refs() virtual
- refs: make verify_refname_available() virtual
- refs: make read_raw_ref() virtual
- resolve_gitlink_ref(): rename path parameter to submodule
- resolve_gitlink_ref(): avoid memory allocation in many cases
- resolve_gitlink_ref(): implement using resolve_ref_recursively()
- resolve_ref_recursively(): new function
- read_raw_ref(): take a (struct ref_store *) argument
- resolve_gitlink_packed_ref(): remove function
- resolve_packed_ref(): rename function from resolve_missing_loose_ref()
- refs: reorder definitions
- refs: add a transaction_commit() method
- {lock,commit,rollback}_packed_refs(): add files_ref_store arguments
- resolve_missing_loose_ref(): add a files_ref_store argument
- get_packed_ref(): add a files_ref_store argument
- add_packed_ref(): add a files_ref_store argument
- refs: create a base class "ref_store" for files_ref_store
- refs: add a backend method structure
- refs: rename struct ref_cache to files_ref_store
- rename_ref_available(): add docstring
- resolve_gitlink_ref(): eliminate temporary variable
The ref-store abstraction was introduced to the refs API so that we
can plug in different backends to store references.
Will merge to 'next' after waiting for a few days for further comments.
* nd/shallow-deepen (2016-06-13) 27 commits
- fetch, upload-pack: --deepen=N extends shallow boundary by N commits
- upload-pack: add get_reachable_list()
- upload-pack: split check_unreachable() in two, prep for get_reachable_list()
- t5500, t5539: tests for shallow depth excluding a ref
- clone: define shallow clone boundary with --shallow-exclude
- fetch: define shallow boundary with --shallow-exclude
- upload-pack: support define shallow boundary by excluding revisions
- refs: add expand_ref()
- t5500, t5539: tests for shallow depth since a specific date
- clone: define shallow clone boundary based on time with --shallow-since
- fetch: define shallow boundary with --shallow-since
- upload-pack: add deepen-since to cut shallow repos based on time
- shallow.c: implement a generic shallow boundary finder based on rev-list
- fetch-pack: use a separate flag for fetch in deepening mode
- fetch-pack.c: mark strings for translating
- fetch-pack: use a common function for verbose printing
- fetch-pack: use skip_prefix() instead of starts_with()
- upload-pack: move rev-list code out of check_non_tip()
- upload-pack: make check_non_tip() clean things up on error
- upload-pack: tighten number parsing at "deepen" lines
- upload-pack: use skip_prefix() instead of starts_with()
- upload-pack: move "unshallow" sending code out of deepen()
- upload-pack: remove unused variable "backup"
- upload-pack: move "shallow" sending code out of deepen()
- upload-pack: move shallow deepen code out of receive_needs()
- transport-helper.c: refactor set_helper_option()
- remote-curl.c: convert fetch_git() to use argv_array
The existing "git fetch --depth=<n>" option was hard to use
correctly when making the history of an existing shallow clone
deeper. A new option, "--deepen=<n>", has been added to make this
easier to use. "git clone" also learned "--shallow-since=<date>"
and "--shallow-exclude=<tag>" options to make it easier to specify
"I am interested only in the recent N months worth of history" and
"Give me only the history since that version".
Needs review.
Rerolled. What this topic attempts to achieve is worthwhile, I
would think.
* pb/bisect (2016-08-23) 27 commits
. bisect--helper: remove the dequote in bisect_start()
. bisect--helper: retire `--bisect-auto-next` subcommand
. bisect--helper: retire `--bisect-autostart` subcommand
. bisect--helper: retire `--check-and-set-terms` subcommand
. bisect--helper: retire `--bisect-write` subcommand
. bisect--helper: `bisect_replay` shell function in C
. bisect--helper: `bisect_log` shell function in C
. bisect--helper: retire `--write-terms` subcommand
. bisect--helper: retire `--check-expected-revs` subcommand
. bisect--helper: `bisect_state` & `bisect_head` shell function in C
. bisect--helper: `bisect_autostart` shell function in C
. bisect--helper: retire `--next-all` subcommand
. bisect--helper: retire `--bisect-clean-state` subcommand
. bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
. bisect--helper: `bisect_start` shell function partially in C
. bisect--helper: `get_terms` & `bisect_terms` shell function in C
. bisect--helper: `bisect_next_check` & bisect_voc shell function in C
. bisect--helper: `check_and_set_terms` shell function in C
. bisect--helper: `bisect_write` shell function in C
. bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
. bisect--helper: `bisect_reset` shell function in C
. wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
. t6030: explicitly test for bisection cleanup
. bisect--helper: `bisect_clean_state` shell function in C
. bisect--helper: `write_terms` shell function in C
. bisect: rewrite `check_term_format` shell function in C
. bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
GSoC "bisect" topic.
I'd prefer to see early part solidified so that reviews can focus
on the later part that is still in flux. We are almost there but
not quite yet.
* kn/ref-filter-branch-list (2016-05-17) 17 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled.
Needs review.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
- merge: drop 'git merge <message> HEAD <commit>' syntax
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
It has been reported that git-gui still uses the deprecated syntax,
which needs to be fixed before this final step can proceed.
cf. <5671DB28.8020901@kdbg.org>
--------------------------------------------------
[Discarded]
* jn/fix-connect-unexpected-hangup-diag (2016-09-08) 1 commit
. connect: tighten check for unexpected early hang up
Now part of jt/accept-capability-advertisement-when-fetching-from-void
topic.
^ permalink raw reply
* Re: [RFC/PATCH] ls-files: adding support for submodules
From: Junio C Hamano @ 2016-09-09 22:40 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git
In-Reply-To: <20160909223513.3rirneqxmrcyi4k4@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I suppose another option would be to teach ls-files a "prefix" option
> to add to each filename, and just pass in the submodule path. Then you
> can let the sub-processes write directly to the common stdout, and I
> think it would be safe to blindly pass the parent argv into the child
> processes.
I think that is a sensible way to do this, instead of reading from -z
and showing things depending on various output modes you were told
to use.
^ permalink raw reply
* Re: [RFC/PATCH] ls-files: adding support for submodules
From: Jeff King @ 2016-09-09 22:35 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <1473458004-41460-1-git-send-email-bmwill@google.com>
On Fri, Sep 09, 2016 at 02:53:24PM -0700, Brandon Williams wrote:
> Allow ls-files to recognize submodules in order to retrieve a list of
> files from a repository's submodules. This is done by forking off a
> process to recursively call ls-files on all submodules.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> Hey git developers!
>
> I'm new to the community and this is the first patch for an open source project
> that I have worked on.
>
> I'm looking forward to working on the project!
Welcome. :)
Submodules are not really my area of expertise, so I don't have any
commentary on the goal of the patch, except that it sounds reasonable to
my layman's ears.
The implementation looks fairly clean. A few comments:
> +static void show_gitlink(const struct cache_entry *ce)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf buf = STRBUF_INIT;
> + struct strbuf name = STRBUF_INIT;
> + int submodule_name_len;
> + FILE *fp;
> +
> + argv_array_push(&cp.args, "ls-files");
> + argv_array_push(&cp.args, "--recurse-submodules");
> + cp.git_cmd = 1;
> + cp.dir = ce->name;
> + cp.out = -1;
> + start_command(&cp);
> + fp = fdopen(cp.out, "r");
You should error-check the result of start_command(). I guess the
reasonable outcome would be to die(), as it is a sign that we could not
fork, find git, etc.
Ditto for fdopen (you can use xfdopen for that).
> + /*
> + * The ls-files child process produces filenames relative to
> + * the submodule. Prefix each line with the submodule path
> + * to make it relative to the current repository.
> + */
> + strbuf_addstr(&name, ce->name);
> + strbuf_addch(&name, '/');
> + submodule_name_len = name.len;
> + while (strbuf_getline(&buf, fp) != EOF) {
> + strbuf_addbuf(&name, &buf);
> + write_name(name.buf);
> + strbuf_setlen(&name, submodule_name_len);
> + }
What happens if the filename in the submodule needs quoting? You'll get
the quoted value in your buffer, and then re-quote it again in
write_name().
The simplest thing would probably be to use "ls-files -z" for the
recursive invocation, and then split on NUL bytes (we have
strbuf_getline_nul for that).
> + finish_command(&cp);
What should happen if finish_command() tells us that the ls-files
sub-process reported an error? It may not be worth aborting the rest of
the listing, but we might want to propagate that in our own return code.
> + strbuf_release(&buf);
> + strbuf_release(&name);
> + fclose(fp);
> +}
A minor style nit, but I would generally fclose(fp) before running
finish_command() (i.e., resource clean up in the reverse order of
allocation). It doesn't matter in this case because "fp" is output from
the process, and we know we've already read to EOF. For other cases, it
could cause a deadlock (e.g., we end up in wait() for the child process
to finish, but it is blocked in write() waiting for us to read). So I
think it's a good habit to get into.
> @@ -519,6 +566,17 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
> if (require_work_tree && !is_inside_work_tree())
> setup_work_tree();
>
> + if (recurse_submodules &&
> + (show_stage || show_deleted || show_others || show_unmerged ||
> + show_killed || show_modified || show_resolve_undo ||
> + show_valid_bit || show_tag || show_eol))
> + die("ls-files --recurse-submodules can only be used in "
> + "--cached mode");
Woah, that list of variables is getting rather long. This is not a
problem introduced by your patch, so it's not a blocker. But I wonder if
some of them are mutually exclusive and could be collapsed to a single
variable.
I guess the reason for this "only with --cached" is that you do not
propagate the options down to the recursive process. If we were to do
that, then this big list of restrictions would go away. I'd be OK with
starting with more limited functionality like your patch, though. I
think doing the recursive thing correctly would also involve parsing the
output of each to append the filename prefix.
So I suppose another option would be to teach ls-files a "prefix" option
to add to each filename, and just pass in the submodule path. Then you
can let the sub-processes write directly to the common stdout, and I
think it would be safe to blindly pass the parent argv into the child
processes.
-Peff
^ permalink raw reply
* Re: How to simulate a real checkout to test a new smudge filter?
From: Junio C Hamano @ 2016-09-09 22:34 UTC (permalink / raw)
To: john smith; +Cc: Jakub Narębski, Torsten Bögershausen, git
In-Reply-To: <CAKmQUfafCP6ZwUm7Ec5n2PzvNzBHnFWEJL1usMTFB6Ef0m=8pw@mail.gmail.com>
john smith <wempwer@gmail.com> writes:
> So it seems that clean filter is only run when checking out paths but
> not when checking out branches. Is my thinking correct?
If the new branch you are going to and your current branch record
different contents for a path, that path will have to be checked out
to the working tree, and smudge will be involved in the process to
turn the "clean" contents stored in a Git object needs to be smudged
for use in the outside world. If both branches have the same
contents, then there is no need to overwrite the path in the working
tree with the same thing, so it is not touched.
The clean and smudge operations should look _only_ at the contents
they are filtering, and nothing else, and the clean/smudge filtering
mechanism is designed to support that use case. It is not designed
to do things like embedding the name of the branch that is being
checked out into the result.
^ permalink raw reply
* Re: How to simulate a real checkout to test a new smudge filter?
From: john smith @ 2016-09-09 22:03 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Torsten Bögershausen, git
In-Reply-To: <e17a88cd-2de7-dc84-2262-743e82d8b047@gmail.com>
On 9/8/16, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 06.09.2016 o 23:01, john smith pisze:
>
>> I'd prefer smudge/clean filters instead of `make' scripts etc. to
>> convert template dotfiles into something usable and back because
>> filters:
>>
>> 1. could be run automatically
>>
>> 2. do not modify files as shown by `git show HEAD:<file>' and
>> therefore no files are reported as modified by git status and also
>> there are not conflicts when merging master into work/home branch.
>>
>> I have problems because with point 1 because apparently smudge filter
>> is not run automatically every time when branch is changed if files
>> listed in .gitattributes do not change. As the last resort I could
>> force smudge/clean filter to run just to keep advantage specified in
>> point 2.
>
> Couldn't you use post-checkout hook plus clean filter instead of
> clean/smudge filter pair, if the smudge part depends on the branch?
>
> Or make post-checkout hook invoke smudge filter... though
> `git cat-file --filters` is not in any released version, I think...
I moved smudge-filter functionality to post-checkout hook and only
left clean filter. Now post-checkout hooks is called every time when
changing branches but it's not working together with clean filter when
changing branches. For example, when I have this on master in
bash/.bashrc:
export EMAIL="@EMAIL@"
After moving to `work' it becomes this thanks to post-checkout hook
which replaces @EMAIL@ with correct definition:
export EMAIL="work@address.com"
But when I move to `home' branch from `work' I still get this:
export EMAIL="work@address.com"
post-checkout didn't work here because it can't see @EMAIL@ part in
bash/.bashrc because clean filter has not been run. I need to run this
single command to force `clean' filter to run to replace
work@address.com with @EMAIL@ and let post-checkout replace it with
correct value:
git checkout .
And now I get the correct definition:
export EMAIL="home@address.com"
So it seems that clean filter is only run when checking out paths but
not when checking out branches. Is my thinking correct?
--
<wempwer@gmail.com>
^ permalink raw reply
* Re: [PATCH v2] checkout: eliminate unnecessary merge for trivial checkout
From: Junio C Hamano @ 2016-09-09 21:55 UTC (permalink / raw)
To: Ben Peart; +Cc: git, pclouds, Ben Peart
In-Reply-To: <20160909192520.4812-1-benpeart@microsoft.com>
Ben Peart <peartben@gmail.com> writes:
> @@ -802,6 +806,87 @@ static void orphaned_commit_warning(struct commit *old, struct commit *new)
> free(refs.objects);
> }
>
> +static int needs_working_tree_merge(const struct checkout_opts *opts,
> + const struct branch_info *old,
> + const struct branch_info *new)
> +{
> + /*
> + * We must do the merge if we are actually moving to a new
> + * commit tree.
> + */
> + if (!old->commit || !new->commit ||
> + oidcmp(&old->commit->tree->object.oid, &new->commit->tree->object.oid))
> + return 1;
A huge helper function helps it somewhat, compared with the earlier
unreadable mess ;-).
Are we certain that at this point the commit objects are both parsed
and their tree->object.oid are both valid?
> + /*
> + * Honor the explicit request for a three-way merge or to throw away
> + * local changes
> + */
> + if (opts->merge || opts->force)
> + return 1;
Hmph, "git checkout -m HEAD" wouldn't have to do anything wrt the
index status, no?
For that matter, neither "git checkout -f HEAD". Unless we rely on
unpack_trees() to write over the working tree files.
... me goes and looks, and finds that merge_working_tree()
indeed does have a logic to do quite different thing when
"--force" is given.
This makes me wonder if the "merge_working_tree() is expensive, so
selectively skip calling it" approach is working at a wrong level.
Wouldn't the merge_working_tree() function itself a better place to
do this kind of "we may not have to do the full two-way merge"
optimization? It already looks at opts and does things differently
(e.g. when running with "--force", it does not even call unpack).
If you can optimize even more by looking at other fields in opts to
avoid unpack, that would fit better with the structure of the code
that we already have.
> + /*
> + * Checking out the requested commit may require updating the working
> + * directory and index, let the merge handle it.
> + */
> + if (opts->force_detach)
> + return 1;
This does not make much sense to me. After "git branch -f foo
HEAD", there is no difference in what is done to the index and the
working directory between "git checkout --detach HEAD" and "git
checkout foo", is there?
> + /*
> + * opts->writeout_stage cannot be used with switching branches so is
> + * not tested here
> + */
> +
> + /*
> + * Honor the explicit ignore requests
> + */
> + if (!opts->overwrite_ignore || opts->ignore_skipworktree
> + || opts->ignore_other_worktrees)
> + return 1;
Style. I think you earlier had
if (a || b ||
c)
and here you are doing
if (a || b
|| c)
Please pick one and stick to it (I'd pick the former).
> + /*
> + * If we're not creating a new branch, by definition we're changing
> + * the existing one so need to do the merge
> + */
> + if (!opts->new_branch)
> + return 1;
Sorry, but I fail to follow that line of thought. Starting from a
state where your HEAD points at commit A,
- switching to a detached HEAD pointing at a commit A,
- switching to an existing branch that already points at the same
commit A, and
- force updating an existing branch that was pointing at something
else to point at the same commit A,
would have the same effect as creating a new branch at commit A and
switching to it, no? The same comment applies to the remainder of
this function.
More importantly, merge_working_tree() checks things other than what
this function is checking. For example, it prevents you from
branch-switching (whether it is to switch to an existing branch that
has the same commit as the current HEAD, to switch to detached HEAD
state at the same commit as the current HEAD, or to switch to a new
branch that points at the same commit as the current HEAD) if your
index is unmerged (i.e. you are in the middle of a mergy operation).
So my gut feeling is that this:
> + /*
> + * Optimize the performance of "git checkout foo" by skipping the call
> + * to merge_working_tree where possible.
> + */
> + if (needs_working_tree_merge(opts, &old, new)) {
> + ret = merge_working_tree(opts, &old, new, &writeout_error);
works at the wrong level. The comment up to 'Optimize the
performance of "git checkout foo"' may correctly state what we want
to achieve, but I think we should do so not with "by skipping the
call to", but with "by optimizing merge_working_tree()".
Thanks.
^ permalink raw reply
* [RFC/PATCH] ls-files: adding support for submodules
From: Brandon Williams @ 2016-09-09 21:53 UTC (permalink / raw)
To: git; +Cc: Brandon Williams
Allow ls-files to recognize submodules in order to retrieve a list of
files from a repository's submodules. This is done by forking off a
process to recursively call ls-files on all submodules.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Hey git developers!
I'm new to the community and this is the first patch for an open source project
that I have worked on.
I'm looking forward to working on the project!
Brandon Williams
Documentation/git-ls-files.txt | 7 ++-
builtin/ls-files.c | 58 +++++++++++++++++++
t/t3007-ls-files-recurse-submodules.sh | 103 +++++++++++++++++++++++++++++++++
3 files changed, 167 insertions(+), 1 deletion(-)
create mode 100644 t/t3007-ls-files-recurse-submodules.sh
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 0d933ac..446209e 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -18,7 +18,8 @@ SYNOPSIS
[--exclude-per-directory=<file>]
[--exclude-standard]
[--error-unmatch] [--with-tree=<tree-ish>]
- [--full-name] [--abbrev] [--] [<file>...]
+ [--full-name] [--recurse-submodules]
+ [--abbrev] [--] [<file>...]
DESCRIPTION
-----------
@@ -137,6 +138,10 @@ a space) at the start of each line:
option forces paths to be output relative to the project
top directory.
+--recurse-submodules::
+ Recursively calls ls-files on each submodule in the repository.
+ Currently there is only support for the --cached mode.
+
--abbrev[=<n>]::
Instead of showing the full 40-byte hexadecimal object
lines, show only a partial prefix.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 00ea91a..c428a51 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,6 +14,7 @@
#include "resolve-undo.h"
#include "string-list.h"
#include "pathspec.h"
+#include "run-command.h"
static int abbrev;
static int show_deleted;
@@ -28,6 +29,7 @@ static int show_valid_bit;
static int line_terminator = '\n';
static int debug_mode;
static int show_eol;
+static int recurse_submodules;
static const char *prefix;
static int max_prefix_len;
@@ -152,6 +154,45 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+/**
+ * Recursively call ls-files on a submodule
+ */
+static void show_gitlink(const struct cache_entry *ce)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf name = STRBUF_INIT;
+ int submodule_name_len;
+ FILE *fp;
+
+ argv_array_push(&cp.args, "ls-files");
+ argv_array_push(&cp.args, "--recurse-submodules");
+ cp.git_cmd = 1;
+ cp.dir = ce->name;
+ cp.out = -1;
+ start_command(&cp);
+ fp = fdopen(cp.out, "r");
+
+ /*
+ * The ls-files child process produces filenames relative to
+ * the submodule. Prefix each line with the submodule path
+ * to make it relative to the current repository.
+ */
+ strbuf_addstr(&name, ce->name);
+ strbuf_addch(&name, '/');
+ submodule_name_len = name.len;
+ while (strbuf_getline(&buf, fp) != EOF) {
+ strbuf_addbuf(&name, &buf);
+ write_name(name.buf);
+ strbuf_setlen(&name, submodule_name_len);
+ }
+
+ finish_command(&cp);
+ strbuf_release(&buf);
+ strbuf_release(&name);
+ fclose(fp);
+}
+
static void show_ce_entry(const char *tag, const struct cache_entry *ce)
{
int len = max_prefix_len;
@@ -163,6 +204,10 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
len, ps_matched,
S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
return;
+ if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+ show_gitlink(ce);
+ return;
+ }
if (tag && *tag && show_valid_bit &&
(ce->ce_flags & CE_VALID)) {
@@ -468,6 +513,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{ OPTION_SET_INT, 0, "full-name", &prefix_len, NULL,
N_("make the output relative to the project top directory"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+ OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+ N_("recurse through submodules")),
OPT_BOOL(0, "error-unmatch", &error_unmatch,
N_("if any <file> is not in the index, treat this as an error")),
OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"),
@@ -519,6 +566,17 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
+ if (recurse_submodules &&
+ (show_stage || show_deleted || show_others || show_unmerged ||
+ show_killed || show_modified || show_resolve_undo ||
+ show_valid_bit || show_tag || show_eol))
+ die("ls-files --recurse-submodules can only be used in "
+ "--cached mode");
+
+ if (recurse_submodules && argc)
+ die("ls-files --recurse-submodules does not support path "
+ "arguments");
+
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD |
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
new file mode 100644
index 0000000..78deded
--- /dev/null
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='Test ls-files recurse-submodules feature
+
+This test verifies the recurse-submodules feature correctly lists files from
+submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup directory structure and submodules' '
+ echo a >a &&
+ mkdir b &&
+ echo b >b/b &&
+ git add a b &&
+ git commit -m "add a and b" &&
+ mkdir submodule &&
+ (
+ cd submodule &&
+ git init &&
+ echo c >c &&
+ git add c &&
+ git commit -m "add c"
+ ) &&
+ git submodule add ./submodule &&
+ git commit -m "added submodule"
+'
+
+cat >expect <<EOF
+.gitmodules
+a
+b/b
+submodule/c
+EOF
+
+test_expect_success 'ls-files correctly outputs files in submodule' '
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files does not output files not added to a repo' '
+ echo a >not_added &&
+ echo b >b/not_added &&
+ (
+ cd submodule &&
+ echo c >not_added
+ ) &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<EOF
+.gitmodules
+a
+b/b
+submodule/.gitmodules
+submodule/c
+submodule/subsub/d
+EOF
+
+test_expect_success 'ls-files recurses more than 1 level' '
+ (
+ cd submodule &&
+ mkdir subsub &&
+ (
+ cd subsub &&
+ git init &&
+ echo d >d &&
+ git add d &&
+ git commit -m "add d"
+ ) &&
+ git submodule add ./subsub &&
+ git commit -m "added subsub"
+ ) &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+cat >expect_error <<EOF
+fatal: ls-files --recurse-submodules does not support path arguments
+EOF
+
+test_expect_success 'error when using path arguments' '
+ test_must_fail git ls-files --recurse-submodules b 2>actual &&
+ test_cmp expect_error actual
+'
+
+cat >expect_error <<EOF
+fatal: ls-files --recurse-submodules can only be used in --cached mode
+EOF
+
+test_expect_success 'error when using different modes' '
+ for opt in {v,t}; do
+ test_must_fail git ls-files --recurse-submodules -$opt 2>actual &&
+ test_cmp expect_error actual
+ done &&
+ for opt in {deleted,modified,others,ignored,stage,killed,unmerged,eol}; do
+ test_must_fail git ls-files --recurse-submodules --$opt 2>actual &&
+ test_cmp expect_error actual
+ done
+'
+
+test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Git Ignore Exception bug
From: Nathan Williams @ 2016-09-09 21:39 UTC (permalink / raw)
To: git; +Cc: Eric Severtson
it ignore doesn't seem to be working properly when adding exceptions.
Environment:
Mac, running latest El Capitan (10.11.6)
% git --version
git version 2.9.0
-----------cut here-------------
#!/bin/sh
TEMP=mktemp
# Create a temporary repo
mkdir repo
cd repo
git init
echo "foo/*" >> .gitignore
echo "!foo/bar" >> .gitignore
git add .gitignore
git commit -m "Ignore file with exceptions"
# Create the directory structure
mkdir foo
mkdir foo/bar
# Create files to be ignored
touch foo/1
touch foo/2
# Create files that should be identified
touch foo/bar/a
touch foo/bar/b
# Should ignore foo and indicate that all the files in foo/bar are un-committed
git status
# Move back to the regular directory
cd ..
-----------cut here-------------
Expected results
% git st
On branch master
Untracked files:
(use "git add <file>..." to include in what will be committed)
foo/bar/
Nate
^ permalink raw reply
* Re: [PATCH] Move format-patch base commit and prerequisites before email signature
From: Junio C Hamano @ 2016-09-09 21:16 UTC (permalink / raw)
To: Josh Triplett; +Cc: Jeff King, git
In-Reply-To: <20160909210040.zlsczhcotrxnu4e4@x>
Josh Triplett <josh@joshtriplett.org> writes:
> It doesn't seem like a functional issue, but aesthetically it
> doesn't look good.
>
> Do you plan to make that change to print an additional blank line
> (likely inside print_bases), or should I?
I do not mind doing it myself, but I am already in today's
integration cycle (which will merge a handful of topics to
'master'), so I won't get around to it for some time. If you are
inclined to, please be my guest ;-)
Thanks.
^ permalink raw reply
* Re: [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null"
From: Junio C Hamano @ 2016-09-09 21:13 UTC (permalink / raw)
To: Jeff King
Cc: git, Michael Haggerty, Kevin Willford, Xiaolong Ye,
Johannes Schindelin, Josh Triplett
In-Reply-To: <20160909203447.nlyx6v76fpyx2vzl@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This patch defines the patch-id of a merge commit as
> essentially "null"; it has no patch-id. As a result,
> merges cannot match patch-ids via "--cherry-pick", and
> "format-patch --base" will not list merges in its list of
> prerequisite patch ids.
At first I wondered if such a change would make all merges look the
same, but the patch-ids.c comparison is not for ordering/sorting but
only for equality, so as long as the comparison function knows that
a comparison of anything with "null" yields "They are different", we
are OK.
> diff --git a/patch-ids.c b/patch-ids.c
> index 77e4663..8d06099 100644
> --- a/patch-ids.c
> +++ b/patch-ids.c
> @@ -7,18 +7,40 @@
> int commit_patch_id(struct commit *commit, struct diff_options *options,
> unsigned char *sha1, int diff_header_only)
> {
> - if (commit->parents)
> + if (commit->parents) {
> + if (commit->parents->next)
> + return PATCH_ID_NONE;
> diff_tree_sha1(commit->parents->item->object.oid.hash,
> commit->object.oid.hash, "", options);
> - else
> + } else
> diff_root_tree_sha1(commit->object.oid.hash, "", options);
> diffcore_std(options);
> - return diff_flush_patch_id(options, sha1, diff_header_only);
> + if (diff_flush_patch_id(options, sha1, diff_header_only))
> + return PATCH_ID_ERROR;
> + return PATCH_ID_OK;
> +}
Looks sensible. Thanks.
^ permalink raw reply
* Re: [PATCH v5 0/3] handle empty spec-compliant remote repos correctly
From: Jonathan Nieder @ 2016-09-09 21:07 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, gitster
In-Reply-To: <cover.1473452153.git.jonathantanmy@google.com>
Jonathan Tan wrote:
> connect.c | 32 ++++++++++++++++++++++++++------
> t/t5310-pack-bitmaps.sh | 4 ----
> t/t5512-ls-remote.sh | 40 ++++++++++++++++++++++++++++++++++++++++
> t/test-lib.sh | 4 ++++
> 4 files changed, 70 insertions(+), 10 deletions(-)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thank you,
Jonathan
^ permalink raw reply
* Re: [PATCH v3 0/2] patch-id for merges
From: Junio C Hamano @ 2016-09-09 21:01 UTC (permalink / raw)
To: Jeff King
Cc: git, Michael Haggerty, Kevin Willford, Xiaolong Ye,
Johannes Schindelin, Josh Triplett
In-Reply-To: <20160909203406.5j5pmom442yoe4su@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> And here is v3. Besides commit-message fixups, it drops patch 2, and
> instead the third patch teaches commit_patch_id() to distinguish between
> errors and "no patch id".
>
> Frankly, I still like v2 better, but I do not feel like arguing with
> Johannes about it anymore.
FWIW, I too like the simplicity of v2, as all the error-to-die
conversion is for cases in which there is no sane recovery path.
I'll have to take a bit deeper look at [v3 2/2] that had to become
more involved to decide if the additional flexibility is really
worth it.
Thanks.
^ permalink raw reply
* Re: [PATCH] Move format-patch base commit and prerequisites before email signature
From: Josh Triplett @ 2016-09-09 21:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqpoocajbb.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 09, 2016 at 01:51:04PM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
>
> > On Fri, Sep 09, 2016 at 12:41:56PM -0700, Junio C Hamano wrote:
> >> So here is a suggested replacement. I notice that in the MIME case,
> >> we do not leave any blank line between the last line of the patch
> >> and the baseinfo, which makes it look a bit strange, e.g. output of
> >> "format-patch --attach=mimemime -1" may end like this:
> >>
> >> + test_write_lines 1 2 >expect &&
> >> + test_cmp expect actual
> >> +'
> >> +
> >> test_expect_success 'format-patch --pretty=mboxrd' '
> >> sp=" " &&
> >> cat >msg <<-INPUT_END &&
> >> base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b
> >>
> >> --------------mimemime--
> >>
> >> We may want to tweak it a bit further.
> >> ...
> >
> > Looks good to me.
>
> Thanks.
>
> Do you mean that the base information that appears immediately after
> the patch text (either for MIME case or plain-text) does not bother
> you, though?
Sorry, I should have clarified that further. I meant that the
additional tests looked good to me.
As it turns out, the patch I used to test this on happened to have a
blank line as the last line of context before the base-commit line, so
I'd overlooked this in the non-MIME case. The issue you mentioned does
apply to both the MIME and non-MIME cases, and I agree that it needs
fixing. It doesn't seem like a functional issue, but aesthetically it
doesn't look good.
Do you plan to make that change to print an additional blank line
(likely inside print_bases), or should I?
^ permalink raw reply
* Re: git commit -p with file arguments
From: Christian Neukirchen @ 2016-09-09 20:52 UTC (permalink / raw)
To: git
In-Reply-To: <23de5ffe-eaf8-2d62-2202-f1bf6087d44b@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
> Which means that with "git add -p <pathspec> && git commit <pathspec>",
> the "git add -p <pathspec>" would carefully craft the <pathspec> state
> in the index... and "git commit <pathspec>" would take worktree version
> of <pathspec> for commit, ignoring what was in the index :-(
>
> Currently there is no way to create commit out of subset of the index,
> e.g. with "git commit :0:<path>"
I played around with creating a new index just for "add -p" and then
committing that one. Seems to have worked...
Perhaps I'll just wrap git-commit myself then.
cu,
--
Christian Neukirchen <chneukirchen@gmail.com> http://chneukirchen.org
^ permalink raw reply
* Re: [PATCH] Move format-patch base commit and prerequisites before email signature
From: Junio C Hamano @ 2016-09-09 20:51 UTC (permalink / raw)
To: Josh Triplett; +Cc: Jeff King, git
In-Reply-To: <20160909200721.xfkbud377ja4wkrt@x>
Josh Triplett <josh@joshtriplett.org> writes:
> On Fri, Sep 09, 2016 at 12:41:56PM -0700, Junio C Hamano wrote:
>> So here is a suggested replacement. I notice that in the MIME case,
>> we do not leave any blank line between the last line of the patch
>> and the baseinfo, which makes it look a bit strange, e.g. output of
>> "format-patch --attach=mimemime -1" may end like this:
>>
>> + test_write_lines 1 2 >expect &&
>> + test_cmp expect actual
>> +'
>> +
>> test_expect_success 'format-patch --pretty=mboxrd' '
>> sp=" " &&
>> cat >msg <<-INPUT_END &&
>> base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b
>>
>> --------------mimemime--
>>
>> We may want to tweak it a bit further.
>> ...
>
> Looks good to me.
Thanks.
Do you mean that the base information that appears immediately after
the patch text (either for MIME case or plain-text) does not bother
you, though?
^ permalink raw reply
* Re: [PATCH v4 3/3] connect: advertized capability is not a ref
From: Junio C Hamano @ 2016-09-09 20:40 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jonathan Tan, git, spearce, sbeller, peff
In-Reply-To: <20160909194037.GG25016@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Jonathan Tan wrote:
>
>> --- a/connect.c
>> +++ b/connect.c
>> @@ -172,8 +173,24 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>> continue;
>> }
>>
>> + if (!strcmp(name, "capabilities^{}")) {
>> + if (saw_response)
>> + warning("protocol error: unexpected capabilities^{}, "
>> + "continuing anyway");
>
> Please use die() for these.
> ...
> The rest looks good.
Will squash this in, then.
connect.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/connect.c b/connect.c
index df25d21..5ccbd10 100644
--- a/connect.c
+++ b/connect.c
@@ -175,11 +175,9 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
if (!strcmp(name, "capabilities^{}")) {
if (saw_response)
- warning("protocol error: unexpected capabilities^{}, "
- "continuing anyway");
+ die("protocol error: unexpected capabilities^{}");
if (got_dummy_ref_with_capabilities_declaration)
- warning("protocol error: multiple capabilities^{}, "
- "continuing anyway");
+ die("protocol error: multiple capabilities^{}");
got_dummy_ref_with_capabilities_declaration = 1;
continue;
}
@@ -188,8 +186,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
continue;
if (got_dummy_ref_with_capabilities_declaration)
- warning("protocol error: unexpected ref after capabilities^{}, "
- "using this ref and continuing anyway");
+ die("protocol error: unexpected ref after capabilities^{}");
ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);
oidcpy(&ref->old_oid, &old_oid);
^ permalink raw reply related
* Re: git commit -p with file arguments
From: Jakub Narębski @ 2016-09-09 20:39 UTC (permalink / raw)
To: Junio C Hamano, Jacob Keller; +Cc: Christian Neukirchen, Git mailing list
In-Reply-To: <xmqq8tv1c5nb.fsf@gitster.mtv.corp.google.com>
W dniu 09.09.2016 o 20:03, Junio C Hamano pisze:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>> It wants to commit bar too because you already added bar before. It works like:
>>
>> "git add bar && git add -p foo && git commit" does it not?
>>
>> I fail to see why "git commit -p <path>" would unstage the bar you
>> already added? Or am I missing some assumption here?
>
> Yes.
>
> "git commit -p <pathspec>" were added originally for lazy people who
> do not want to type "git add -p <pathspec> && git commit", which
> matches your expectation. If you already added "bar" that is
> outside of the <pathspec> given to "add -p", the final "git commit"
> step would record the latest contents of "bar" in it.
>
> For obvious reasons, "git commit -p <pathspec>" cannot be a
> short-hand to "git add -p <pathspec> && git commit <pathspec>", so
> the current behaviour was the best they could do for those who aded
> "commit -p", I guess.
The 'obvious reasons' are that
$ git add -p <pathspec> && git commit <pathspec>
would not work as intended, that is it wouldn't create commit out of
HEAD and changes to <pathspec> created interactively in the index.
"git commit <pathspec>" is a shortcut to "git commit --only <pathspec>";
the git-commit(1) manpage explains (emphasis mine):
-o
--only
Make a commit by taking the updated *working tree contents* of
the paths specified on the command line, disregarding any contents
that have been staged for other paths. [...]
Which means that with "git add -p <pathspec> && git commit <pathspec>",
the "git add -p <pathspec>" would carefully craft the <pathspec> state
in the index... and "git commit <pathspec>" would take worktree version
of <pathspec> for commit, ignoring what was in the index :-(
Currently there is no way to create commit out of subset of the index,
e.g. with "git commit :0:<path>"
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null"
From: Jeff King @ 2016-09-09 20:37 UTC (permalink / raw)
To: git
Cc: Michael Haggerty, Kevin Willford, Xiaolong Ye,
Johannes Schindelin, Josh Triplett
In-Reply-To: <20160909203447.nlyx6v76fpyx2vzl@sigill.intra.peff.net>
On Fri, Sep 09, 2016 at 04:34:47PM -0400, Jeff King wrote:
> This patch defines the patch-id of a merge commit as
> essentially "null"; it has no patch-id. As a result,
> merges cannot match patch-ids via "--cherry-pick", and
> "format-patch --base" will not list merges in its list of
> prerequisite patch ids.
>
> To distinguish between real errors and "null", we have to
> expand the semantics of commit_patch_id()'s return value,
> and callers need to distinguish these cases.
One alternative would be to add an out-parameter that is set in the
success case saying "yes, we have a real patch-id". And then the callers
could look like:
if (commit_patch_id(commit, &diffopt, sha1, 0, &got_one))
die("error!");
if (!got_one)
continue; /* silently skip */
We could even use the null sha1 to signal that rather than an extra
parameter, I suppose.
I dunno. It would make the callers less clunky, I think, but it does
feel a bit magical.
-Peff
^ permalink raw reply
* [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null"
From: Jeff King @ 2016-09-09 20:34 UTC (permalink / raw)
To: git
Cc: Michael Haggerty, Kevin Willford, Xiaolong Ye,
Johannes Schindelin, Josh Triplett
In-Reply-To: <20160909203406.5j5pmom442yoe4su@sigill.intra.peff.net>
The patch-id code which powers "log --cherry-pick" doesn't
look at whether each commit is a merge or not. It just feeds
the commit's first parent to the diff, and ignores any
additional parents.
In theory, this might be useful if you wanted to find
equivalence between, say, a merge commit and a squash-merge
that does the same thing. But it also promotes a false
equivalence between distinct merges. For example, every
"merge -s ours" would look like identical to an empty commit
(which is true in a sense, but presumably there was a value
in merging in the discarded history). Since patch-ids are
meant for throwing away duplicates, we should err on the
side of _not_ matching such merges.
Moreover, we may spend a lot of extra time computing these
merge diffs. In the case that inspired this patch, a "git
format-patch --cherry-pick" dropped from over 3 minutes to
less than 4 seconds.
This seems pretty drastic, but is easily explained. The
command was invoked by a "git rebase" of an older topic
branch; there had been tens of thousands of commits on the
upstream branch in the meantime. In addition, this project
used a topic-branch workflow with occasional "back-merges"
from "master" to each topic (to resolve conflicts on the
topics rather than in the merge commits). So there were not
only extra merges, but the diffs for these back-merges were
generally quite large (because they represented _everything_
that had been merged to master since the topic branched).
This patch defines the patch-id of a merge commit as
essentially "null"; it has no patch-id. As a result,
merges cannot match patch-ids via "--cherry-pick", and
"format-patch --base" will not list merges in its list of
prerequisite patch ids.
To distinguish between real errors and "null", we have to
expand the semantics of commit_patch_id()'s return value,
and callers need to distinguish these cases.
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/log.c | 10 +++++++++-
patch-ids.c | 40 ++++++++++++++++++++++++++++------------
patch-ids.h | 11 +++++++++--
3 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 92dc34d..ced1ea7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1343,8 +1343,16 @@ static void prepare_bases(struct base_tree_info *bases,
struct object_id *patch_id;
if (commit->util)
continue;
- if (commit_patch_id(commit, &diffopt, sha1, 0))
+
+ switch (commit_patch_id(commit, &diffopt, sha1, 0)) {
+ case PATCH_ID_OK:
+ break;
+ case PATCH_ID_NONE:
+ continue;
+ case PATCH_ID_ERROR:
die(_("cannot get patch id"));
+ }
+
ALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);
patch_id = bases->patch_id + bases->nr_patch_id;
hashcpy(patch_id->hash, sha1);
diff --git a/patch-ids.c b/patch-ids.c
index 77e4663..8d06099 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -7,18 +7,40 @@
int commit_patch_id(struct commit *commit, struct diff_options *options,
unsigned char *sha1, int diff_header_only)
{
- if (commit->parents)
+ if (commit->parents) {
+ if (commit->parents->next)
+ return PATCH_ID_NONE;
diff_tree_sha1(commit->parents->item->object.oid.hash,
commit->object.oid.hash, "", options);
- else
+ } else
diff_root_tree_sha1(commit->object.oid.hash, "", options);
diffcore_std(options);
- return diff_flush_patch_id(options, sha1, diff_header_only);
+ if (diff_flush_patch_id(options, sha1, diff_header_only))
+ return PATCH_ID_ERROR;
+ return PATCH_ID_OK;
+}
+
+/* avoid repeating ourselves in patch_id_cmp */
+static int cmp_setup(struct patch_id *p, struct diff_options *opt)
+{
+ if (!is_null_sha1(p->patch_id))
+ return 0; /* OK, already computed id */
+
+ switch (commit_patch_id(p->commit, opt, p->patch_id, 0)) {
+ case PATCH_ID_OK:
+ return 0;
+ case PATCH_ID_ERROR:
+ return error("Could not get patch ID for %s",
+ oid_to_hex(&p->commit->object.oid));
+ case PATCH_ID_NONE:
+ return -1; /* not an error, but nothing to compare */
+ }
+ die("BUG: unhandled patch_result");
}
/*
* When we cannot load the full patch-id for both commits for whatever
- * reason, the function returns -1 (i.e. return error(...)). Despite
+ * reason, the function returns -1. Despite
* the "cmp" in the name of this function, the caller only cares about
* the return value being zero (a and b are equivalent) or non-zero (a
* and b are different), and returning non-zero would keep both in the
@@ -30,14 +52,8 @@ static int patch_id_cmp(struct patch_id *a,
struct patch_id *b,
struct diff_options *opt)
{
- if (is_null_sha1(a->patch_id) &&
- commit_patch_id(a->commit, opt, a->patch_id, 0))
- return error("Could not get patch ID for %s",
- oid_to_hex(&a->commit->object.oid));
- if (is_null_sha1(b->patch_id) &&
- commit_patch_id(b->commit, opt, b->patch_id, 0))
- return error("Could not get patch ID for %s",
- oid_to_hex(&b->commit->object.oid));
+ if (cmp_setup(a, opt) || cmp_setup(b, opt))
+ return -1;
return hashcmp(a->patch_id, b->patch_id);
}
diff --git a/patch-ids.h b/patch-ids.h
index 0f34ea1..96fd2b9 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -12,8 +12,15 @@ struct patch_ids {
struct diff_options diffopts;
};
-int commit_patch_id(struct commit *commit, struct diff_options *options,
- unsigned char *sha1, int);
+enum patch_id_result {
+ PATCH_ID_ERROR = -1,
+ PATCH_ID_OK = 0,
+ PATCH_ID_NONE
+};
+
+enum patch_id_result commit_patch_id(struct commit *commit,
+ struct diff_options *options,
+ unsigned char *sha1, int);
int init_patch_ids(struct patch_ids *);
int free_patch_ids(struct patch_ids *);
struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
--
2.10.0.161.gdb62534
^ permalink raw reply related
* [PATCH v3 1/2] patch-ids: turn off rename detection
From: Jeff King @ 2016-09-09 20:34 UTC (permalink / raw)
To: git
Cc: Michael Haggerty, Kevin Willford, Xiaolong Ye,
Johannes Schindelin, Josh Triplett
In-Reply-To: <20160909203406.5j5pmom442yoe4su@sigill.intra.peff.net>
The patch-id code may be running inside another porcelain
like "git log" or "git format-patch", and therefore may have
set diff_detect_rename_default, either via the diff-ui
config, or by default since 5404c11 (diff: activate
diff.renames by default, 2016-02-25). This is the case even
if a command is run with `--no-renames`, as that is applied
only to the diff-options used by the command itself.
Rename detection doesn't help the patch-id results. It
_may_ actually hurt, as minor differences in the files that
would be overlooked by patch-id's canonicalization might
result in different renames (though I'd doubt that it ever
comes up in practice).
But mostly it is just a waste of CPU to compute these
renames.
Note that this does have one user-visible impact: the
prerequisite patches listed by "format-patch --base". There
may be some confusion between different versions of git as
older ones will enable renames, but newer ones will not.
However, this was already a problem, as people with
different settings for the "diff.renames" config would get
different results. After this patch, everyone should get the
same results, regardless of their config.
Signed-off-by: Jeff King <peff@peff.net>
---
patch-ids.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/patch-ids.c b/patch-ids.c
index 082412a..77e4663 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -45,6 +45,7 @@ int init_patch_ids(struct patch_ids *ids)
{
memset(ids, 0, sizeof(*ids));
diff_setup(&ids->diffopts);
+ ids->diffopts.detect_rename = 0;
DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
diff_setup_done(&ids->diffopts);
hashmap_init(&ids->patches, (hashmap_cmp_fn)patch_id_cmp, 256);
--
2.10.0.161.gdb62534
^ permalink raw reply related
* [PATCH v3 0/2] patch-id for merges
From: Jeff King @ 2016-09-09 20:34 UTC (permalink / raw)
To: git
Cc: Michael Haggerty, Kevin Willford, Xiaolong Ye,
Johannes Schindelin, Josh Triplett
In-Reply-To: <20160907220101.hwwutkiagfottbdd@sigill.intra.peff.net>
On Wed, Sep 07, 2016 at 06:01:01PM -0400, Jeff King wrote:
> Here's a re-roll of the series I posted at:
>
> http://public-inbox.org/git/20160907075346.z6wtmqnfc6bsunjb@sigill.intra.peff.net/
>
> Basically, it drops the time for "format-patch --cherry-pick" on a
> particular case from 3 minutes down to 3 seconds, by avoiding diffs
> on merge commits. Compared to v1, it fixes the totally-broken handling
> of commit_patch_id() pointed out by Johannes.
>
> [1/3]: patch-ids: turn off rename detection
> [2/3]: diff_flush_patch_id: stop returning error result
> [3/3]: patch-ids: use commit sha1 as patch-id for merge commits
And here is v3. Besides commit-message fixups, it drops patch 2, and
instead the third patch teaches commit_patch_id() to distinguish between
errors and "no patch id".
Frankly, I still like v2 better, but I do not feel like arguing with
Johannes about it anymore.
[1/2]: patch-ids: turn off rename detection
[2/2]: patch-ids: define patch-id of merge commits as "null"
-Peff
^ permalink raw reply
* Re: [RFCv3] Proposed questions for "Git User's Survey 2016", take three
From: Jakub Narębski @ 2016-09-09 20:20 UTC (permalink / raw)
To: David Bainbridge; +Cc: git
In-Reply-To: <CAFO0PHdGppfgSSYW4YB-0LEsuohyzSCkrk48fovppNfDYZx78w@mail.gmail.com>
Hello David,
On 7 September 2016 at 20:49, David Bainbridge
<david.bainbridge@gmail.com> wrote:
> Hi Jakub,
>
> I guess we could keep polishing this forever!
>
> Anyway, a couple of last comments from me:
> 1. Could you use the new Git logo instead of the +++/---- ? It would show
> some clear relationship with the Git community. The new logo has been in use
> for a few years now so I guess many people will recognise this.
Good idea. Done.
Though I say I like old logo one better...
> 2. If survs have dropdowns for countries and ages (integers) it might be
> worth using those instead of free text fields. Countries in particular
> should have a drop-down if available. It could probably avoid some
> post-survey correction.
Unfortunately Survs.com do not offer pre-made dropdowns for countries.
It offers a way to provide answers for a question by copy'n'paste of a plain
text file, for example with list of countries.
Coming up with correct list of countries is not easy, as I wrote:
JN> Survs.com do not offer list of countries as a pre-defined drop-down
JN> list (select, with search), and it looks like it is not as easy as
JN> I thought (though I could push responsibility to Locale::Country ;-):
JN>
JN> https://en.wikipedia.org/wiki/List_of_sovereign_states
JN>
JN> This question originally read "What country are you from?"
JN> which might be thought as country of birth... which may not
JN> exist any longer.
Though perhaps I make it harder that it really is:
$ perl -MLocale::Country -e 'print join "\n", all_country_names();'
> 3. The comment: Both question "3. Have you found Git easy to learn?" and "4.
> Have you found Git easy to use?" are about being it easy or hard in general;
> for mathematically inclined you can think about it as a weighted average."
> Not sure that this adds anything useful to the survey and could be removed,
> or at least needs correcting to mention the correct questions.
Fixed question numbers, removed unnecessary and not useful note about
"weighted average".
> OK, that's it from me!
Thanks!
Regards,
--
Jakub Narębski
^ permalink raw reply
* [PATCH v5 3/3] connect: advertized capability is not a ref
From: Jonathan Tan @ 2016-09-09 20:17 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, jrnieder, gitster
In-Reply-To: <cover.1473452153.git.jonathantanmy@google.com>
When cloning an empty repository served by standard git, "git clone" produces
the following reassuring message:
$ git clone git://localhost/tmp/empty
Cloning into 'empty'...
warning: You appear to have cloned an empty repository.
Checking connectivity... done.
Meanwhile when cloning an empty repository served by JGit, the output is more
haphazard:
$ git clone git://localhost/tmp/empty
Cloning into 'empty'...
Checking connectivity... done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.
This is a common command to run immediately after creating a remote repository
as preparation for adding content to populate it and pushing. The warning is
confusing and needlessly worrying.
The cause is that, since v3.1.0.201309270735-rc1~22 (Advertise capabilities
with no refs in upload service., 2013-08-08), JGit's ref advertisement includes
a ref named capabilities^{} to advertise its capabilities on (following the
specification in pack-protocol.txt), while git's ref advertisement is empty in
this case. This allows the client to learn about the server's capabilities and
is needed, for example, for fetch-by-sha1 to work when no refs are advertised.
This also affects "ls-remote". For example, against an empty repository served
by JGit:
$ git ls-remote git://localhost/tmp/empty
0000000000000000000000000000000000000000 capabilities^{}
Git advertises the same capabilities^{} ref in its ref advertisement for push
but since it never did so for fetch, the client didn't need to handle this
case. Handle it.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
connect.c | 14 ++++++++++++++
t/t5512-ls-remote.sh | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
diff --git a/connect.c b/connect.c
index 0c01a49..7224b5e 100644
--- a/connect.c
+++ b/connect.c
@@ -123,6 +123,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
* response does not necessarily mean an ACL problem, though.
*/
int saw_response;
+ int got_dummy_ref_with_capabilities_declaration = 0;
*list = NULL;
for (saw_response = 0; ; saw_response = 1) {
@@ -172,8 +173,21 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
continue;
}
+ if (!strcmp(name, "capabilities^{}")) {
+ if (saw_response)
+ die("protocol error: unexpected capabilities^{}");
+ if (got_dummy_ref_with_capabilities_declaration)
+ die("protocol error: multiple capabilities^{}");
+ got_dummy_ref_with_capabilities_declaration = 1;
+ continue;
+ }
+
if (!check_ref(name, flags))
continue;
+
+ if (got_dummy_ref_with_capabilities_declaration)
+ die("protocol error: unexpected ref after capabilities^{}");
+
ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);
oidcpy(&ref->old_oid, &old_oid);
*list = ref;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 819b9dd..befdfee 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -207,5 +207,45 @@ test_expect_success 'ls-remote --symref omits filtered-out matches' '
test_cmp expect actual
'
+test_lazy_prereq GIT_DAEMON '
+ test_tristate GIT_TEST_GIT_DAEMON &&
+ test "$GIT_TEST_GIT_DAEMON" != false
+'
+
+# This test spawns a daemon, so run it only if the user would be OK with
+# testing with git-daemon.
+test_expect_success PIPE,JGIT,GIT_DAEMON 'indicate no refs in standards-compliant empty remote' '
+ JGIT_DAEMON_PORT=${JGIT_DAEMON_PORT-${this_test#t}} &&
+ JGIT_DAEMON_PID= &&
+ git init --bare empty.git &&
+ >empty.git/git-daemon-export-ok &&
+ mkfifo jgit_daemon_output &&
+ {
+ jgit daemon --port="$JGIT_DAEMON_PORT" . >jgit_daemon_output &
+ JGIT_DAEMON_PID=$!
+ } &&
+ test_when_finished kill "$JGIT_DAEMON_PID" &&
+ {
+ read line &&
+ case $line in
+ Exporting*)
+ ;;
+ *)
+ echo "Expected: Exporting" &&
+ false;;
+ esac &&
+ read line &&
+ case $line in
+ "Listening on"*)
+ ;;
+ *)
+ echo "Expected: Listening on" &&
+ false;;
+ esac
+ } <jgit_daemon_output &&
+ # --exit-code asks the command to exit with 2 when no
+ # matching refs are found.
+ test_expect_code 2 git ls-remote --exit-code git://localhost:$JGIT_DAEMON_PORT/empty.git
+'
test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox