Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
Hi Elijah, On Tue, 9 Aug 2016, Elijah Newren wrote: > [... detailed review ...] > > So, I think the series looks good. Thank you very much for this quite thorough review! > Sorry that I didn't spot any errors. That's quite alright with me ;-) Thanks again! Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
> > * sb/submodule-update-dot-branch (2016-08-03) 7 commits > (merged to 'next' on 2016-08-04 at 47bff41) > + submodule update: allow '.' for branch value > + submodule--helper: add remote-branch helper > + submodule-config: keep configured branch around > + submodule--helper: fix usage string for relative-path > + submodule update: narrow scope of local variable > + submodule update: respect depth in subsequent fetches > + t7406: future proof tests with hard coded depth > > A few updates to "git submodule update". > > Will merge to 'master'. I think "t7406: future proof tests with hard coded depth" breaks the tests on OSX: https://travis-ci.org/git/git/jobs/150779244 Cheers, Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
> On 10 Aug 2016, at 19:36, Stefan Beller wrote: > > On Wed, Aug 10, 2016 at 10:30 AM, Lars Schneider > wrote: >> >>> >>> * sb/submodule-update-dot-branch (2016-08-03) 7 commits >>> (merged to 'next' on 2016-08-04 at 47bff41) >>> + submodule update: allow '.' for branch value >>> + submodule--helper: add remote-branch helper >>> + submodule-config: keep configured branch around >>> + submodule--helper: fix usage string for relative-path >>> + submodule update: narrow scope of local variable >>> + submodule update: respect depth in subsequent fetches >>> + t7406: future proof tests with hard coded depth >>> >>> A few updates to "git submodule update". >>> >>> Will merge to 'master'. >> >> I think "t7406: future proof tests with hard coded depth" >> breaks the tests on OSX: >> >> https://travis-ci.org/git/git/jobs/150779244 >> >> Cheers, >> Lars >> > > > error: pathspec '4' did not match any file(s) known to git. > > not ok 46 - submodule update clone shallow submodule > > # > # test_when_finished "rm -rf super3" && > # first=$(git -C cloned submodule status submodule |cut -c2-41) && > # second=$(git -C submodule rev-parse HEAD) && > # commit_count=$(git -C submodule rev-list $first^..$second | wc -l) && > # git clone cloned super3 && > # pwd=$(pwd) && > # ( > # cd super3 && > # sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp && > # mv -f .gitmodules.tmp .gitmodules && > # git submodule update --init --depth=$commit_count && > # test 1 = $(git -C submodule log --oneline | wc -l) > # ) > # > > > Is it possible that the "wc -l" produces SP on OSX, > such that the > > # git submodule update --init --depth=$commit_count > > contains "--depth= 4" which means empty depth and 4 as the pathspec > for the update command? Consider this: ~code/git git:(master) ▶ ls | wc -l 747 Apparently `wc -l` adds 5 spaces on OS X... - Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
On Wed, Aug 10, 2016 at 10:30 AM, Lars Schneider wrote: > >> >> * sb/submodule-update-dot-branch (2016-08-03) 7 commits >> (merged to 'next' on 2016-08-04 at 47bff41) >> + submodule update: allow '.' for branch value >> + submodule--helper: add remote-branch helper >> + submodule-config: keep configured branch around >> + submodule--helper: fix usage string for relative-path >> + submodule update: narrow scope of local variable >> + submodule update: respect depth in subsequent fetches >> + t7406: future proof tests with hard coded depth >> >> A few updates to "git submodule update". >> >> Will merge to 'master'. > > I think "t7406: future proof tests with hard coded depth" > breaks the tests on OSX: > > https://travis-ci.org/git/git/jobs/150779244 > > Cheers, > Lars > error: pathspec '4' did not match any file(s) known to git. not ok 46 - submodule update clone shallow submodule # # test_when_finished "rm -rf super3" && # first=$(git -C cloned submodule status submodule |cut -c2-41) && # second=$(git -C submodule rev-parse HEAD) && # commit_count=$(git -C submodule rev-list $first^..$second | wc -l) && # git clone cloned super3 && # pwd=$(pwd) && # ( # cd super3 && # sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp && # mv -f .gitmodules.tmp .gitmodules && # git submodule update --init --depth=$commit_count && # test 1 = $(git -C submodule log --oneline | wc -l) # ) # Is it possible that the "wc -l" produces SP on OSX, such that the # git submodule update --init --depth=$commit_count contains "--depth= 4" which means empty depth and 4 as the pathspec for the update command? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
Lars Schneider writes: > I think "t7406: future proof tests with hard coded depth" > breaks the tests on OSX: > > https://travis-ci.org/git/git/jobs/150779244 Thanks. "| wc -l" can probably be replaced with "--count", if the upstream is "git rev-list". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
Elijah Newren writes: > So, I think the series looks good. Sorry that I didn't spot any errors. That's nothing to be sorry about--it is an excellent news. Thanks for helping. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
Hi,
On Mon, Aug 8, 2016 at 3:32 PM, Junio C Hamano wrote:
> * js/am-3-merge-recursive-direct (2016-08-01) 16 commits
> (merged to 'next' on 2016-08-05 at dc1c9bb)
> + merge-recursive: flush output buffer even when erroring out
> + merge_trees(): ensure that the callers release output buffer
> + merge-recursive: offer an option to retain the output in 'obuf'
> + merge-recursive: write the commit title in one go
> + merge-recursive: flush output buffer before printing error messages
> + am -3: use merge_recursive() directly again
> + merge-recursive: switch to returning errors instead of dying
> + merge-recursive: handle return values indicating errors
> + merge-recursive: allow write_tree_from_memory() to error out
> + merge-recursive: avoid returning a wholesale struct
> + merge_recursive: abort properly upon errors
> + prepare the builtins for a libified merge_recursive()
> + merge-recursive: clarify code in was_tracked()
> + die(_("BUG")): avoid translating bug messages
> + die("bug"): report bugs consistently
> + t5520: verify that `pull --rebase` shows the helpful advice when failing
>
> "git am -3" calls "git merge-recursive" when it needs to fall back
> to a three-way merge; this call has been turned into an internal
> subroutine call instead of spawning a separate subprocess.
>
> Will merge to 'master'.
>
> Eyes from other people are highly appreciated, as my eyes (and the
> original author's, too) have rotten by staring many rerolls of the
> same topic and are not effective in spotting errors.
I gave it a whirl and read over all these patches. I read them first
trying to see if there was any difference in behavior for cases where
the old code wouldn't hit a die(). As far as I can tell, the only
place that could change the non-die() paths was the early return added
when a filed couldn't be removed in remove_file_from_cache (inside the
handle_change_delete() function):
- remove_file_from_cache(path);
- update_file(o, 0, o_oid, o_mode, renamed ? renamed : path);
+ ret = remove_file_from_cache(path);
+ if (!ret)
+ ret = update_file(o, 0, o_oid, o_mode,
+ renamed ? renamed : path);
However, remove_file_from_cache() unconditionally returns 0 and
appears to have done so since it was introduced in 2005. That seems a
bit odd (Is the function just buggy? Should it be transformed into a
void function to make it clear that there's no point checking return
values?) Anyway, even if remove_file_from_cache() was modified to
return a nonzero result when its argument wasn't already in the index,
I believe that wouldn't matter here because path must be in the index
in order to reach this point of the code (if it somehow wasn't in the
index, that probably would have been a die()-worthy condition in the
old code that we just didn't bother checking for).
After my reading, I'm fairly confident that things that worked before
without hitting a die() should have the same behavior after this set
of patches (modulo the intentional changes like output buffering).
I then re-read in another pass for when the code would have hit a
die() previously, and in all those cases Johannes is returning as
early as possible and avoiding falling through to subsequent code,
which is probably the most reasonable thing to do. There might be a
few cases where a few extra steps could be taken, but it's safer to
just return early.
So, I think the series looks good. Sorry that I didn't spot any errors.
Elijah
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
Stefan Beller writes: > On Mon, Aug 8, 2016 at 3:32 PM, Junio C Hamano wrote: >> >> * sb/submodule-clone-rr (2016-08-06) 6 commits >> - clone: reference flag is used for submodules as well >> - submodule update: add super-reference flag >> - 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 >> >> Waiting for review discussion to settle. >> cf. <20160806012318.17968-1-sbel...@google.com> > > I will reroll today with a totally different approach. OK. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #03; Mon, 8)
On Mon, Aug 8, 2016 at 3:32 PM, Junio C Hamano wrote: > > * sb/submodule-clone-rr (2016-08-06) 6 commits > - clone: reference flag is used for submodules as well > - submodule update: add super-reference flag > - 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 > > Waiting for review discussion to settle. > cf. <20160806012318.17968-1-sbel...@google.com> I will reroll today with a totally different approach. No need to further discuss the last 2 patches. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Aug 2016, #03; Mon, 8)
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. Many topics in "Cooking" section without seeing activity have been moved to "Stalled" status and marked as "Will discard". This is unfortunate but with way many people wanting to throw random new topics while too few people able/willing to review them, it is inevitable. On the 'master' front, the individual commit count now exceeds 500 since the last major release, and the early preview -rc0 is expected to happen at the end of the week. The 'maint' branch has been accumulating enough material to make it the next maintenance release v2.9.3, which hopefully happen late this week. 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"] * ab/gitweb-link-html-escape (2016-08-01) 1 commit (merged to 'next' on 2016-08-03 at 44b6088) + gitweb: escape link body in format_ref_marker The characters in the label shown for tags/refs for commits in "gitweb" output are now properly escaped for proper HTML output. * cp/completion-clone-recurse-submodules (2016-07-27) 1 commit (merged to 'next' on 2016-08-03 at cbf0d94) + completion: add option '--recurse-submodules' to 'git clone' * da/subtree-modernize (2016-07-27) 2 commits (merged to 'next' on 2016-08-03 at 06ad015) + subtree: adjust function definitions to match CodingGuidelines + subtree: adjust style to match CodingGuidelines Style fixes for "git subtree" (in contrib/). * ew/build-time-pager-tweaks (2016-08-04) 1 commit (merged to 'next' on 2016-08-05 at 4f0b11b) + pager: move pager-specific setup into the build The build procedure learned PAGER_ENV knob that lists what default environment variable settings to export for popular pagers. This mechanism is used to tweak the default settings to MORE on FreeBSD. * ew/git-svn-http-tests (2016-07-25) 2 commits (merged to 'next' on 2016-08-03 at 2b23920) + git svn: migrate tests to use lib-httpd + t/t91*: do not say how to avoid the tests Tests for "git svn" have been taught to reuse the lib-httpd test infrastructure when testing the subversion integration that interacts with subversion repositories served over the http:// protocol. * ib/t3700-add-chmod-x-updates (2016-08-01) 3 commits (merged to 'next' on 2016-08-03 at 1753346) + t3700: add a test_mode_in_index helper function + t3700: merge two tests into one + t3700: remove unwanted leftover files before running new tests The t3700 test about "add --chmod=-x" have been made a bit more robust and generally cleaned up. * jc/hashmap-doc-init (2016-08-02) 1 commit (merged to 'next' on 2016-08-05 at 2eb946a) + hashmap: clarify that hashmap_entry can safely be discarded The API documentation for hashmap was unclear if hashmap_entry can be safely discarded without any other consideration. State that it is safe to do so. * jh/clean-smudge-f-doc (2016-08-03) 1 commit (merged to 'next' on 2016-08-04 at c2fc8e6) + clarify %f documentation A minor documentation update. This was split out from a stalled jh/clean-smudge-annex topic before discarding it. * jk/difftool-in-subdir (2016-07-28) 3 commits (merged to 'next' on 2016-08-03 at 90f195a) + difftool: use Git::* functions instead of passing around state + difftool: avoid $GIT_DIR and $GIT_WORK_TREE + difftool: fix argument handling in subdirs "git difftool ..." started in a subdirectory failed to interpret the paths relative to that directory, which has been fixed. * jk/pack-objects-optim (2016-07-29) 6 commits (merged to 'next' on 2016-08-03 at ad8caca) + pack-objects: compute local/ignore_pack_keep early + pack-objects: break out of want_object loop early + find_pack_entry: replace last_found_pack with MRU cache + add generic most-recently-used list + sha1_file: drop free_pack_by_name + t/perf: add tests for many-pack scenarios (this branch is used by jk/pack-objects-optim-mru and ks/pack-objects-bitmap.) "git pack-objects" has a few options that tell it not to pack objects found in certain packfiles, which require it to scan .idx files of all available packs. The codepaths involved in these operations have been optimized for a common case of not having any non-local pack and/or any .kept pack. * jk/parseopt-string-list (2016-08-03) 1 commit (merged to 'next' on 2016-08-04 at a7f0cd2) + blame: drop strdup of string literal A small memory leak in the command line parsing of "git blame" has been plugged. * jk/reflog-date (2016-07-27) 7 commits (merged to 'next' on 2016-08-03 at cd8e92b) + date: clarify --date=raw description + date: add "unix" format + date: document and t
