* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Brandon Williams @ 2016-11-21 21:56 UTC (permalink / raw)
To: Stefan Beller; +Cc: jrnieder, git, gitster, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-4-sbeller@google.com>
On 11/21, Stefan Beller wrote:
> diff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh
> new file mode 100755
> index 0000000000..8938a4c8b7
> --- /dev/null
> +++ b/t/t7412-submodule-interngitdirs.sh
> @@ -0,0 +1,41 @@
> +#!/bin/sh
> +
> +test_description='Test submodule interngitdirs
> +
> +This test verifies that `git submodue interngitdirs` moves a submodules git
> +directory into the superproject.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup a real submodule' '
> + git init sub1 &&
> + test_commit -C sub1 first &&
> + git submodule add ./sub1 &&
> + test_tick &&
> + git commit -m superproject
> +'
> +
> +test_expect_success 'intern the git dir' '
> + git submodule interngitdirs &&
> + test -f sub1/.git &&
> + test -d .git/modules/sub1 &&
> + # check that we did not break the repository:
> + git status
> +'
> +
> +test_expect_success 'setup a gitlink with missing .gitmodules entry' '
> + git init sub2 &&
> + test_commit -C sub2 first &&
> + git add sub2 &&
> + git commit -m superproject
> +'
> +
> +test_expect_success 'intern the git dir fails for incomplete submodules' '
> + test_must_fail git submodule interngitdirs &&
> + # check that we did not break the repository:
> + git status
> +'
> +
> +test_done
> +
Could we add a test which has nested submodules that need to be
migrated? Hopfully its just as easy as adding the test :)
--
Brandon Williams
^ permalink raw reply
* What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Junio C Hamano @ 2016-11-21 22:19 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.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* jc/for-each-ref-head-segfault-fix (2016-11-18) 1 commit
(merged to 'next' on 2016-11-21 at 3c1f352316)
+ for-each-ref: do not segv with %(HEAD) on an unborn branch
Using a %(HEAD) placeholder in "for-each-ref --format=" option
caused the command to segfault when on an unborn branch.
Will cook in 'next'.
* js/prepare-sequencer (2016-11-21) 1 commit
(merged to 'next' on 2016-11-21 at 7cdf4ca421)
+ i18n: fix unmatched single quote in error message
Fix for an error message string.
Will merge to 'master'.
* js/rebase-i-commentchar-fix (2016-11-21) 3 commits
- rebase -i: handle core.commentChar=auto
- stripspace: respect repository config
- rebase -i: highlight problems with core.commentchar
"git rebase -i" did not work well with core.commentchar
configuration variable for two reasons, both of which have been
fixed.
Waiting for an ack for updates.
Hopefully we can merge this before 2.11 final, as one of the
breakages is a recent regression.
* jc/cache-tree-wip (2016-11-18) 4 commits
- cache-tree: freshen the tree object at the top level
- cache-tree: retire cache_tree_fully_valid() API function
- commit: remove redundant check for active_cache_changed
- freshen_object(): factor out a helper function
* jk/trailers-placeholder-in-pretty (2016-11-21) 2 commits
- ref-filter: add support to display trailers as part of contents
- pretty: add %(trailers) format for displaying trailers of a commit message
* jt/trailer-with-cruft (2016-11-21) 1 commit
- doc: mention user-configured trailers
* sb/submodule-intern-gitdir (2016-11-21) 3 commits
- submodule--helper: add intern-git-dir function
- test-lib-functions.sh: teach test_commit -C <dir>
- submodule: use absolute path for computing relative path connecting
--------------------------------------------------
[Graduated to "master"]
* jk/create-branch-remove-unused-param (2016-11-09) 1 commit
(merged to 'next' on 2016-11-16 at 621254c832)
+ create_branch: drop unused "head" parameter
Code clean-up.
* nd/worktree-lock (2016-11-13) 1 commit
(merged to 'next' on 2016-11-16 at 67b731de07)
+ git-worktree.txt: fix typo "to"/"two", and add comma
Typofix.
* tk/diffcore-delta-remove-unused (2016-11-14) 1 commit
(merged to 'next' on 2016-11-16 at 51e66c2fa7)
+ diffcore-delta: remove unused parameter to diffcore_count_changes()
Code cleanup.
--------------------------------------------------
[Stalled]
* sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
- push: change submodule default to check when submodules exist
- submodule add: extend force flag to add existing repos
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will hold to wait for hv/submodule-not-yet-pushed-fix
* 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.
* 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.
* 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]
* dt/empty-submodule-in-merge (2016-11-17) 1 commit
- submodules: allow empty working-tree dirs in merge/cherry-pick
An empty directory in a working tree that can simply be nuked used
to interfere while merging or cherry-picking a change to create a
submodule directory there, which has been fixed..
Waiting for review.
* bw/grep-recurse-submodules (2016-11-18) 6 commits
- grep: search history of moved submodules
- grep: enable recurse-submodules to work on <tree> objects
- grep: optionally recurse into submodules
- grep: add submodules as a grep source type
- submodules: load gitmodules file from commit sha1
- submodules: add helper functions to determine presence of submodules
"git grep" learns to optionally recurse into submodules
Waiting for review.
* dt/smart-http-detect-server-going-away (2016-11-18) 2 commits
(merged to 'next' on 2016-11-21 at d502523347)
+ upload-pack: optionally allow fetching any sha1
+ remote-curl: don't hang when a server dies before any output
When the http server gives an incomplete response to a smart-http
rpc call, it could lead to client waiting for a full response that
will never come. Teach the client side to notice this condition
and abort the transfer.
An improvement counterproposal has failed.
cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>
Will cook in 'next'.
* mm/push-social-engineering-attack-doc (2016-11-14) 1 commit
(merged to 'next' on 2016-11-16 at b7c1b27563)
+ doc: mention transfer data leaks in more places
Doc update on fetching and pushing.
Will cook in 'next'.
* nd/worktree-move (2016-11-12) 11 commits
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
. copy.c: convert copy_file() to copy_dir_recursively()
. copy.c: style fix
. copy.c: convert bb_(p)error_msg to error(_errno)
. copy.c: delete unused code in copy_file()
. copy.c: import copy_file() from busybox
"git worktree" learned move and remove subcommands.
Waiting for a reroll.
Seems to break a test or two.
* jc/compression-config (2016-11-15) 1 commit
- compression: unify pack.compression configuration parsing
Compression setting for producing packfiles were spread across
three codepaths, one of which did not honor any configuration.
Unify these so that all of them honor core.compression and
pack.compression variables the same way.
Needs tests for pack-objects and fast-import.
* mm/gc-safety-doc (2016-11-16) 1 commit
(merged to 'next' on 2016-11-17 at fc0d225b6b)
+ git-gc.txt: expand discussion of races with other processes
Doc update.
Will cook in 'next'.
* hv/submodule-not-yet-pushed-fix (2016-11-16) 4 commits
(merged to 'next' on 2016-11-21 at 1a599af962)
+ submodule_needs_pushing(): explain the behaviour when we cannot answer
+ batch check whether submodule needs pushing into one call
+ serialize collection of refs that contain submodule changes
+ serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
Will cook in 'next'.
* kn/ref-filter-branch-list (2016-11-15) 18 commits
- for-each-ref: do not segv with %(HEAD) on an unborn branch
- 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, reviewed, looking good.
Expecting a reroll.
cf. <20161108201211.25213-1-Karthik.188@gmail.com>
cf. <CAOLa=ZQqe3vEj_428d41vd_4kfjzsm87Wam6Zm2dhXWkPdJ8Rw@mail.gmail.com>
cf. <xmqq7f84tqa7.fsf_-_@gitster.mtv.corp.google.com>
* bw/transport-protocol-policy (2016-11-09) 2 commits
(merged to 'next' on 2016-11-16 at 1391d3eeed)
+ transport: add protocol policy config option
+ lib-proto-disable: variable name fix
Finer-grained control of what protocols are allowed for transports
during clone/fetch/push have been enabled via a new configuration
mechanism.
Will cook in 'next'.
* jt/fetch-no-redundant-tag-fetch-map (2016-11-11) 1 commit
(merged to 'next' on 2016-11-16 at 5846c27cc5)
+ fetch: do not redundantly calculate tag refmap
Code cleanup to avoid using redundant refspecs while fetching with
the --tags option.
Will cook in 'next'.
* jc/retire-compaction-heuristics (2016-11-02) 3 commits
- SQUASH???
- SQUASH???
- diff: retire the original experimental "compaction" heuristics
Waiting for a reroll.
* jc/abbrev-autoscale-config (2016-11-01) 1 commit
- config.abbrev: document the new default that auto-scales
Waiting for a reroll.
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
- exclude: do not respect symlinks for in-tree .gitignore
- attr: do not respect symlinks for in-tree .gitattributes
- exclude: convert "check_index" into a flags field
- attr: convert "macro_ok" into a flags field
- add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
* sb/submodule-config-cleanup (2016-11-02) 3 commits
- submodule-config: clarify parsing of null_sha1 element
- submodule-config: rename commit_sha1 to commit_or_tree
- submodule config: inline config_from_{name, path}
What's the doneness of this topic?
* jc/push-default-explicit (2016-10-31) 2 commits
(merged to 'next' on 2016-11-01 at 8dc3a6cf25)
+ push: test pushing ambiguously named branches
+ push: do not use potentially ambiguous default refspec
A lazy "git push" without refspec did not internally use a fully
specified refspec to perform 'current', 'simple', or 'upstream'
push, causing unnecessary "ambiguous ref" errors.
Will cook in 'next'.
* jt/use-trailer-api-in-commands (2016-11-02) 6 commits
- sequencer: use trailer's trailer layout
- trailer: have function to describe trailer layout
- trailer: avoid unnecessary splitting on lines
- commit: make ignore_non_trailer take buf/len
- SQUASH???
- trailer: be stricter in parsing separators
Commands that operate on a log message and add lines to the trailer
blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
"commit -s", have been taught to use the logic of and share the
code with "git interpret-trailer".
What's the doneness of this topic?
* nd/rebase-forget (2016-10-28) 1 commit
- rebase: add --forget to cleanup rebase, leave HEAD untouched
"git rebase" learned "--forget" option, which allows a user to
remove the metadata left by an earlier "git rebase" that was
manually aborted without using "git rebase --abort".
Waiting for a reroll.
* jc/git-open-cloexec (2016-11-02) 3 commits
- sha1_file: stop opening files with O_NOATIME
- git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
- git_open(): untangle possible NOATIME and CLOEXEC interactions
The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
opens has been simplified.
We may want to drop the tip one.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-10-26 at 220e160451)
+ setup_git_env: avoid blind fall-back to ".git"
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/reset-unmerge (2016-10-24) 1 commit
- reset: --unmerge
After "git add" is run prematurely during a conflict resolution,
"git diff" can no longer be used as a way to sanity check by
looking at the combined diff. "git reset" learned a new
"--unmerge" option to recover from this situation.
Will discard.
This may not be needed, given that update-index has a similar
option.
* jc/merge-base-fp-only (2016-10-19) 8 commits
. merge-base: fp experiment
- merge: allow to use only the fp-only merge bases
- merge-base: limit the output to bases that are on first-parent chain
- merge-base: mark bases that are on first-parent chain
- merge-base: expose get_merge_bases_many_0() a bit more
- merge-base: stop moving commits around in remove_redundant()
- sha1_name: remove ONELINE_SEEN bit
- commit: simplify fastpath of merge-base
An experiment of merge-base that ignores common ancestors that are
not on the first parent chain.
Will discard.
The whole premise feels wrong.
* tb/convert-stream-check (2016-10-27) 2 commits
- convert.c: stream and fast search for binary
- read-cache: factor out get_sha1_from_index() helper
End-of-line conversion sometimes needs to see if the current blob
in the index has NULs and CRs to base its decision. We used to
always get a full statistics over the blob, but in many cases we
can return early when we have seen "enough" (e.g. if we see a
single NUL, the blob will be handled as binary). The codepaths
have been optimized by using streaming interface.
Waiting for review.
The tip seems to do too much in a single commit and may be better split.
cf. <20161012134724.28287-1-tboegi@web.de>
cf. <xmqqd1il5w4e.fsf@gitster.mtv.corp.google.com>
* pb/bisect (2016-10-18) 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 `--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
- t6030: no cleanup with bad merge base
- 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
Move more parts of "git bisect" to C.
Waiting for review.
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for a reroll.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sb/attr (2016-11-11) 35 commits
- completion: clone can initialize specific submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- 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
- attr: keep attr stack for each check
- attr: convert to new threadsafe API
- attr: make git_check_attr_counted static
- 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
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- 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
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
Waiting for review.
* va/i18n-perl-scripts (2016-11-11) 16 commits
- i18n: difftool: mark warnings for translation
- i18n: send-email: mark composing message for translation
- i18n: send-email: mark string with interpolation for translation
- i18n: send-email: mark warnings and errors for translation
- i18n: send-email: mark strings for translation
- i18n: add--interactive: mark status words for translation
- i18n: add--interactive: remove %patch_modes entries
- i18n: add--interactive: mark edit_hunk_manually message for translation
- i18n: add--interactive: i18n of help_patch_cmd
- i18n: add--interactive: mark patch prompt for translation
- i18n: add--interactive: mark plural strings
- i18n: clean.c: match string with git-add--interactive.perl
- i18n: add--interactive: mark strings with interpolation for translation
- i18n: add--interactive: mark simple here-documents for translation
- i18n: add--interactive: mark strings for translation
- Git.pm: add subroutines for commenting lines
Porcelain scripts written in Perl are getting internationalized.
Waiting for review.
* jc/latin-1 (2016-09-26) 2 commits
(merged to 'next' on 2016-09-28 at c8673e03c2)
+ utf8: accept "latin-1" as ISO-8859-1
+ utf8: refactor code to decide fallback encoding
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will hold to see if people scream.
* 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>
* 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.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-10-11 at 8928c8b9b3)
+ 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>
Will cook in 'next'.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Stefan Beller @ 2016-11-21 22:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqpoloh3xa.fsf@gitster.mtv.corp.google.com>
> * sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
> - push: change submodule default to check when submodules exist
> - submodule add: extend force flag to add existing repos
>
> Turn the default of "push.recurseSubmodules" to "check" when
> submodules seem to be in use.
>
> Will hold to wait for hv/submodule-not-yet-pushed-fix
Which is cooking in next, so we'd want to include this into next as well?
When including this series, we get 2 benefits:
* the cooking of hv/submodule-not-yet-pushed-fix is greatly enhanced as
more submodule users will make use of it (as it would be the default).
* for non submodule users we would see if the approximated estimation
if the user cares about submodules produces false positives:
if (has_submodules_configured || file_exists(git_path("modules")) ||
(!is_bare_repository() && file_exists(".gitmodules")))
recurse_submodules = RECURSE_SUBMODULES_CHECK;
else
recurse_submodules = RECURSE_SUBMODULES_OFF;
This heuristic was introduced after we got burned and called out by Linus,
so I would expect this series to not stress non submodule users any more.
>
> * dt/empty-submodule-in-merge (2016-11-17) 1 commit
> - submodules: allow empty working-tree dirs in merge/cherry-pick
>
> An empty directory in a working tree that can simply be nuked used
> to interfere while merging or cherry-picking a change to create a
> submodule directory there, which has been fixed..
>
> Waiting for review.
I thought I had reviewed it, will do again and comment.
> * bw/grep-recurse-submodules (2016-11-18) 6 commits
> - grep: search history of moved submodules
> - grep: enable recurse-submodules to work on <tree> objects
> - grep: optionally recurse into submodules
> - grep: add submodules as a grep source type
> - submodules: load gitmodules file from commit sha1
> - submodules: add helper functions to determine presence of submodules
>
> "git grep" learns to optionally recurse into submodules
>
> Waiting for review.
I did review both off and on list and I think the latest version is good.
> * hv/submodule-not-yet-pushed-fix (2016-11-16) 4 commits
> (merged to 'next' on 2016-11-21 at 1a599af962)
> + submodule_needs_pushing(): explain the behaviour when we cannot answer
> + batch check whether submodule needs pushing into one call
> + serialize collection of refs that contain submodule changes
> + serialize collection of changed submodules
>
> The code in "git push" to compute if any commit being pushed in the
> superproject binds a commit in a submodule that hasn't been pushed
> out was overly inefficient, making it unusable even for a small
> project that does not have any submodule but have a reasonable
> number of refs.
>
> Will cook in 'next'.
Thanks!
>
> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
> - sequencer: use trailer's trailer layout
> - trailer: have function to describe trailer layout
> - trailer: avoid unnecessary splitting on lines
> - commit: make ignore_non_trailer take buf/len
> - SQUASH???
> - trailer: be stricter in parsing separators
>
> Commands that operate on a log message and add lines to the trailer
> blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
> "commit -s", have been taught to use the logic of and share the
> code with "git interpret-trailer".
>
> What's the doneness of this topic?
From a cursory read (with the SQUASH applied)
this seems to be done to me.
> * sb/submodule-config-cleanup (2016-11-02) 3 commits
> - submodule-config: clarify parsing of null_sha1 element
> - submodule-config: rename commit_sha1 to commit_or_tree
> - submodule config: inline config_from_{name, path}
>
> What's the doneness of this topic?
Jake Keller reviewed this and it turns out I was not careful in patch 2/3.
Will resend.
Thanks,
Stefan
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Junio C Hamano @ 2016-11-21 23:05 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kbCebHojPDUdHn2mmjUwRe-FOcnJqtoa828PJwC6P5mNw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> * sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
>> - push: change submodule default to check when submodules exist
>> - submodule add: extend force flag to add existing repos
>>
>> Turn the default of "push.recurseSubmodules" to "check" when
>> submodules seem to be in use.
>>
>> Will hold to wait for hv/submodule-not-yet-pushed-fix
>
> Which is cooking in next, so we'd want to include this into next as well?
Not really. One step at a time.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Junio C Hamano @ 2016-11-21 23:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kbCebHojPDUdHn2mmjUwRe-FOcnJqtoa828PJwC6P5mNw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> * bw/grep-recurse-submodules (2016-11-18) 6 commits
>> - grep: search history of moved submodules
>> - grep: enable recurse-submodules to work on <tree> objects
>> - grep: optionally recurse into submodules
>> - grep: add submodules as a grep source type
>> - submodules: load gitmodules file from commit sha1
>> - submodules: add helper functions to determine presence of submodules
>>
>> "git grep" learns to optionally recurse into submodules
>>
>> Waiting for review.
>
> I did review both off and on list and I think the latest version is good.
I thought that there were strange mixups of two enumeration types
that are incompatible, at least. Is there an update that I didn't
see, or you didn't read problems pointed out on list?
>> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
>> - sequencer: use trailer's trailer layout
>> - trailer: have function to describe trailer layout
>> - trailer: avoid unnecessary splitting on lines
>> - commit: make ignore_non_trailer take buf/len
>> - SQUASH???
>> - trailer: be stricter in parsing separators
>>
>> Commands that operate on a log message and add lines to the trailer
>> blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
>> "commit -s", have been taught to use the logic of and share the
>> code with "git interpret-trailer".
>>
>> What's the doneness of this topic?
>
> From a cursory read (with the SQUASH applied)
> this seems to be done to me.
We are not all that in a hurry to move non-fix to 'next' only with a
cursory read at this point in the cycle ;-).
>> * sb/submodule-config-cleanup (2016-11-02) 3 commits
>> - submodule-config: clarify parsing of null_sha1 element
>> - submodule-config: rename commit_sha1 to commit_or_tree
>> - submodule config: inline config_from_{name, path}
>>
>> What's the doneness of this topic?
>
> Jake Keller reviewed this and it turns out I was not careful in patch 2/3.
>
> Will resend.
OK. Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Stefan Beller @ 2016-11-21 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqh970h1da.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 3:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I did review both off and on list and I think the latest version is good.
>
> I thought that there were strange mixups of two enumeration types
> that are incompatible, at least. Is there an update that I didn't
> see, or you didn't read problems pointed out on list?
Oh right; there was no resolution to that one IIRC.
>
>>> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
..
>> From a cursory read (with the SQUASH applied)
>> this seems to be done to me.
>
> We are not all that in a hurry to move non-fix to 'next' only with a
> cursory read at this point in the cycle ;-).
But the cycle only applies to patches going to master, but when
asking for doneness I assumed you were asking for opinions on
the transition pu -> next, which I would support.
Thanks,
Stefan
^ permalink raw reply
* [PATCHv2 0/3] submodule-config: clarify/cleanup docs and header
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
replacing sb/submodule-config-cleanup
v2:
addressed Jacobs concerns in patch2, fixing all occurrences of commit_sha1.
Thanks,
Stefan
interdiff to v1:
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 1df7a827ff..a91c1f085e 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -49,7 +49,7 @@ Functions
`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
v1:
A small series that would have helped me understand the submodule config
once again.
Thanks,
Stefan
Stefan Beller (3):
submodule config: inline config_from_{name, path}
submodule-config: rename commit_sha1 to commit_or_tree
submodule-config: clarify parsing of null_sha1 element
Documentation/technical/api-submodule-config.txt | 13 ++++--
submodule-config.c | 59 ++++++++++--------------
submodule-config.h | 4 +-
t/t7411-submodule-config.sh | 11 +++++
4 files changed, 45 insertions(+), 42 deletions(-)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv2 1/3] submodule config: inline config_from_{name, path}
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
In-Reply-To: <20161121232709.8906-1-sbeller@google.com>
There is no other user of config_from_{name, path}, such that there is no
reason for the existence of these one liner functions. Just inline these
to increase readability.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule-config.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 098085be69..15ffab6af4 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -471,18 +471,6 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return submodule;
}
-static const struct submodule *config_from_path(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *path)
-{
- return config_from(cache, commit_sha1, path, lookup_path);
-}
-
-static const struct submodule *config_from_name(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *name)
-{
- return config_from(cache, commit_sha1, name, lookup_name);
-}
-
static void ensure_cache_init(void)
{
if (is_cache_init)
@@ -508,14 +496,14 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name)
{
ensure_cache_init();
- return config_from_name(&the_submodule_cache, commit_sha1, name);
+ return config_from(&the_submodule_cache, commit_sha1, name, lookup_name);
}
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path)
{
ensure_cache_init();
- return config_from_path(&the_submodule_cache, commit_sha1, path);
+ return config_from(&the_submodule_cache, commit_sha1, path, lookup_path);
}
void submodule_free(void)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv2 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
In-Reply-To: <20161121232709.8906-1-sbeller@google.com>
It is also possible to pass in a tree hash to lookup a submodule config.
Make it clear by naming the variables accrodingly. Looking up a submodule
config by tree hash will come in handy in a later patch.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-submodule-config.txt | 8 ++--
submodule-config.c | 47 ++++++++++++------------
submodule-config.h | 4 +-
t/t7411-submodule-config.sh | 11 ++++++
4 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 941fa178dd..768458580f 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -47,15 +47,15 @@ Functions
Can be passed to the config parsing infrastructure to parse
local (worktree) submodule configurations.
-`const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path)`::
+`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
-`const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const char *name)`::
+`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
The same as above but lookup by name.
-If given the null_sha1 as commit_sha1 the local configuration of a
+If given the null_sha1 as commit_or_tree the local configuration of a
submodule will be returned (e.g. consolidated values from local git
configuration and the .gitmodules file in the worktree).
diff --git a/submodule-config.c b/submodule-config.c
index 15ffab6af4..4c5f5d074b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -263,12 +263,12 @@ int parse_push_recurse_submodules_arg(const char *opt, const char *arg)
return parse_push_recurse(opt, arg, 1);
}
-static void warn_multiple_config(const unsigned char *commit_sha1,
+static void warn_multiple_config(const unsigned char *commit_or_tree,
const char *name, const char *option)
{
const char *commit_string = "WORKTREE";
- if (commit_sha1)
- commit_string = sha1_to_hex(commit_sha1);
+ if (commit_or_tree)
+ commit_string = sha1_to_hex(commit_or_tree);
warning("%s:.gitmodules, multiple configurations found for "
"'submodule.%s.%s'. Skipping second one!",
commit_string, name, option);
@@ -276,7 +276,7 @@ static void warn_multiple_config(const unsigned char *commit_sha1,
struct parse_config_parameter {
struct submodule_cache *cache;
- const unsigned char *commit_sha1;
+ const unsigned char *commit_or_tree;
const unsigned char *gitmodules_sha1;
int overwrite;
};
@@ -300,7 +300,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->path)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"path");
else {
if (submodule->path)
@@ -314,7 +314,7 @@ static int parse_config(const char *var, const char *value, void *data)
int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
submodule->fetch_recurse != RECURSE_SUBMODULES_NONE)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"fetchrecursesubmodules");
else
submodule->fetch_recurse = parse_fetch_recurse(
@@ -324,7 +324,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->ignore)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"ignore");
else if (strcmp(value, "untracked") &&
strcmp(value, "dirty") &&
@@ -340,7 +340,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value) {
ret = config_error_nonbool(var);
} else if (!me->overwrite && submodule->url) {
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"url");
} else {
free((void *) submodule->url);
@@ -351,21 +351,21 @@ static int parse_config(const char *var, const char *value, void *data)
ret = config_error_nonbool(var);
else if (!me->overwrite &&
submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"update");
else if (parse_submodule_update_strategy(value,
&submodule->update_strategy) < 0)
die(_("invalid value for %s"), var);
} else if (!strcmp(item.buf, "shallow")) {
if (!me->overwrite && submodule->recommend_shallow != -1)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"shallow");
else
submodule->recommend_shallow =
git_config_bool(var, value);
} else if (!strcmp(item.buf, "branch")) {
if (!me->overwrite && submodule->branch)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"branch");
else {
free((void *)submodule->branch);
@@ -379,18 +379,18 @@ static int parse_config(const char *var, const char *value, void *data)
return ret;
}
-static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+static int gitmodule_sha1_from_commit(const unsigned char *commit_or_tree,
unsigned char *gitmodules_sha1,
struct strbuf *rev)
{
int ret = 0;
- if (is_null_sha1(commit_sha1)) {
+ if (is_null_sha1(commit_or_tree)) {
hashclr(gitmodules_sha1);
return 1;
}
- strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
+ strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_or_tree));
if (get_sha1(rev->buf, gitmodules_sha1) >= 0)
ret = 1;
@@ -402,7 +402,7 @@ static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
* revisions.
*/
static const struct submodule *config_from(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *key,
+ const unsigned char *commit_or_tree, const char *key,
enum lookup_type lookup_type)
{
struct strbuf rev = STRBUF_INIT;
@@ -418,7 +418,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
* return the first submodule. Can be used to check whether
* there are any submodules parsed.
*/
- if (!commit_sha1 || !key) {
+ if (!commit_or_tree || !key) {
struct hashmap_iter iter;
struct submodule_entry *entry;
@@ -428,7 +428,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev))
+ if (!gitmodule_sha1_from_commit(commit_or_tree, sha1, &rev))
goto out;
switch (lookup_type) {
@@ -448,7 +448,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
/* fill the submodule config into the cache */
parameter.cache = cache;
- parameter.commit_sha1 = commit_sha1;
+ // todo: get the actual tree here:
+ parameter.commit_or_tree = commit_or_tree;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf,
@@ -484,7 +485,7 @@ int parse_submodule_config_option(const char *var, const char *value)
{
struct parse_config_parameter parameter;
parameter.cache = &the_submodule_cache;
- parameter.commit_sha1 = NULL;
+ parameter.commit_or_tree = NULL;
parameter.gitmodules_sha1 = null_sha1;
parameter.overwrite = 1;
@@ -492,18 +493,18 @@ int parse_submodule_config_option(const char *var, const char *value)
return parse_config(var, value, ¶meter);
}
-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_name(const unsigned char *commit_or_tree,
const char *name)
{
ensure_cache_init();
- return config_from(&the_submodule_cache, commit_sha1, name, lookup_name);
+ return config_from(&the_submodule_cache, commit_or_tree, name, lookup_name);
}
-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_path(const unsigned char *commit_or_tree,
const char *path)
{
ensure_cache_init();
- return config_from(&the_submodule_cache, commit_sha1, path, lookup_path);
+ return config_from(&the_submodule_cache, commit_or_tree, path, lookup_path);
}
void submodule_free(void)
diff --git a/submodule-config.h b/submodule-config.h
index d05c542d2c..99df8e593c 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -25,9 +25,9 @@ struct submodule {
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
int parse_submodule_config_option(const char *var, const char *value);
-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_name(const unsigned char *commit_or_tree,
const char *name);
-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_path(const unsigned char *commit_or_tree,
const char *path);
void submodule_free(void);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 47562ce465..301ed5e48f 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -93,6 +93,17 @@ test_expect_success 'error message contains blob reference' '
)
'
+test_expect_success 'using tree sha1 works' '
+ (
+ cd super &&
+ tree=$(git rev-parse HEAD^{tree}) &&
+ commit=$(git rev-parse HEAD^{commit}) &&
+ test-submodule-config $commit b >expect &&
+ test-submodule-config $tree b >actual &&
+ test_cmp expect actual
+ )
+'
+
cat >super/expect_url <<EOF
Submodule url: 'git@somewhere.else.net:a.git' for path 'b'
Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv2 3/3] submodule-config: clarify parsing of null_sha1 element
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
In-Reply-To: <20161121232709.8906-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/technical/api-submodule-config.txt | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 768458580f..a91c1f085e 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -55,8 +55,11 @@ Functions
The same as above but lookup by name.
-If given the null_sha1 as commit_or_tree the local configuration of a
-submodule will be returned (e.g. consolidated values from local git
+Whenever a submodule configuration is parsed in `parse_submodule_config_option`
+via e.g. `gitmodules_config()`, it will be overwrite the entry with the sha1
+zeroed out. So in the normal case, when HEAD:.gitmodules is parsed first and
+then overlayed with the repository configuration, the null_sha1 entry contains
+the local configuration of a submodule (e.g. consolidated values from local git
configuration and the .gitmodules file in the worktree).
For an example usage see test-submodule-config.c.
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Jeff King @ 2016-11-21 23:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, git, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqtwb0ir4o.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 11:12:39AM -0800, Junio C Hamano wrote:
> > Yeah, I noticed that while reading the patch. My b9605bc4f2 did regress
> > this case, but called out the fact that "cd subdir && git stripspace"
> > would never have worked. So one step back, 2 steps forward, and Dscho's
> > patch is the right step forward.
>
> Yes, absolutely.
>
> I sent out a set of proposed amends, and the one for this step 1/3
> runs the command inside a subdirectory to force it not to find the
> .git/config file relative to its pwd, which can reveal the existing
> breakage without help by b9605bc4f2 ;-) hence can be forked for
> older maintenance tracks.
Makes sense, and your amended patch looks good.
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-11-22 0:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <CAGZ79kY=tzrsn7rS4UsRfSku_pKNNWNDc2OiTO-4-vg5h8NwWQ@mail.gmail.com>
On Mon, Nov 21, 2016 at 1:03 PM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Nov 21, 2016 at 1:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Can the effect of this change demonstrated in a new test? There
>> must be a scenario where the current behaviour is broken and this
>> change fixes an incorrect computation of relative path, no?
I do not think the current usage exposes this bug in
connect_work_tree_and_git_dir. It is only used in builtin/mv.c,
which fills the second parameter `git_dir` via a call to read_gitfile,
which itself produces an absolute path.
The latest patch of this series however passes in relative path for the
respective git directories.
So the commit message of this patch is misleading at least.
Maybe:
The current caller of connect_work_tree_and_git_dir passes
an absolute path for the `git_dir` parameter. In the future patch
we will also pass in relative path for `git_dir`. Extend the functionality
of connect_work_tree_and_git_dir to take relative paths for parameters.
We could work around this in the future patch by computing the absolute
path for the git_dir in the calling site, however accepting relative
paths for either parameter makes the API for this function easier
to use.
While at it, change `real_work_tree` to be non const as we own
the memory.
>
> I found the latest patch of this series broken without this patch.
> I'll try to find existing broken code, though.
^ permalink raw reply
* Re: [PATCHv2 3/3] submodule-config: clarify parsing of null_sha1 element
From: Brandon Williams @ 2016-11-22 0:09 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, gitster, jacob.keller
In-Reply-To: <20161121232709.8906-4-sbeller@google.com>
On 11/21, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/technical/api-submodule-config.txt | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
> index 768458580f..a91c1f085e 100644
> --- a/Documentation/technical/api-submodule-config.txt
> +++ b/Documentation/technical/api-submodule-config.txt
> @@ -55,8 +55,11 @@ Functions
>
> The same as above but lookup by name.
>
> -If given the null_sha1 as commit_or_tree the local configuration of a
> -submodule will be returned (e.g. consolidated values from local git
> +Whenever a submodule configuration is parsed in `parse_submodule_config_option`
> +via e.g. `gitmodules_config()`, it will be overwrite the entry with the sha1
s/will be overwrite/will overwrite
> +zeroed out. So in the normal case, when HEAD:.gitmodules is parsed first and
> +then overlayed with the repository configuration, the null_sha1 entry contains
> +the local configuration of a submodule (e.g. consolidated values from local git
> configuration and the .gitmodules file in the worktree).
>
> For an example usage see test-submodule-config.c.
> --
> 2.11.0.rc2.18.g0126045.dirty
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCHv2 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Brandon Williams @ 2016-11-22 0:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, gitster, jacob.keller
In-Reply-To: <20161121232709.8906-3-sbeller@google.com>
On 11/21, Stefan Beller wrote:
>
> switch (lookup_type) {
> @@ -448,7 +448,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
>
> /* fill the submodule config into the cache */
> parameter.cache = cache;
> - parameter.commit_sha1 = commit_sha1;
> + // todo: get the actual tree here:
s/todo/TODO
Makes it more clear that this is a TODO
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v15 19/27] bisect--helper: `bisect_state` & `bisect_head` shell function in C
From: Stephan Beyer @ 2016-11-22 0:12 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1b54-0ac61448-98b1-4c7d-bfdd-47af95f52153-000000@eu-west-1.amazonses.com>
Hi,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> Reimplement the `bisect_state` shell function in C and also add a
> subcommand `--bisect-state` to `git-bisect--helper` to call it from
> git-bisect.sh .
>
> Using `--bisect-state` subcommand is a temporary measure to port shell
> function to C so as to use the existing test suite. As more functions
> are ported, this subcommand will be retired and will be called by some
> other methods.
>
> `bisect_head` is called from `bisect_state` thus its not required to
> introduce another subcommand.
Missing comma before "thus", and "it is" (or "it's") instead of "its" :)
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 1767916..1481c6d 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -784,6 +786,79 @@ static int bisect_autostart(struct bisect_terms *terms)
> return 0;
> }
>
> +static char *bisect_head(void)
> +{
> + if (is_empty_or_missing_file(git_path_bisect_head()))
> + return "HEAD";
> + else
> + return "BISECT_HEAD";
> +}
This is very shellish.
In C I'd expect something like
static int bisect_head_sha1(unsigned char *sha)
{
int res;
if (is_empty_or_missing_file(git_path_bisect_head()))
res = get_sha1("HEAD", sha);
else
res = get_sha1("BISECT_HEAD", sha);
if (res)
return error(_("Could not find BISECT_HEAD or HEAD."));
return 0;
}
> +
> +static int bisect_state(struct bisect_terms *terms, const char **argv,
> + int argc)
> +{
> + const char *state = argv[0];
> +
> + get_terms(terms);
> + if (check_and_set_terms(terms, state))
> + return -1;
> +
> + if (!argc)
> + die(_("Please call `--bisect-state` with at least one argument"));
I think this check should move to cmd_bisect__helper. There are also the
other argument number checks.
> +
> + if (argc == 1 && one_of(state, terms->term_good,
> + terms->term_bad, "skip", NULL)) {
> + const char *bisected_head = xstrdup(bisect_head());
> + const char *hex[1];
Maybe:
const char *hex;
> + unsigned char sha1[20];
> +
> + if (get_sha1(bisected_head, sha1))
> + die(_("Bad rev input: %s"), bisected_head);
And instead of...
> + if (bisect_write(state, sha1_to_hex(sha1), terms, 0))
> + return -1;
> +
> + *hex = xstrdup(sha1_to_hex(sha1));
> + if (check_expected_revs(hex, 1))
> + return -1;
... simply:
hex = xstrdup(sha1_to_hex(sha1));
if (set_state(terms, state, hex)) {
free(hex);
return -1;
}
free(hex);
where:
static int set_state(struct bisect_terms *terms, const char *state,
const char *hex)
{
if (bisect_write(state, hex, terms, 0))
return -1;
if (check_expected_revs(&hex, 1))
return -1;
return 0;
}
> + return bisect_auto_next(terms, NULL);
> + }
> +
> + if ((argc == 2 && !strcmp(state, terms->term_bad)) ||
> + one_of(state, terms->term_good, "skip", NULL)) {
> + int i;
> + struct string_list hex = STRING_LIST_INIT_DUP;
> +
> + for (i = 1; i < argc; i++) {
> + unsigned char sha1[20];
> +
> + if (get_sha1(argv[i], sha1)) {
> + string_list_clear(&hex, 0);
> + die(_("Bad rev input: %s"), argv[i]);
> + }
> + string_list_append(&hex, sha1_to_hex(sha1));
> + }
> + for (i = 0; i < hex.nr; i++) {
... And replace this:
> + const char **hex_string = (const char **) &hex.items[i].string;
> + if(bisect_write(state, *hex_string, terms, 0)) {
> + string_list_clear(&hex, 0);
> + return -1;
> + }
> + if (check_expected_revs(hex_string, 1)) {
> + string_list_clear(&hex, 0);
> + return -1;
> + }
by:
const char *hex_str = hex.items[i].string;
if (set_state(terms, state, hex_string)) {
string_list_clear(&hex, 0);
return -1;
}
> + }
> + string_list_clear(&hex, 0);
> + return bisect_auto_next(terms, NULL);
> + }
> +
> + if (!strcmp(state, terms->term_bad))
> + die(_("'git bisect %s' can take only one argument."),
> + terms->term_bad);
> +
> + return -1;
> +}
> +
> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> {
> enum {
> @@ -823,6 +899,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> N_("verify the next bisection state then find the next bisection state"), BISECT_AUTO_NEXT),
> OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
> N_("start the bisection if BISECT_START empty or missing"), BISECT_AUTOSTART),
> + OPT_CMDMODE(0, "bisect-state", &cmdmode,
> + N_("mark the state of ref (or refs)"), BISECT_STATE),
"mark the state of the given revs"
Note that rev != ref
> @@ -902,6 +980,14 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> terms.term_bad = "bad";
> res = bisect_autostart(&terms);
> break;
> + case BISECT_STATE:
> + if (argc == 0)
> + die(_("--bisect-state requires at least 1 argument"));
"at least one revision"
> diff --git a/git-bisect.sh b/git-bisect.sh
> index cd56551..a9eebbb 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -61,44 +51,7 @@ bisect_skip() {
> esac
> all="$all $revs"
> done
> - eval bisect_state 'skip' $all
[...deleted lines...]
> + eval git bisect--helper --bisect-state 'skip' $all
I think you don't need "eval" here any longer.
> @@ -184,8 +137,8 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
> state="$TERM_GOOD"
> fi
>
> - # We have to use a subshell because "bisect_state" can exit.
> - ( bisect_state $state >"$GIT_DIR/BISECT_RUN" )
> + # We have to use a subshell because "--bisect-state" can exit.
> + ( git bisect--helper --bisect-state $state >"$GIT_DIR/BISECT_RUN" )
The new comment is funny, but you don't need a subshell here any longer.
~Stephan
^ permalink raw reply
* Re: [PATCHv2 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Stefan Beller @ 2016-11-22 0:15 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano, Jacob Keller
In-Reply-To: <20161122001148.GD149321@google.com>
On Mon, Nov 21, 2016 at 4:11 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/21, Stefan Beller wrote:
>>
>> switch (lookup_type) {
>> @@ -448,7 +448,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
>>
>> /* fill the submodule config into the cache */
>> parameter.cache = cache;
>> - parameter.commit_sha1 = commit_sha1;
>> + // todo: get the actual tree here:
>
> s/todo/TODO
>
> Makes it more clear that this is a TODO
>
The // is more annoying here. I'll review these changes closely
before sending out v3.
^ permalink raw reply
* Re: [PATCHv2 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Brandon Williams @ 2016-11-22 0:16 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Junio C Hamano, Jacob Keller
In-Reply-To: <CAGZ79kY+0WHFgXfVmEmw7H-qwuUcWcSssRyMzg2XqsGSDq+FCw@mail.gmail.com>
On 11/21, Stefan Beller wrote:
> On Mon, Nov 21, 2016 at 4:11 PM, Brandon Williams <bmwill@google.com> wrote:
> > On 11/21, Stefan Beller wrote:
> >>
> >> switch (lookup_type) {
> >> @@ -448,7 +448,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
> >>
> >> /* fill the submodule config into the cache */
> >> parameter.cache = cache;
> >> - parameter.commit_sha1 = commit_sha1;
> >> + // todo: get the actual tree here:
> >
> > s/todo/TODO
> >
> > Makes it more clear that this is a TODO
> >
>
> The // is more annoying here. I'll review these changes closely
> before sending out v3.
Well I prefer // to /* */ but that's not the style we use :)
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Jonathan Nieder @ 2016-11-22 0:44 UTC (permalink / raw)
To: Jeff King; +Cc: git, Duy Nguyen
In-Reply-To: <20161020062430.rxupwheaeydtcvf3@sigill.intra.peff.net>
Hi,
Jeff King wrote:
> This passes the test suite (after the adjustments in the
> previous patches), but there's a risk of regression for any
> cases where the fallback usually works fine but the code
> isn't exercised by the test suite. So by itself, this
> commit is a potential step backward, but lets us take two
> steps forward once we've identified and fixed any such
> instances.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> environment.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/environment.c b/environment.c
> index cd5aa57..b1743e6 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -164,8 +164,11 @@ static void setup_git_env(void)
> const char *replace_ref_base;
>
> git_dir = getenv(GIT_DIR_ENVIRONMENT);
> - if (!git_dir)
> + if (!git_dir) {
> + if (!startup_info->have_repository)
> + die("BUG: setup_git_env called without repository");
> git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
> + }
This trips reproducibly for
git ls-remote https://kernel.googlesource.com/pub/scm/git/git
when run outside of a git repository.
Backtrace:
#0 setup_git_env () at environment.c:172
#1 get_git_dir () at environment.c:214
#2 get_helper at transport-helper.c:127
#3 get_refs_list (for_push=0) at transport-helper.c:1038
#4 transport_get_remote_refs at transport.c:1076
#5 cmd_ls_remote at builtin/ls-remote.c:97
transport-helper.c:127 is
argv_array_pushf(&helper->env_array, "%s=%s", GIT_DIR_ENVIRONMENT,
get_git_dir());
That code is pretty clearly wrong. Should it be made conditional
on have_git_dir(), like this?
Thanks,
Jonathan
transport-helper.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 91aed35..e4fd982 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -124,8 +124,9 @@ static struct child_process *get_helper(struct transport *transport)
helper->git_cmd = 0;
helper->silent_exec_failure = 1;
- argv_array_pushf(&helper->env_array, "%s=%s", GIT_DIR_ENVIRONMENT,
- get_git_dir());
+ if (have_git_dir())
+ argv_array_pushf(&helper->env_array, "%s=%s",
+ GIT_DIR_ENVIRONMENT, get_git_dir());
code = start_command(helper);
if (code < 0 && errno == ENOENT)
--
^ permalink raw reply related
* Re: [PATCH v15 23/27] bisect--helper: `bisect_replay` shell function in C
From: Stephan Beyer @ 2016-11-22 0:49 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1b29-65f79716-42c6-4327-acda-8c8d0fe05471-000000@eu-west-1.amazonses.com>
Okay Pranit,
this is the last patch for me to review in this series.
Now that I have a coarse overview of what you did, I have the general
remark that imho the "terms" variable should simply be global instead of
being passed around all the time.
I also had some other remarks but I forgot them... maybe they come to my
mind again when I see patch series v16.
I also want to remark again that I am not a Git developer and only
reviewed this because of my interest in git-bisect. So some of my
suggestions might conflict with other beliefs here. For example, I
consider it very bad style to leak memory... but Git is rather written
as a scripting tool than a genuine library, so perhaps many people here
do not care about it as long as it works...
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index c18ca07..b367d8d 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -601,7 +602,6 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
> terms->term_good = arg;
> } else if (!strcmp(arg, "--term-bad") ||
> !strcmp(arg, "--term-new")) {
> - const char *next_arg;
This should already have been removed in patch 15/27, not here.
> @@ -875,6 +875,117 @@ static int bisect_log(void)
> return status;
> }
>
> +static int get_next_word(const char *line, int pos, struct strbuf *word)
> +{
> + int i, len = strlen(line), begin = 0;
> + strbuf_reset(word);
> + for (i = pos; i < len; i++) {
> + if (line[i] == ' ' && begin)
> + return i + 1;
> +
> + if (!begin)
> + begin = 1;
> + strbuf_addch(word, line[i]);
> + }
> +
> + return i;
> +}
> +
> +static int bisect_replay(struct bisect_terms *terms, const char *filename)
> +{
> + struct strbuf line = STRBUF_INIT;
> + struct strbuf word = STRBUF_INIT;
> + FILE *fp = NULL;
(The initialization is not necessary here.)
> + int res = 0;
> +
> + if (is_empty_or_missing_file(filename)) {
> + error(_("no such file with name '%s' exists"), filename);
The error message is misleading if the file exists but is empty.
Maybe something like "cannot read file '%s' for replaying"?
> + res = -1;
> + goto finish;
goto fail;
:D
> + }
> +
> + if (bisect_reset(NULL)) {
> + res = -1;
> + goto finish;
goto fail;
> + }
> +
> + fp = fopen(filename, "r");
> + if (!fp) {
> + res = -1;
> + goto finish;
goto fail;
> + }
> +
> + while (strbuf_getline(&line, fp) != EOF) {
> + int pos = 0;
> + while (pos < line.len) {
> + pos = get_next_word(line.buf, pos, &word);
> +
> + if (!strcmp(word.buf, "git")) {
> + continue;
> + } else if (!strcmp(word.buf, "git-bisect")) {
> + continue;
> + } else if (!strcmp(word.buf, "bisect")) {
> + continue;
> + } else if (!strcmp(word.buf, "#")) {
> + break;
Maybe it is more robust to check whether word.buf begins with #
> + }
> +
> + get_terms(terms);
> + if (check_and_set_terms(terms, word.buf)) {
> + res = -1;
> + goto finish;
goto fail;
> + }
> +
> + if (!strcmp(word.buf, "start")) {
> + struct argv_array argv = ARGV_ARRAY_INIT;
> + sq_dequote_to_argv_array(line.buf+pos, &argv);
> + if (bisect_start(terms, 0, argv.argv, argv.argc)) {
> + argv_array_clear(&argv);
> + res = -1;
> + goto finish;
goto fail;
> + }
> + argv_array_clear(&argv);
> + break;
> + }
> +
> + if (one_of(word.buf, terms->term_good,
> + terms->term_bad, "skip", NULL)) {
> + if (bisect_write(word.buf, line.buf+pos, terms, 0)) {
> + res = -1;
> + goto finish;
goto fail;
> + }
> + break;
> + }
> +
> + if (!strcmp(word.buf, "terms")) {
> + struct argv_array argv = ARGV_ARRAY_INIT;
> + sq_dequote_to_argv_array(line.buf+pos, &argv);
> + if (bisect_terms(terms, argv.argv, argv.argc)) {
> + argv_array_clear(&argv);
> + res = -1;
> + goto finish;
goto fail;
> + }
> + argv_array_clear(&argv);
> + break;
> + }
> +
> + error(_("?? what are you talking about?"));
I know this string is taken from the original source. However, even
something like error(_("Replay file contains rubbish (\"%s\")"),
word.buf) is more informative.
> + res = -1;
> + goto finish;
goto fail;
> + }
> + }
> + goto finish;
+fail:
+ res = -1;
I just wanted to make finally clear what I was referring to by the
"goto fail" trick. :D
Also I think the readability could be improved by extracting the body of
the outer while loop into an extra function replay_line(). Then most of
my suggested "goto fail;" lines simply become "return -1;" :)
> @@ -998,6 +1112,13 @@ int cmd_bisect__helper(...)
> die(_("--bisect-log requires 0 arguments"));
> res = bisect_log();
> break;
> + case BISECT_REPLAY:
> + if (argc != 1)
> + die(_("--bisect-replay requires 1 argument"));
I'd keep the (already translated) string from the original source:
"No logfile given"
Cheers,
Stephan
^ permalink raw reply
* Re: [PATCHv2 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Stefan Beller @ 2016-11-22 1:25 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano, Jacob Keller
In-Reply-To: <20161122001658.GE149321@google.com>
On Mon, Nov 21, 2016 at 4:16 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/21, Stefan Beller wrote:
>> On Mon, Nov 21, 2016 at 4:11 PM, Brandon Williams <bmwill@google.com> wrote:
>> > On 11/21, Stefan Beller wrote:
>> >>
>> >> switch (lookup_type) {
>> >> @@ -448,7 +448,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
>> >>
>> >> /* fill the submodule config into the cache */
>> >> parameter.cache = cache;
>> >> - parameter.commit_sha1 = commit_sha1;
>> >> + // todo: get the actual tree here:
>> >
>> > s/todo/TODO
>> >
>> > Makes it more clear that this is a TODO
>> >
>>
>> The // is more annoying here. I'll review these changes closely
>> before sending out v3.
>
> Well I prefer // to /* */ but that's not the style we use :)
background: Initially I assume we would need to do work here as that part
is exposed to the user in error messages, such that we maybe want to
go the reverse direction and not state a tree but instead the commit containing
it. Since then I decided that it is not worth to optimize for some
hypothetical future
and hence I did not go with the internal todo note I put there. Then I
forgot about
it and it just showed up in the patch here.
Having looked through the patch again, the rest looks clean to me.
Stefan
>
> --
> Brandon Williams
^ permalink raw reply
* [PATCHv3 0/3] submodule-config: clarify/cleanup docs and header
From: Stefan Beller @ 2016-11-22 1:35 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jacob.keller, Stefan Beller
replacing sb/submodule-config-cleanup
v3:
diff to current origin/sb/submodule-config-cleanup:
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 1df7a827ff..e06a3fd2de 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -49,17 +49,17 @@ Functions
`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
The same as above but lookup by name.
Whenever a submodule configuration is parsed in `parse_submodule_config_option`
-via e.g. `gitmodules_config()`, it will be overwrite the entry with the sha1
-zeroed out. So in the normal case, when HEAD:.gitmodules is parsed first and
-then overlayed with the repository configuration, the null_sha1 entry contains
-the local configuration of a submodule (e.g. consolidated values from local git
+via e.g. `gitmodules_config()`, it will be overwrite the null_sha1 entry.
+So in the normal case, when HEAD:.gitmodules is parsed first and then overlayed
+with the repository configuration, the null_sha1 entry contains the local
+configuration of a submodule (e.g. consolidated values from local git
configuration and the .gitmodules file in the worktree).
For an example usage see test-submodule-config.c.
diff --git a/submodule-config.c b/submodule-config.c
index 4c5f5d074b..d88a746c56 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -448,7 +448,6 @@ static const struct submodule *config_from(struct submodule_cache *cache,
/* fill the submodule config into the cache */
parameter.cache = cache;
- // todo: get the actual tree here:
parameter.commit_or_tree = commit_or_tree;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
v2:
addressed Jacobs concerns in patch2, fixing all occurrences of commit_sha1.
Thanks,
Stefan
interdiff to v1:
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 1df7a827ff..a91c1f085e 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -49,7 +49,7 @@ Functions
`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
v1:
A small series that would have helped me understand the submodule config
once again.
Thanks,
Stefan
Stefan Beller (3):
submodule config: inline config_from_{name, path}
submodule-config: rename commit_sha1 to commit_or_tree
submodule-config: clarify parsing of null_sha1 element
Documentation/technical/api-submodule-config.txt | 13 ++++--
submodule-config.c | 58 ++++++++++--------------
submodule-config.h | 4 +-
t/t7411-submodule-config.sh | 11 +++++
4 files changed, 44 insertions(+), 42 deletions(-)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv3 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Stefan Beller @ 2016-11-22 1:35 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jacob.keller, Stefan Beller
In-Reply-To: <20161122013550.1800-1-sbeller@google.com>
It is also possible to pass in a tree hash to lookup a submodule config.
Make it clear by naming the variables accrodingly. Looking up a submodule
config by tree hash will come in handy in a later patch.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-submodule-config.txt | 8 ++---
submodule-config.c | 46 ++++++++++++------------
submodule-config.h | 4 +--
t/t7411-submodule-config.sh | 11 ++++++
4 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 941fa178dd..768458580f 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -47,15 +47,15 @@ Functions
Can be passed to the config parsing infrastructure to parse
local (worktree) submodule configurations.
-`const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path)`::
+`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
-`const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const char *name)`::
+`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
The same as above but lookup by name.
-If given the null_sha1 as commit_sha1 the local configuration of a
+If given the null_sha1 as commit_or_tree the local configuration of a
submodule will be returned (e.g. consolidated values from local git
configuration and the .gitmodules file in the worktree).
diff --git a/submodule-config.c b/submodule-config.c
index 15ffab6af4..d88a746c56 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -263,12 +263,12 @@ int parse_push_recurse_submodules_arg(const char *opt, const char *arg)
return parse_push_recurse(opt, arg, 1);
}
-static void warn_multiple_config(const unsigned char *commit_sha1,
+static void warn_multiple_config(const unsigned char *commit_or_tree,
const char *name, const char *option)
{
const char *commit_string = "WORKTREE";
- if (commit_sha1)
- commit_string = sha1_to_hex(commit_sha1);
+ if (commit_or_tree)
+ commit_string = sha1_to_hex(commit_or_tree);
warning("%s:.gitmodules, multiple configurations found for "
"'submodule.%s.%s'. Skipping second one!",
commit_string, name, option);
@@ -276,7 +276,7 @@ static void warn_multiple_config(const unsigned char *commit_sha1,
struct parse_config_parameter {
struct submodule_cache *cache;
- const unsigned char *commit_sha1;
+ const unsigned char *commit_or_tree;
const unsigned char *gitmodules_sha1;
int overwrite;
};
@@ -300,7 +300,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->path)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"path");
else {
if (submodule->path)
@@ -314,7 +314,7 @@ static int parse_config(const char *var, const char *value, void *data)
int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
submodule->fetch_recurse != RECURSE_SUBMODULES_NONE)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"fetchrecursesubmodules");
else
submodule->fetch_recurse = parse_fetch_recurse(
@@ -324,7 +324,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->ignore)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"ignore");
else if (strcmp(value, "untracked") &&
strcmp(value, "dirty") &&
@@ -340,7 +340,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value) {
ret = config_error_nonbool(var);
} else if (!me->overwrite && submodule->url) {
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"url");
} else {
free((void *) submodule->url);
@@ -351,21 +351,21 @@ static int parse_config(const char *var, const char *value, void *data)
ret = config_error_nonbool(var);
else if (!me->overwrite &&
submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"update");
else if (parse_submodule_update_strategy(value,
&submodule->update_strategy) < 0)
die(_("invalid value for %s"), var);
} else if (!strcmp(item.buf, "shallow")) {
if (!me->overwrite && submodule->recommend_shallow != -1)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"shallow");
else
submodule->recommend_shallow =
git_config_bool(var, value);
} else if (!strcmp(item.buf, "branch")) {
if (!me->overwrite && submodule->branch)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"branch");
else {
free((void *)submodule->branch);
@@ -379,18 +379,18 @@ static int parse_config(const char *var, const char *value, void *data)
return ret;
}
-static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+static int gitmodule_sha1_from_commit(const unsigned char *commit_or_tree,
unsigned char *gitmodules_sha1,
struct strbuf *rev)
{
int ret = 0;
- if (is_null_sha1(commit_sha1)) {
+ if (is_null_sha1(commit_or_tree)) {
hashclr(gitmodules_sha1);
return 1;
}
- strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
+ strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_or_tree));
if (get_sha1(rev->buf, gitmodules_sha1) >= 0)
ret = 1;
@@ -402,7 +402,7 @@ static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
* revisions.
*/
static const struct submodule *config_from(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *key,
+ const unsigned char *commit_or_tree, const char *key,
enum lookup_type lookup_type)
{
struct strbuf rev = STRBUF_INIT;
@@ -418,7 +418,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
* return the first submodule. Can be used to check whether
* there are any submodules parsed.
*/
- if (!commit_sha1 || !key) {
+ if (!commit_or_tree || !key) {
struct hashmap_iter iter;
struct submodule_entry *entry;
@@ -428,7 +428,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev))
+ if (!gitmodule_sha1_from_commit(commit_or_tree, sha1, &rev))
goto out;
switch (lookup_type) {
@@ -448,7 +448,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
/* fill the submodule config into the cache */
parameter.cache = cache;
- parameter.commit_sha1 = commit_sha1;
+ parameter.commit_or_tree = commit_or_tree;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf,
@@ -484,7 +484,7 @@ int parse_submodule_config_option(const char *var, const char *value)
{
struct parse_config_parameter parameter;
parameter.cache = &the_submodule_cache;
- parameter.commit_sha1 = NULL;
+ parameter.commit_or_tree = NULL;
parameter.gitmodules_sha1 = null_sha1;
parameter.overwrite = 1;
@@ -492,18 +492,18 @@ int parse_submodule_config_option(const char *var, const char *value)
return parse_config(var, value, ¶meter);
}
-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_name(const unsigned char *commit_or_tree,
const char *name)
{
ensure_cache_init();
- return config_from(&the_submodule_cache, commit_sha1, name, lookup_name);
+ return config_from(&the_submodule_cache, commit_or_tree, name, lookup_name);
}
-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_path(const unsigned char *commit_or_tree,
const char *path)
{
ensure_cache_init();
- return config_from(&the_submodule_cache, commit_sha1, path, lookup_path);
+ return config_from(&the_submodule_cache, commit_or_tree, path, lookup_path);
}
void submodule_free(void)
diff --git a/submodule-config.h b/submodule-config.h
index d05c542d2c..99df8e593c 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -25,9 +25,9 @@ struct submodule {
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
int parse_submodule_config_option(const char *var, const char *value);
-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_name(const unsigned char *commit_or_tree,
const char *name);
-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_path(const unsigned char *commit_or_tree,
const char *path);
void submodule_free(void);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 47562ce465..301ed5e48f 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -93,6 +93,17 @@ test_expect_success 'error message contains blob reference' '
)
'
+test_expect_success 'using tree sha1 works' '
+ (
+ cd super &&
+ tree=$(git rev-parse HEAD^{tree}) &&
+ commit=$(git rev-parse HEAD^{commit}) &&
+ test-submodule-config $commit b >expect &&
+ test-submodule-config $tree b >actual &&
+ test_cmp expect actual
+ )
+'
+
cat >super/expect_url <<EOF
Submodule url: 'git@somewhere.else.net:a.git' for path 'b'
Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv3 1/3] submodule config: inline config_from_{name, path}
From: Stefan Beller @ 2016-11-22 1:35 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jacob.keller, Stefan Beller
In-Reply-To: <20161122013550.1800-1-sbeller@google.com>
There is no other user of config_from_{name, path}, such that there is no
reason for the existence of these one liner functions. Just inline these
to increase readability.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule-config.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 098085be69..15ffab6af4 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -471,18 +471,6 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return submodule;
}
-static const struct submodule *config_from_path(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *path)
-{
- return config_from(cache, commit_sha1, path, lookup_path);
-}
-
-static const struct submodule *config_from_name(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *name)
-{
- return config_from(cache, commit_sha1, name, lookup_name);
-}
-
static void ensure_cache_init(void)
{
if (is_cache_init)
@@ -508,14 +496,14 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name)
{
ensure_cache_init();
- return config_from_name(&the_submodule_cache, commit_sha1, name);
+ return config_from(&the_submodule_cache, commit_sha1, name, lookup_name);
}
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path)
{
ensure_cache_init();
- return config_from_path(&the_submodule_cache, commit_sha1, path);
+ return config_from(&the_submodule_cache, commit_sha1, path, lookup_path);
}
void submodule_free(void)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv3 3/3] submodule-config: clarify parsing of null_sha1 element
From: Stefan Beller @ 2016-11-22 1:35 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jacob.keller, Stefan Beller
In-Reply-To: <20161122013550.1800-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/technical/api-submodule-config.txt | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 768458580f..e06a3fd2de 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -55,8 +55,11 @@ Functions
The same as above but lookup by name.
-If given the null_sha1 as commit_or_tree the local configuration of a
-submodule will be returned (e.g. consolidated values from local git
+Whenever a submodule configuration is parsed in `parse_submodule_config_option`
+via e.g. `gitmodules_config()`, it will be overwrite the null_sha1 entry.
+So in the normal case, when HEAD:.gitmodules is parsed first and then overlayed
+with the repository configuration, the null_sha1 entry contains the local
+configuration of a submodule (e.g. consolidated values from local git
configuration and the .gitmodules file in the worktree).
For an example usage see test-submodule-config.c.
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Stefan Beller @ 2016-11-22 2:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqqy40ch6wp.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 1:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Does this format correctly?
>
> I somehow thought that second and subsequent paragraphs continued
> with "+" want no indentation before them. See for example the
> Values section in config.txt and see how entries for boolean:: and
> color:: use multiple '+' paragraphs.
>
> If we do not have to refrain from indenting the second and
> subsequent paragraphs, that would be great for readability, but I
> take the existing practice as telling me that we cannot do that X-<.
Will fix and test in a resend.
>
>> +test_expect_success 'setup a gitlink with missing .gitmodules entry' '
>> + git init sub2 &&
>> + test_commit -C sub2 first &&
>> + git add sub2 &&
>> + git commit -m superproject
>> +'
>> +
>> +test_expect_success 'intern the git dir fails for incomplete submodules' '
>> + test_must_fail git submodule interngitdirs &&
>> + # check that we did not break the repository:
>> + git status
>> +'
>
> It is not clear what the last "git status" wants to test.
Any errors that I ran into when manually truing to embed a submodules
git dir, were fatal with `git status` already, e.g. missing or wrong
call of connect_work_tree_and_git_dir were my main failure points.
So I guess we should test a bit more extensively, maybe
git status >expect
git submodule embedgitdirs
git status >actual
test_cmp expect actual
# further testing via
test -f ..
test -d ..
> In the
> extreme, if the failed "git submodule" command did
>
> rm -fr .git ?* && git init
>
> wouldn't "git status" still succeed?
In that particular case you'd get
$ git status
fatal: Not a git repository (or any parent up to mount point ....)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
$ echo $?
128
but I get the idea, which is why I propose the double check via status.
That would detect any logical change for the repository, e.g. a
change to the .gitmodules file.
>
> What are the minimum things that we expect from "did not break" to
> see? sub2/.git is still a directory and is a valid repository? The
> contents of the .git/modules/* before and after the "git submodule"
> does not change? Some other things?
I thought about making up a name for such a repo and creating that engry
in .gitmodules. But I refrained from doing so, because it seems too much
for this command.
I dunno, but I would suspect the double status is fine here, too?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox