Re: [PATCH v1 0/3] [RFC] Speeding up checkout (and merge, rebase, etc)
On Wed, Jul 25, 2018 at 10:56 PM Ben Peart wrote: > I'm still very new to this part of the code so am trying to figure out > what you're suggesting. I've read your description a few times and what > I'm getting out of it is that with some additional checks (ie verify > it's a twoway_merge, df_conflict_entry, not CE_CONFLICTED) that we > should be able to skip the whole tree similar to how Peff demonstrated > below without having to invalidate the cache tree to reflect modified > on-disk files. Is that correct or am I missing something? And I didn't give you an easy time because I was not very clear in my suggestion, I think. So let's start again. But first let's start with a potentially more generic optimization using cache-tree that I noticed just now. You now know traverse_trees() is used to walk N trees and the index at the same time. Cache tree is also used to quickly check if a big chunk of the index matches some tree object. So what if we try to avoid tree objects if possible (which reduces I/O, object inflation and tree parsing cost)? Let's say we're walking two trees X and Y, then we notice through cache-tree that X is the same in the index. Then instead of walking the actual X, you could just get the same entry from the index and make it "X". This way you only need to walk Y and the index (until the shared tree ends of course). If Y happens to match cache-tree too, all the better! Let's get back to two-way merge. I suggest you read the two-way merge in git-read-tree.txt. That table could give you a pretty good idea what's going on. twoway_merge() will be given a tuple of three entries (I, H, M) of the same path name, for every path. I think what we need is determine the condition where the outcome is known in advance, so that we can just skip walking the index for one directory. One of the checks we could do quickly is I==M or I==H (using cache-tree) and H==M (using tree hash). The first obvious cases that we can optimize are clean (H==M) -- 14 yes exists exists keep index 15 no exists exists keep index In other words if we know H==M, there's no much we need to do since we're keeping the index the same. But you don't really know how many entries are in this directory where H==M. You would need cache-tree for that, so in reality it's I==H==M. The "clean" column is what fsmonitor comes in, though I'm not sure if it's actually needed. I haven't checked how '-u' flag works. There's two other cases that we can also optimize, though I think it's less likely to happen: clean I==H I==M (H!=M) -- 18 yes noyes exists exists keep index 19 nonoyes exists exists keep index Some other cases where I==H can benefit from the generic tree walk optimization above since we can skip parsing H. -- Duy
Fwd: Git Help !!!
Hi, I am unable to use Git (version 2.18 latest). Since i couldnt find any help/support/contact email on https://git-scm.com and i couldnt find the solution using Google as well i am directly contacting you as i need to get git working. Hope you can help me. please find attached an image file regarding my problem kind regards vishwas
Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack
On Wed, Jul 25, 2018 at 08:41:31PM +0200, SZEDER Gábor wrote: > On Wed, Jul 25, 2018 at 4:51 PM Max Kirillov wrote: >>> I just happened to stumble upon a failure because of 'fatal: the >>> remote end hung up unexpectedly' in the test 'push plain'. >> >> Did it happen once or repeated? It is rather strange, that >> one shoud not fail. Which OS it was? > > Only once, so far. It was one of my OSX build jobs on Travis CI, but > I don't know what OSX version is used. > > 'act.err' contained this (which will get line-wrapped, I'm afraid): > > ++handler_type=receive > ++shift > ++env CONTENT_TYPE=application/x-git-receive-pack-request > QUERY_STRING=/repo.git/git-receive-pack > 'PATH_TRANSLATED=/Users/travis/t/trash > dir.t5562/.git/git-receive-pack' GIT_HTTP_EXPORT_ALL=TRUE > REQUEST_METHOD=POST > /Users/travis/build/szeder/git-cooking-topics-for-travis-ci/t/t5562/invoke-with-content-length.pl > push_body git http-backend > <...128 zero bytes...>fatal: the remote end hung up unexpectedly > > I couldn't reproduce it on my Linux box. The only reason for this I could imagine is some perl utility failure to feed the body to git http-backend. I could not reproduce it either, but if such things happen often again maybe should concider C helper instead. Though I'm afraid I easily can make more mistakes in it than perl interpreter authors. I'll make the other changes, and sofar just hope it would not happen again.
Re: [PATCH] git-rebase--interactive.sh: Remove superfluous tab in rebase
Daniel Baumann writes: > On 07/25/2018 12:54 PM, Ævar Arnfjörð Bjarmason wrote: >> The code you're modifying doesn't exist in the "pu" branch since >> 249d626f2c > > indeed, thanks. > >> From looking at it it seems we no longer have this problem, >> but perhaps you'd like to check that out for yourself? > Yes, it's not needed anymore at all, yay! > > Sorry for the noise. Thanks for reporting. It is 100% better to see occasional duplicates than nothing.
Re: What's cooking in git.git (Jul 2018, #03; Wed, 25)
Junio C Hamano writes: > Stefan Beller writes: > >>> * ag/rebase-i-in-c (2018-07-10) 13 commits >> [...] >>> >>> Piecemeal rewrite of the remaining "rebase -i" machinery in C. >>> >>> A reroll (which is rumored to be quite good) exists, but hasn't >>> been picked up yet. >> >> Forgot to state so on either the mailing list (or Github or IRC), >> but I read the tip of the reroll and I think it is good. > > No, you did not forget (otherwise I wouldn't have said anything > about the rumor). Skimming "git diff master..." is not a review > X-<. I was reading the early part of the series and all updates from the previous round looked OK. I haven't had enough time to spend on the new stuff at the end.
Re: What's cooking in git.git (Jul 2018, #03; Wed, 25)
Junio C Hamano writes: > Stefan Beller writes: > >>> * js/range-diff (2018-07-25) 21 commits > > The non-coloring part, like patch matching and driving diff over a > pair of "git show" output, looked reasonably solid when I read it > (even though I admit I did not audit for leaks). > > The only thing I think we would be better off without is the > coloring of whitespace errors (at least in the current shape). I > cannot shake the feeling that temporarily setting core.whitespace to > nothing (i.e. we do not detect any whitespace errors and hence we do > not show any) while running diff-of-diff may be a workaround that is > less damaging to the code base than piling band-aid on the codepath > that is shared with plain diff (not diff-of-diff). Having said all that, after re-reading the patches, I think the series is more or less solid already, so I am hoping that the next issue of "What's cooking" report can mark it as "Will merge to 'next'". Although I am not sure if an update for replacing "oops" will have enough time to come in before that happens, I do not think anything in this series is so seriously broken that would result in embarrassing history littered with incremental fix-up.
Re: [PATCH] git-rebase--interactive.sh: Remove superfluous tab in rebase
Hi, On 07/25/2018 12:54 PM, Ævar Arnfjörð Bjarmason wrote: > The code you're modifying doesn't exist in the "pu" branch since > 249d626f2c indeed, thanks. > From looking at it it seems we no longer have this problem, > but perhaps you'd like to check that out for yourself? Yes, it's not needed anymore at all, yay! Sorry for the noise. Regards, Daniel
Re: [PATCH 1/1] add hook pre-p4-submit
Thanks for the review.
The hook does not receive any information or input from git. The
original requirement
comes from my colleague. He want to run unit test automatically before
submitting code
to remote repository. Or else CI server will send the blame mail to the manager.
The hook actually stops the submit process from start instead of abort
submit in midway.
So nothing is touched when hook exits with status 1.
I'm not sure whether `git-p4` should print some "hook rejection" message.
Current implementation is same as other hooks (`pre-commit`, for example).
Only hook itself is responsible to print error messages.
Personally I don't have opinion whether we should print out hook
related message inside
`git-p4.py`. I just try to following existing convention of git.
What you guys think?
Regards,
Chen
On Thu, Jul 26, 2018 at 6:00 AM, Eric Sunshine wrote:
> On Wed, Jul 25, 2018 at 9:45 AM Chen Bin wrote:
>> Hook pre-p4-submit is executed before git-p4 actually submits code.
>> If the hook exits with non-zero value, submit process will abort.
>
> Bikeshedding: Should this be named 'p4-pre-submit'? That way, if
> git-p4 ever grows additional hooks, they can all be grouped under the
> "p4-" prefix.
>
> More below...
>
>> Signed-off-by: Chen Bin
>> ---
>> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
>> @@ -374,6 +374,13 @@ These options can be used to modify 'git p4 submit'
>> behavior.
>> +Hook for submit
>> +~~~
>> +Hook `pre-p4-submit` is executed if it exists and is executable.
>> +Exiting with non-zero status from this script cause `git-p4 submit` to
>> abort.
>> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be changed.
>
> Does the hook receive any arguments? Does it receive anything on its
> standard input? Those questions ought to be answered by the
> documentation.
>
> Also, how is the hook supposed to determine whether the "submit"
> should proceed? How does it know what is being submitted? Perhaps the
> documentation could provide some explanation of how the hook should
> glean such information (especially if the hook does not itself receive
> any input).
>
>> diff --git a/git-p4.py b/git-p4.py
>> +# locate hook
>> +hooks_path = gitConfig("core.hooksPath")
>> +if len(hooks_path) > 0:
>> +hook_file = os.path.join(hooks_path, "pre-p4-submit")
>> +else:
>> +hook_file = os.path.join(read_pipe("git rev-parse
>> --git-dir").strip(), "hooks", "pre-p4-submit")
>> +
>> +# Execute hook. If it exits with non-zero value, do NOT continue.
>> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
>> subprocess.call([hook_file]) != 0:
>> +sys.exit(1)
>
> Nit: The two #comments merely repeat what the code itself already says
> clearly enough, thus don't add value (so can be dropped).
>
> Should this be emitting some sort of message letting the user know
> that the operation was aborted due to the hook's "rejection"? That
> could be especially important if the hook itself is silent (or buggy).
>
>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
>> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should
>> display error' '
>> +# Test following scenarios:
>> +# - Without hook ".git/hooks/pre-p4-submit", submit should continue
>> +# - With hook returning 0, submit should continue
>> +# - With hook returning 1, submit should abort
>
> Testing three separate cases would normally be done with three
> separate tests, making it easier to figure out which case failed. But,
> perhaps it doesn't matter here.
>
>> +test_expect_success 'run hook pre-p4-submit before submit' '
>> + test_when_finished cleanup_git &&
>> + git p4 clone --dest="$git" //depot &&
>> + (
>> + cd "$git" &&
>> + echo "hello world" >hello.txt &&
>> + git add hello.txt &&
>> + git commit -m "add hello.txt" &&
>> + git config git-p4.skipSubmitEdit true &&
>
> The wholesale removal of the $git directory by cleanup_git() when the
> test finishes is undoing this config setting. Ditto regarding the
> hooks created below. Okay.
>
>> + git p4 submit --dry-run | grep "Would apply" &&
>
> We normally don't use a git command upstream in a pipe since the exit
> code of that command gets lost (so, if it crashes, we don't know about
> it). Instead:
>
> git p4 submit --dry-run >out &&
> grep "Would apply" out &&
>
> Also, if "Would apply" is localized, we'd use test_i18ngrep (but you
> don't need it in this case).
>
>> + mkdir -p .git/hooks &&
>> + : >.git/hooks/pre-p4-submit && chmod +x
>> .git/hooks/pre-p4-submit &&
>
> It might be clearer to readers of this test to use an explicit "exit
> 0" here (complementing the "exit 1" below). The current asymmetry
> required extra cognitive effort to realize that this is the "returning
>
Re: [PATCH v2] packfile: ensure that enum object_type is defined
Beat Bolli wrote: > When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with > "make DEVELOPER=1 DEVOPTS=pedantic", the compiler says > > error: redeclaration of already-defined enum 'object_type' is a GNU > extension [-Werror,-Wgnu-redeclared-enum] > > According to https://en.cppreference.com/w/c/language/declarations > (section "Redeclaration"), a repeated declaration after the definition > is only legal for structs and unions, but not for enums. > > Drop the belated declaration of enum object_type and include cache.h > instead to make sure the enum is defined. > > Helped-by: Jonathan Nieder > Signed-off-by: Beat Bolli > --- > packfile.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks! I had run into this using clang on Linux, too, but hadn't spent the time to track it down and write a patch. Running git grep -e 'enum [^ ]*;' doesn't find any other instances of this error. Thanks for fixing it. Reviewed-by: Jonathan Nieder By the way, not about this patch: [...] > +++ b/packfile.h > @@ -1,12 +1,12 @@ > #ifndef PACKFILE_H > #define PACKFILE_H > > +#include "cache.h" > #include "oidset.h" > > /* in object-store.h */ > struct packed_git; > struct object_info; > -enum object_type; This '/* in object-store.h */' comment can easily go stale since nothing enforces that it stays accurate. I don't think it's a useful comment to have anyway, since it's straightforward to grep for where the struct is defined. I think we should remove the comment. Thanks, Jonathan
Re: What's cooking in git.git (Jul 2018, #03; Wed, 25)
Stefan Beller writes: >> * ag/rebase-i-in-c (2018-07-10) 13 commits > [...] >> >> Piecemeal rewrite of the remaining "rebase -i" machinery in C. >> >> A reroll (which is rumored to be quite good) exists, but hasn't >> been picked up yet. > > Forgot to state so on either the mailing list (or Github or IRC), > but I read the tip of the reroll and I think it is good. No, you did not forget (otherwise I wouldn't have said anything about the rumor). Skimming "git diff master..." is not a review X-<.
Re: What's cooking in git.git (Jul 2018, #03; Wed, 25)
Stefan Beller writes: >> * jh/structured-logging (2018-07-25) 25 commits > [...] >> - structured-logging: design document >> (this branch uses jh/json-writer.) >> >> X-Gah. > > I am not sure what to make of this comment? That's just a line/editor noise. > Does it need review or is the design/intent to be > discussed? The topic certainy needs serious reviews and response with updated patches (when reviews come, that is), I think. I also agree with somebody who said slog is not a good way to present it in the doc that are to be used by non git developers.
Re: What's cooking in git.git (Jul 2018, #03; Wed, 25)
Stefan Beller writes: >> * js/range-diff (2018-07-25) 21 commits > I think the current coloring is good enough to ship, but it still has > errors around corners, for example introduction of new files, > having lines in the inner diff as: > > diff --git a/Makefile b/Makefile > --- a/Makefile > +++ b/Makefile > > will be colored white/red/green (in that order), but in the outer diff > these are all "context", but as these specific context lines happen > to start with +/- we color them. > If we want to be perfect, we rather need to parse > the inner diff on a more detailed level, but I would argue to leave > that to a later stage for another volunteer to step in and cleanup. I think the primary part of coloring i.e. "white is common, green is added, red is removed" together with "bold is new, dimmed is old" is quite usable and not broken. The non-coloring part, like patch matching and driving diff over a pair of "git show" output, looked reasonably solid when I read it (even though I admit I did not audit for leaks). The only thing I think we would be better off without is the coloring of whitespace errors (at least in the current shape). I cannot shake the feeling that temporarily setting core.whitespace to nothing (i.e. we do not detect any whitespace errors and hence we do not show any) while running diff-of-diff may be a workaround that is less damaging to the code base than piling band-aid on the codepath that is shared with plain diff (not diff-of-diff).
Re: [PATCH 2/2] doc hash-function-transition: pick SHA-256 as NewHash
On Wed, Jul 25, 2018 at 09:45:52AM -0700, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > > > @@ -125,19 +122,19 @@ Detailed Design > > --- > > Repository format extension > > ~~~ > > -A NewHash repository uses repository format version `1` (see > > +A SHA-256 repository uses repository format version `1` (see > > Documentation/technical/repository-version.txt) with extensions > > `objectFormat` and `compatObjectFormat`: > > > > [core] > > repositoryFormatVersion = 1 > > [extensions] > > - objectFormat = newhash > > + objectFormat = sha256 > > compatObjectFormat = sha1 > > Whenever we said SHA1, somebody came and told us that the name of > the hash is SHA-1 (with dash). Would we be nitpicker-prone in the > same way with "sha256" here? I actually have a patch to make the names "sha1" and "sha256". My rationale is that it's shorter and easier to type. People can quibble about it when I send it to the list, but that's what I'm proposing at least. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: What's cooking in git.git (Jul 2018, #03; Wed, 25)
On Wed, Jul 25, 2018 at 3:13 PM Junio C Hamano wrote: > > 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 have moved to 'master' and 'next' from 'next' to 'pu' > respectively. > > You can find the changes described here in the integration branches > of the repositories listed at (using an alias to fetch all using protocol v2) Fetching o >From https://github.com/gitster/git - [deleted] (none) -> origin/ab/fetch-tags-noclobber - [deleted] (none) -> origin/nd/use-the-index-compat-less - [deleted] (none) -> origin/sb/diff-color-move remote: Counting objects: 830, done. remote: Compressing objects: 100% (379/379), done. remote: Total 830 (delta 404), reused 777 (delta 404), pack-reused 47 Receiving objects: 100% (830/830), 470.12 KiB | 9.04 MiB/s, done. Resolving deltas: 100% (404/404), completed with 58 local objects. fatal: Couldn't find remote ref refs/notes/amlog error: Could not fetch o ... (goes and pastes this into the email, what if I try again?) $ git -c protocol.version=0 fetch --all Fetching o >From https://github.com/gitster/git * [new branch] ab/newhash-is-sha256 -> origin/ab/newhash-is-sha256 + 6da893ab657...729b3925ed9 bb/make-developer-pedantic -> origin/bb/make-developer-pedantic (forced update) * [new branch] bb/redecl-enum-fix -> origin/bb/redecl-enum-fix * [new branch] es/diff-color-move-fix -> origin/es/diff-color-move-fix + 165e30f8529...d63a0b5e393 es/format-patch-rangediff -> origin/es/format-patch-rangediff (forced update) + 1ac17a46fab...bb4a134ae84 jch-> origin/jch (forced update) * [new branch] jh/structured-logging -> origin/jh/structured-logging + 816325b2109...c255a588bcd js/range-diff -> origin/js/range-diff (forced update) b7bd9486b05..ffc6fa0e396 master -> origin/master 5c9ce644c39..a71716f1ade next -> origin/next + 8a589a4fc91...838143aa5c0 pu -> origin/pu (forced update) + 0041456f5f1...16d09ff91a2 refs/notes/amlog -> refs/notes/amlog (forced update) Fetching googlesource ... > * jh/structured-logging (2018-07-25) 25 commits [...] > - structured-logging: design document > (this branch uses jh/json-writer.) > > X-Gah. I am not sure what to make of this comment? Does it need review or is the design/intent to be discussed? > [Cooking] [...] > * sb/submodule-update-in-c (2018-07-18) 6 commits > - submodule--helper: introduce new update-module-mode helper > - builtin/submodule--helper: factor out method to update a single submodule > - builtin/submodule--helper: store update_clone information in a struct > - builtin/submodule--helper: factor out submodule updating > - git-submodule.sh: rename unused variables > - git-submodule.sh: align error reporting for update mode to use path > > "git submodule update" is getting rewritten piece-by-piece into C. > > It seems to pass its own self-tests standalone, but seems to break > horribly when merged to 'pu'. I need to redo this, please eject from pu if that is easier for us. > * ag/rebase-i-in-c (2018-07-10) 13 commits [...] > > Piecemeal rewrite of the remaining "rebase -i" machinery in C. > > A reroll (which is rumored to be quite good) exists, but hasn't > been picked up yet. Forgot to state so on either the mailing list (or Github or IRC), but I read the tip of the reroll and I think it is good. > * sb/object-store-lookup (2018-06-29) 33 commits [...] > > Will merge to 'master'. Finally. Hooray! I am currently writing its successor in a "less confrontational (but needing more work)"-kind-of way, which converts the ref store to take repository objects. Given this series (and how it was done/reviewed and yet caused so much trouble), I'd like to spark a discussion on how the community wants to see large scale refactorings and eventually document it. > * js/range-diff (2018-07-25) 21 commits [...] > (this branch is used by es/format-patch-rangediff.) > > "git tbdiff" that lets us compare individual patches in two > iterations of a topic has been rewritten and made into a built-in > command. > > Undecided. > > Many "The feature is useful" comments without much real review; we > know the feature is great as this mimicks tbdiff already so that is > not news. It has also seen reviews regarding its non-coloring side in previous rounds, which I think is mature by now. I suggested to submit the range-diff without coloring on IRC, but it was not quite well received as colors were seen as "the main benefit of range-diff" by DScho. (http://colabti.org/irclogger/irclogger_log/git-devel?date=2018-07-25#l72)
Re: [GSoC][PATCH v4 00/20] rebase -i: rewrite in C
Alban Gruin writes:
> This patch series rewrite the interactive rebase from shell to C.
>
> It is based on master (as of 2018-07-24).
With fewer number of bytes than "as of ", you can express it in
a more useful way, even when you and others live in different
timezones and when the 'master' branch is updated more than once in
the 24-hour period.
I think you based it on b7bd9486 ("Third batch for 2.19 cycle",
2018-07-18), so you can say so
It is based on b7bd9486 ("Third batch for 2.19 cycle",
2018-07-18).
with a bit more bytes for the subject and be more informative.
When you rebase your series, please explain why you needed to do so
in your cover letter. "I just wanted to catch up with the latest
'master'" is not a good enough reason. "I wanted to make sure my
series works well with X and Y that the base of my previous attempt
did not have." is a very good reason.
I think the rebase this round is warranted, if it was done to take
advantage of the fix made by Elijah to the code that handles the
"strategy" option in 0060041d ("Fix use of strategy options with
interactive rebases", 2018-06-27), in other words, you wanted to
start from the version of read_strategy_opts() Elijah fixed when
you split parse_strategy_opts() out of it in step [15/20].
I'll send comments on individual patches separately.
Thanks.
[no subject]
[no subject]
What's cooking in git.git (Jul 2018, #03; Wed, 25)
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 have moved to 'master' and 'next' from 'next' to 'pu' respectively. 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"] * ag/rebase-p (2018-07-06) 1 commit (merged to 'next' on 2018-07-18 at c36ebba99b) + git-rebase--preserve-merges: fix formatting of todo help message The help message shown in the editor to edit todo list in "rebase -p" has regressed recently, which has been corrected. * as/sequencer-customizable-comment-char (2018-07-16) 1 commit (merged to 'next' on 2018-07-18 at 4163e23f29) + sequencer: use configured comment character Honor core.commentchar when preparing the list of commits to replay in "rebase -i". * bb/pedantic (2018-07-09) 8 commits (merged to 'next' on 2018-07-18 at e9d075e8ed) + utf8.c: avoid char overflow + string-list.c: avoid conversion from void * to function pointer + sequencer.c: avoid empty statements at top level + convert.c: replace "\e" escapes with "\033". + fixup! refs/refs-internal.h: avoid forward declaration of an enum + refs/refs-internal.h: avoid forward declaration of an enum + fixup! connect.h: avoid forward declaration of an enum + connect.h: avoid forward declaration of an enum (this branch is used by bb/make-developer-pedantic.) The codebase has been updated to compile cleanly with -pedantic option. * bb/unicode-11-width (2018-07-09) 1 commit (merged to 'next' on 2018-07-18 at 075648ed37) + unicode: update the width tables to Unicode 11 The character display width table has been updated to match the latest Unicode standard. * bc/send-email-auto-cte (2018-07-09) 4 commits (merged to 'next' on 2018-07-18 at d16c2a301a) + docs: correct RFC specifying email line length + send-email: automatically determine transfer-encoding + send-email: accept long lines with suitable transfer encoding + send-email: add an auto option for transfer encoding The content-transfer-encoding of the message "git send-email" sends out by default was 8bit, which can cause trouble when there is an overlong line to bust RFC 5322/2822 limit. A new option 'auto' to automatically switch to quoted-printable when there is such a line in the payload has been introduced and is made the default. * bp/log-ref-write-fd-with-strbuf (2018-07-10) 1 commit (merged to 'next' on 2018-07-18 at 25a3a99528) + convert log_ref_write_fd() to use strbuf Code clean-up. * bw/ref-in-want (2018-06-28) 8 commits (merged to 'next' on 2018-07-18 at 7e9f8db37c) + fetch-pack: implement ref-in-want + fetch-pack: put shallow info in output parameter + fetch: refactor to make function args narrower + fetch: refactor fetch_refs into two functions + fetch: refactor the population of peer ref OIDs + upload-pack: test negotiation with changing repository + upload-pack: implement ref-in-want + test-pkt-line: add unpack-sideband subcommand (this branch is used by bw/fetch-pack-i18n, jt/connectivity-check-after-unshallow, jt/partial-clone-fsck-connectivity and jt/tags-to-promised-blobs-fix.) Protocol v2 has been updated to allow slightly out-of-sync set of servers to work together to serve a single client, which would be useful with load-balanced servers that talk smart HTTP transport. * en/apply-comment-fix (2018-06-28) 1 commit (merged to 'next' on 2018-07-18 at 31d818f17d) + apply: fix grammar error in comment * en/rebase-consistency (2018-06-27) 9 commits (merged to 'next' on 2018-07-18 at d597206c79) + git-rebase: make --allow-empty-message the default + t3401: add directory rename testcases for rebase and am + git-rebase.txt: document behavioral differences between modes + directory-rename-detection.txt: technical docs on abilities and limitations + git-rebase.txt: address confusion between --no-ff vs --force-rebase + git-rebase: error out when incompatible options passed + t3422: new testcases for checking when incompatible options passed + git-rebase.sh: update help messages a bit + git-rebase.txt: document incompatible options "git rebase" behaved slightly differently depending on which one of the three backends gets used; this has been documented and an effort to make them more uniform has begun. * en/t5407-rebase-m-fix (2018-06-28) 1 commit (merged to 'next' on 2018-07-18 at 459875daeb) + t5407: fix test to cover intended arguments * es/test-lint-one-shot-export (2018-07-16) 5 commits (merged to 'next' on 2018-07-18 at 26a6124963) + t/check-non-portable-shell: detect "FOO=bar shell_func" + t/check-non-portable-shell: make error messages more compact
[PATCH v2] packfile: ensure that enum object_type is defined
When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with "make DEVELOPER=1 DEVOPTS=pedantic", the compiler says error: redeclaration of already-defined enum 'object_type' is a GNU extension [-Werror,-Wgnu-redeclared-enum] According to https://en.cppreference.com/w/c/language/declarations (section "Redeclaration"), a repeated declaration after the definition is only legal for structs and unions, but not for enums. Drop the belated declaration of enum object_type and include cache.h instead to make sure the enum is defined. Helped-by: Jonathan Nieder Signed-off-by: Beat Bolli --- packfile.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packfile.h b/packfile.h index 51383774ec72..28318c5c7c42 100644 --- a/packfile.h +++ b/packfile.h @@ -1,12 +1,12 @@ #ifndef PACKFILE_H #define PACKFILE_H +#include "cache.h" #include "oidset.h" /* in object-store.h */ struct packed_git; struct object_info; -enum object_type; /* * Generate the filename to be used for a pack file with checksum "sha1" and -- 2.18.0
[PATCH v2 0/1] Pedantic fix for Apple clang
Following up on my previous series bb/pedantic for gcc, this is a fix for pedantic compilation under MacOS 10.13.6 (High Sierra) with the command line tools of Xcode Version 9.4.1 (9F2000). Changes against v1: - [1/2]: include cache.h in packfile.h. - [2/2]: drop it. Christian Couder is going to include this in the next version of the cc/remote-odb topic. Beat Bolli (1): packfile: drop a repeated enum declaration packfile.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.18.0
Re: [PATCH 2/2] doc hash-function-transition: pick SHA-256 as NewHash
Jonathan Nieder writes:
> Regardless of how we spell it in prose, I think `sha256` as an
> identifier in configuration is the spelling people will expect. For
> example, gpg ("gpg --version") calls it SHA256.
OK.
> For what it's worth, I would be in favor of modifying the section
> more heavily. For example:
> ...
> Changes:
>
> - retitled since the hash function has already been selected
> - added some notes about sha1dc
> - when discussing wide implementation availability, mentioned
> CommonCrypto too, as an example of a non-OpenSSL library that the
> libgit2 authors care about
> - named which function is chosen
>
> We could put the runners up in the "alternatives considered" section,
> but I don't think there's much to say about them here so I wouldn't.
All interesting ideas and good suggestions. I'll leave 2/2 in the
mail archive and take only 1/2 for now. I'd expect the final
version, not too soon after mulling over the suggestions raised
here, but not in too distant future to prevent us from forgetting
;-)
Thanks.
Re: [PATCH 2/2] remote-odb: un-inline function remote_odb_reinit
Christian Couder writes: > Hi, > > On Wed, Jul 25, 2018 at 12:03 AM, Beat Bolli wrote: >> >> On 24.07.18 23:59, Jonathan Nieder wrote: >>> >>> Beat Bolli wrote: > -inline void remote_odb_reinit(void) +void remote_odb_reinit(void) >>> >>> This looks like an oversight in >>> https://public-inbox.org/git/20180713174959.16748-6-chrisc...@tuxfamily.org/: >>> there isn't any reason for this function to be inline. >>> >>> Christian, can you squash it in on your next reroll? >> >> That would probably make sense. I didn't check how mature the topics >> were that caused errors. > > Ok, it is in the next version I will send. OK, then I'll ignore this one for now.
Re: [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch
Eric Sunshine writes:
> @@ -750,6 +751,20 @@ void show_log(struct rev_info *opt)
>
> memcpy(_queued_diff, , sizeof(diff_queued_diff));
> }
> +
> + if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
> + struct diff_queue_struct dq;
> +
> + memcpy(, _queued_diff, sizeof(diff_queued_diff));
> + DIFF_QUEUE_CLEAR(_queued_diff);
> +
> + next_commentary_block(opt, NULL);
> + fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
> + show_range_diff(opt->rdiff1, opt->rdiff2,
> + opt->creation_factor, 1, >diffopt);
> +
> + memcpy(_queued_diff, , sizeof(diff_queued_diff));
> + }
> }
>
> int log_tree_diff_flush(struct rev_info *opt)
This essentially repeats what is already done for "interdiff".
Does the global diff_queued_diff gets cleaned up when
show_interdiff() and show_range_diff() return, like diff_flush()
does? Otherwise we'd be leaking the filepairs accumulated in the
diff_queued_diff.
Re: [PATCH v1 0/3] [RFC] Speeding up checkout (and merge, rebase, etc)
On 7/24/2018 11:33 AM, Duy Nguyen wrote: On Tue, Jul 24, 2018 at 6:20 AM Jeff King wrote: At least that's my view of it. unpack_trees() has always been a terrifying beast that I've avoided looking too closely at. /me nods on the terrifying part. After a quick look at the code, the only place I can find that tries to use cache_tree_matches_traversal() is in unpack_callback() and that only happens if n == 1 and in the "git checkout" case, n == 2. Am I missing something? So we do not actually use cache-tree? Big optimization opportunity (if we can make it!). I agree! Assuming we can figure out the technical issues around using the cache tree to optimize two way merges, another question I'm trying to answer is how we can enable this optimization without causing back compat issues? We're discussing detecting that there are no changes for parts of the tree between two commits but that isn't the only thing that can trigger changes to be made to the index entries and working directory. Changes can come from other inputs as well. One example I am aware of is sparse-checkout. If you made changes to your sparse checkout settings or $GIT_DIR/info/sparse-checkout file, that could trigger the need to update index entries and files in the working directory. Since that is a relatively rare occurrence, I can see detecting changes to those settings/file and bypassing the optimization if there have been changes. But are there other cases of things that could cause unexpected changes in behavior? One thought I had was to put the optimization behind a config setting so that people had to opt-in to the difference in behavior. I submitted a canary patch [1] to test out how receptive people would be to that idea. Hopefully I can get some feedback on that aspect of the patch. [1] https://public-inbox.org/git/ab8ee481-54fa-a014-69d9-8f621b136...@gmail.com/T/#m2a425a23df5e064a79b0a72537a5dd6ccba3b07b Looks like it's trying to special-case "diff-index --cached". Which kind-of makes sense. In the non-cached case, we're thinking not only about the relationship between the index and the tree, but also whether the on-disk files are up to date. And that would be the same for checkout. We want to know not only whether there are changes to make to the index, but also whether the on-disk files need to be updated from the index. But I assume in your case that we've just refreshed the index quickly using fsmonitor. So I think in the long run what you want is: 1. fsmonitor tells us which index entries are not clean 2. based on the unclean list, we invalidate cache-tree entries for those paths 3. if we have a valid cache-tree entry, we should be able to skip digging into that tree; if not, then we walk the index and tree as normal, adding/deleting index entries and updating (or complaining about) modified on-disk files If you tie this optimization to twoway_merge specifically (by checking "fn" field), then I think we can do it even better. Since cache_tree_matches_traversal() is one (hopefully not too costly) lookup, we can do it without checking with fsmonitor or whatever and only do so when we have found a cache tree. Then if we write this new special code just for twoway_merge, we need to tighten the checks a bit. I think in this case twoway_merge() will be called with "oldtree" as same as "newtree" (and "current" may contains dirty stuff from the index). Then - o->df_conflict_entry should be NULL (because we handle it slightly differently in twoway_merge) - "current" should not have CE_CONFLICTED then I believe we will fall into case /* 20 or 21 */ where merged_entry() is suppoed to be called on all entries and it would change nothing in the index since newtree is the same as oldtree, and we could just jump over the whole tree in traverse_trees(). I'm fine with tying specific optimizations to twoway_merge as that is a very common (if not the most common) merge. I'm still very new to this part of the code so am trying to figure out what you're suggesting. I've read your description a few times and what I'm getting out of it is that with some additional checks (ie verify it's a twoway_merge, df_conflict_entry, not CE_CONFLICTED) that we should be able to skip the whole tree similar to how Peff demonstrated below without having to invalidate the cache tree to reflect modified on-disk files. Is that correct or am I missing something? I think the "n" adds an extra layer of complexity. n==2 means we're doing a "2-way" merge. Moving from tree X to tree Y, and dealing with the index as we go. Naively I _think_ we'd be OK to just extend the rule to "if both subtrees match each other _and_ match the valid cache-tree, then we can skip". Again, I'm a little out of my area of expertise here, but cargo-culting like this: diff --git a/sha1-file.c b/sha1-file.c index de4839e634..c105af70ce 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -1375,6
Re: [PATCH 11/14] format-patch: extend --range-diff to accept revision range
Eric Sunshine writes:
> When submitting a revised a patch series, the --range-diff option embeds
> a range-diff in the cover letter showing changes since the previous
> version of the patch series. The argument to --range-diff is a simple
> revision naming the tip of the previous series, which works fine if the
> previous and current versions of the patch series share a common base.
>
> However, it fails if the revision ranges of the old and new versions of
> the series are disjoint. To address this shortcoming, extend
> --range-diff to also accept an explicit revision range for the previous
> series. For example:
>
> git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2
>
> Signed-off-by: Eric Sunshine
> ---
Makes sense. Even though a single "common rev" would serve as a
feature to discourage rebasing done "just to catch up" without a
good reason, it is a good idea to give an escape hatch like this
to support a case where rebasing is the right thing to do.
> static void infer_range_diff_ranges(struct strbuf *r1,
> struct strbuf *r2,
> const char *prev,
> + struct commit *origin,
> struct commit *head)
> {
> const char *head_oid = oid_to_hex(>object.oid);
>
> - strbuf_addf(r1, "%s..%s", head_oid, prev);
> - strbuf_addf(r2, "%s..%s", prev, head_oid);
I thought "git range-diff" also took the three-dot notation as a
short-hand but there is no point using that in this application,
which wants the RHS and the LHS range in separate output variables.
> + if (!strstr(prev, "..")) {
> + strbuf_addf(r1, "%s..%s", head_oid, prev);
> + strbuf_addf(r2, "%s..%s", prev, head_oid);
> + } else if (!origin) {
> + die(_("failed to infer range-diff ranges"));
> + } else {
> + strbuf_addstr(r1, prev);
> + strbuf_addf(r2, "%s..%s",
> + oid_to_hex(>object.oid), head_oid);
Interesting.
I actually would feel better to see the second range for the normal
case to be computed exactly the same way, i.e.
int prev_is_range = strstr(prev, "..");
if (prev_is_range)
strbuf_addstr(r1, prev);
else
strbuf_addf(r1, "%s..%s", head_oid, prev);
if (origin)
strbuf_addf(r2, "%s..%s", oid_to_hex(>object.oid,
head_oid);
else if (prev_is_range)
die("cannot figure out the origin of new series");
else {
warning("falling back to use '%s' as the origin of new series",
prev);
strbuf_addf(r2, "%s..%s", prev, head_oid);
}
because origin..HEAD is always the set of the "new" series, no
matter what "prev" the user chooses to compare that series against,
when there is a single "origin".
Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter
Eric Sunshine writes:
> I did consider other approaches, such as a more generic option. For
> example, --embed=range-diff:. I also considered supporting more
> complex use-cases. For instance, git-range-diff itself accepts the two
> ranges in several formats ("a..b c..d" or "x...z" or "i j k"), plus
> the creation-factor can be tweaked, so I weighed ways of incorporating
> all that detail into the single argument to --range-diff.
>
> However, those are all difficult to use (onerous to type and remember)
> and difficult to document. Thus, I opted for simplicity of individual,
> straight-forward options (--interdiff, --range-diff,
> --creation-factor), which are easy to type, remember, and document.
Yes, that matches my conclusion that it is OK to have these on
separate options that can grow as many meta-diff formats we would
support.
Re: [PATCH 1/1] add hook pre-p4-submit
Eric Sunshine writes: > Bikeshedding: Should this be named 'p4-pre-submit'? That way, if > git-p4 ever grows additional hooks, they can all be grouped under the > "p4-" prefix. An excellent suggestion. > Does the hook receive any arguments? Does it receive anything on its > standard input? Those questions ought to be answered by the > documentation. > > Also, how is the hook supposed to determine whether the "submit" > should proceed? How does it know what is being submitted? Perhaps the > documentation could provide some explanation of how the hook should > glean such information (especially if the hook does not itself receive > any input). Yes, such information in the documentation, or lack thereof, would mean the distinction between a useful feature and an unusable one. All other review comments looked quite sensible and an update should address all of them. Thanks for being a careful reviewer, as always.
Re: [PATCH 1/1] add hook pre-p4-submit
On Wed, Jul 25, 2018 at 9:45 AM Chen Bin wrote:
> Hook pre-p4-submit is executed before git-p4 actually submits code.
> If the hook exits with non-zero value, submit process will abort.
Bikeshedding: Should this be named 'p4-pre-submit'? That way, if
git-p4 ever grows additional hooks, they can all be grouped under the
"p4-" prefix.
More below...
> Signed-off-by: Chen Bin
> ---
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> @@ -374,6 +374,13 @@ These options can be used to modify 'git p4 submit'
> behavior.
> +Hook for submit
> +~~~
> +Hook `pre-p4-submit` is executed if it exists and is executable.
> +Exiting with non-zero status from this script cause `git-p4 submit` to abort.
> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be changed.
Does the hook receive any arguments? Does it receive anything on its
standard input? Those questions ought to be answered by the
documentation.
Also, how is the hook supposed to determine whether the "submit"
should proceed? How does it know what is being submitted? Perhaps the
documentation could provide some explanation of how the hook should
glean such information (especially if the hook does not itself receive
any input).
> diff --git a/git-p4.py b/git-p4.py
> +# locate hook
> +hooks_path = gitConfig("core.hooksPath")
> +if len(hooks_path) > 0:
> +hook_file = os.path.join(hooks_path, "pre-p4-submit")
> +else:
> +hook_file = os.path.join(read_pipe("git rev-parse
> --git-dir").strip(), "hooks", "pre-p4-submit")
> +
> +# Execute hook. If it exits with non-zero value, do NOT continue.
> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
> subprocess.call([hook_file]) != 0:
> +sys.exit(1)
Nit: The two #comments merely repeat what the code itself already says
clearly enough, thus don't add value (so can be dropped).
Should this be emitting some sort of message letting the user know
that the operation was aborted due to the hook's "rejection"? That
could be especially important if the hook itself is silent (or buggy).
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should
> display error' '
> +# Test following scenarios:
> +# - Without hook ".git/hooks/pre-p4-submit", submit should continue
> +# - With hook returning 0, submit should continue
> +# - With hook returning 1, submit should abort
Testing three separate cases would normally be done with three
separate tests, making it easier to figure out which case failed. But,
perhaps it doesn't matter here.
> +test_expect_success 'run hook pre-p4-submit before submit' '
> + test_when_finished cleanup_git &&
> + git p4 clone --dest="$git" //depot &&
> + (
> + cd "$git" &&
> + echo "hello world" >hello.txt &&
> + git add hello.txt &&
> + git commit -m "add hello.txt" &&
> + git config git-p4.skipSubmitEdit true &&
The wholesale removal of the $git directory by cleanup_git() when the
test finishes is undoing this config setting. Ditto regarding the
hooks created below. Okay.
> + git p4 submit --dry-run | grep "Would apply" &&
We normally don't use a git command upstream in a pipe since the exit
code of that command gets lost (so, if it crashes, we don't know about
it). Instead:
git p4 submit --dry-run >out &&
grep "Would apply" out &&
Also, if "Would apply" is localized, we'd use test_i18ngrep (but you
don't need it in this case).
> + mkdir -p .git/hooks &&
> + : >.git/hooks/pre-p4-submit && chmod +x
> .git/hooks/pre-p4-submit &&
It might be clearer to readers of this test to use an explicit "exit
0" here (complementing the "exit 1" below). The current asymmetry
required extra cognitive effort to realize that this is the "returning
0" case mentioned in the comment above the test.
> + git p4 submit --dry-run | grep "Would apply" &&
> + echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
Use write_script() for this, which takes care of the #!/bin/sh line
and doing chmod:
write_script >.git/hooks/pre-p4-submit <<-\EOF &&
exit 1
EOF
> + git p4 submit --dry-run | grep "Would apply" || echo "Abort
> submit"
More idiomatic in this test suite:
git p4 submit --dry-run >out &&
! grep "Would apply" out
> + )
> +'
Re: No rule to make target `git-daemon'
Hi, On Fri, 20 Jul 2018, brian m. carlson wrote: > On Fri, Jul 20, 2018 at 05:51:46PM -0400, Jeffrey Walton wrote: > > > (If anyone is interested in first class Solaris support then I am > > happy to help. The patch set needed for the platform has been stable > > for the last couple of years). > > I'd certainly be interested for you to send it to the list. I'm not > sure I can provide a helpful review, since I don't run Solaris, but > having better support out of the box would definitely be nice. The cURL team has some SunOS build IIRC that was nicely integrated with their GitHub-based CI. Maybe we could have something similar? Ciao, Dscho
Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter
On Wed, Jul 25, 2018 at 1:29 PM Junio C Hamano wrote:
> A few random thoughts.
>
> * I find it somewhat disturbing that use of dash is inconsistent
>between "--interdiff=" and "--range-diff=".
I went with the common spellings we've seen on the mailing lists.
"Interdiff", in particular, seems well established. "Range-diff" is
new, so we don't have much data other than what we saw during the
discussion when renaming git-branch-diff, and, of course,
git-branch-diff itself is hyphenated. Another consideration: "inter"
is a prefix, whereas "range" stands on its own.
I don't have a super strong opinion, as I'm used to both the
hyphenated name (from discussion and the command name itself), and the
unhyphenated name ("rangediff" was my local branch name). I'm open to
opinions.
We probably wouldn't want to do so, but another possibility is to
recognize both --range-diff and --rangediff.
> * Perhaps "--interdiff= --range-diff" may be a possible
>way to say "use the same and show both"? Do we want
>"--range-diff= --interdiff" to mean the same two
>meta-diff but shown in different order?
That's not at all a bad shorthand. I like it better than the
"--all-kinds-of-diff=" mentioned earlier. We don't need to make a
decision for this series since such functionality can be added later.
The order of specification of the two options affecting the order of
output is also an interesting idea. We may want to decide that before
graduating this topic.
> * Do we expect that we may find a third-kind of "meta-diff" that
>sits next to interdiff and range-diff in the future? I *think*
>two separate options "--interdiff=..." and "--range-diff=..." is
>still a good way forward, and we'd add "--frotzdiff=..." when
>such a third-kind of "meta-diff" turns out to be useful, without
>fearing proliferation of options, and that would be OK, but I am
>just thinking aloud to see if other people have better ideas.
I did consider other approaches, such as a more generic option. For
example, --embed=range-diff:. I also considered supporting more
complex use-cases. For instance, git-range-diff itself accepts the two
ranges in several formats ("a..b c..d" or "x...z" or "i j k"), plus
the creation-factor can be tweaked, so I weighed ways of incorporating
all that detail into the single argument to --range-diff.
However, those are all difficult to use (onerous to type and remember)
and difficult to document. Thus, I opted for simplicity of individual,
straight-forward options (--interdiff, --range-diff,
--creation-factor), which are easy to type, remember, and document.
We may want to revisit this later if git-format-patch grows additional
similar options.
Re: [GSoC][PATCH v4 00/20] rebase -i: rewrite in C
On Tue, Jul 24, 2018 at 9:34 AM Alban Gruin wrote: > > This patch series rewrite the interactive rebase from shell to C. > > It is based on master (as of 2018-07-24). > > Changes since v3: > > - The `--verbose` option is stored directly into opts.verbose > > - Drop includes in rebase-interactive.h > > - skip_unnecessary_picks() now returns an object_id instead of a string > > - Add a test case to ensure interactive rebase aborts when the todo >list only has commented-out commands > > - complete_action() aborts when the todo list only has commented-out >commands > > - Drop the `keep_empty` parameter of complete_action() > > - Don’t remove the modes `--shorten-oids` and `--expand-oids` from >git-rebase--helper > > - Replace `ret = !!x(); return ret` by `ret = x(); return !!ret` > > - Fail if `--make-script` is provided with two arguments instead of >one > > - Rewrite write_basic_state() and init_basic_state() in C > > - Rewrite git-rebase--interactive.sh as a builtin > I gave that series a read and I think it looks good. Thanks, Stefan
Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack
On Wed, Jul 25, 2018 at 4:51 PM Max Kirillov wrote: > > On Wed, Jul 25, 2018 at 02:14:35PM +0200, SZEDER Gábor wrote: > >> +# sometimes there is fatal error buit the result is still 200 > >> +if grep 'fatal:' act.err > >> +then > >> +return 1 > >> +fi > > > > I just happened to stumble upon a failure because of 'fatal: the > > remote end hung up unexpectedly' in the test 'push plain'. > > Did it happen once or repeated? It is rather strange, that > one shoud not fail. Which OS it was? Only once, so far. It was one of my OSX build jobs on Travis CI, but I don't know what OSX version is used. 'act.err' contained this (which will get line-wrapped, I'm afraid): ++handler_type=receive ++shift ++env CONTENT_TYPE=application/x-git-receive-pack-request QUERY_STRING=/repo.git/git-receive-pack 'PATH_TRANSLATED=/Users/travis/t/trash dir.t5562/.git/git-receive-pack' GIT_HTTP_EXPORT_ALL=TRUE REQUEST_METHOD=POST /Users/travis/build/szeder/git-cooking-topics-for-travis-ci/t/t5562/invoke-with-content-length.pl push_body git http-backend <...128 zero bytes...>fatal: the remote end hung up unexpectedly I couldn't reproduce it on my Linux box. > There have been doubds that a random incoming signal can > trigger such a failure. > > > What does that "sometimes" in the above comment mean, and how often > > does such a failure happen? I see these patches are in 'pu' for over > > a month now, so based on the number of reflog entries since then it > > happened once from about 30-35 builds on Travis CI so far. > > "sometimes" here means "for some kinds of fatal error > failure", there is nothing random in it. > >> +! verify_http_result "200 OK" > > > > ... this function would return error (because of that 'if grep fatal: > > ...' statement) without even looking at the status, but the test would > > still succeed. Is that really the desired behavior here? > > Yes, it is a desired behavior. A failure is expected here, > and the failure does not show up as non-200 status, as > described above. OK, then I misunderstood that comment. Perhaps a different wording could make it slightly better? E.g. "In some of these tests ..." instead of that "sometimes". Dunno.
Re: [PATCH 5/6] pass st.st_size as hint for strbuf_readlink()
On Tue, Jul 24, 2018 at 06:51:39AM -0400, Jeff King wrote: > When we initially added the strbuf_readlink() function in > b11b7e13f4 (Add generic 'strbuf_readlink()' helper function, > 2008-12-17), the point was that we generally have a _guess_ > as to the correct size based on the stat information, but we > can't necessarily trust it. > > Over the years, a few callers have grown up that simply pass > in 0, even though they have the stat information. Let's have > them pass in their hint for consistency (and in theory > efficiency, since it may avoid an extra resize/syscall loop, > but neither location is probably performance critical). > > Note that st.st_size is actually an off_t, so in theory we > need xsize_t() here. But none of the other callsites use it, > and since this is just a hint, it doesn't matter either way > (if we wrap we'll simply start with a too-small hint and > then eventually complain when we cannot allocate the > memory). Thanks a lot for the series. For the last paragraph I would actually vote the other way around - how about something like this ? Note that st.st_size is actually an off_t, so we should use xsize_t() here. In pratise we don't expect links to be large like that, but let's give a good example in the source code and use xsize_t() whenever an off_t is converted into size_t. This will make live easier whenever someones diggs into 32/64 bit "conversion safetyness"
Re: [PATCH v2 15/18] test-reach: test commit_contains
On 7/25/2018 2:08 PM, Junio C Hamano wrote:
Jonathan Tan writes:
+ } else if (!strcmp(av[1], "commit_contains")) {
+ struct ref_filter filter;
+ struct contains_cache cache;
+ init_contains_cache();
+
+ if (ac > 2 && !strcmp(av[2], "--tag"))
+ filter.with_commit_tag_algo = 1;
+ else
+ filter.with_commit_tag_algo = 0;
+
+ printf("%s(_,A,X,_):%d\n", av[1], commit_contains(, A, X,
));
Should we initialize filter (with {NULL} or some equivalent)?
Sounds like a sensible suggestion. Wouldn't we segfault otherwise
depending on what garbage bytes are on the stack?
It's a good idea to initialize the struct properly, but the only part of
the 'filter' struct that is accessed by that method is the
'with_commit_tag_algo' member. Everything else is read from A, X, and cache.
Thanks,
-Stolee
Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'
Eric Sunshine writes: > On Mon, Jul 23, 2018 at 6:59 PM Stefan Beller wrote: >> On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine >> wrote: >> > The actual diffs output by range-diff respect diff_option.file, which >> > range-diff passes down the call-chain, thus are destination-agnostic. >> > However, output_pair_header() is hard-coded to emit to 'stdout'. Fix >> > this by making output_pair_header() respect diff_option.file, as well. >> > >> > Signed-off-by: Eric Sunshine >> >> Depending how much the range diff series has progressed >> already, it might make sense to squash it there? > > It could be done that way, though it would be nice for Dscho's > range-diff series to land without another re-roll, and it looks like > I'll probably be re-rolling this series, anyhow, to move > show_interdiff() to diff-lib.c and to fix its signature. Junio could > manage it as a "squash", but that's probably more work for him than > for me to retain it in this series. *And*, this change _is_ required > for this series, whereas it doesn't really matter for Dscho's series, > even though it's an obvious "fix". > > Anyhow, whichever way Junio and Dscho would like to play it is fine with me. Having it at the beginning of your series (I consider 1-6/14 to be a separate series, on top of which another series whose first change is this 7/14 is builds upon) like you do here is probably fine.
Re: [PATCH v2 15/18] test-reach: test commit_contains
Jonathan Tan writes:
>> +} else if (!strcmp(av[1], "commit_contains")) {
>> +struct ref_filter filter;
>> +struct contains_cache cache;
>> +init_contains_cache();
>> +
>> +if (ac > 2 && !strcmp(av[2], "--tag"))
>> +filter.with_commit_tag_algo = 1;
>> +else
>> +filter.with_commit_tag_algo = 0;
>> +
>> +printf("%s(_,A,X,_):%d\n", av[1], commit_contains(, A,
>> X, ));
>
> Should we initialize filter (with {NULL} or some equivalent)?
Sounds like a sensible suggestion. Wouldn't we segfault otherwise
depending on what garbage bytes are on the stack?
Re: [RFC] push: add documentation on push v2
On 07/25, Duy Nguyen wrote: > On Tue, Jul 24, 2018 at 9:29 PM Brandon Williams wrote: > > > > On 07/17, Brandon Williams wrote: > > > Signed-off-by: Brandon Williams > > > --- > > > > > > Since introducing protocol v2 and enabling fetch I've been thinking > > > about what its inverse 'push' would look like. After talking with a > > > number of people I have a longish list of things that could be done to > > > improve push and I think I've been able to distill the core features we > > > want in push v2. Thankfully (due to the capability system) most of the > > > other features/improvements can be added later with ease. > > > > > > What I've got now is a rough design for a more flexible push, more > > > flexible because it allows for the server to do what it wants with the > > > refs that are pushed and has the ability to communicate back what was > > > done to the client. The main motivation for this is to work around > > > issues when working with Gerrit and other code-review systems where you > > > need to have Change-Ids in the commit messages (now the server can just > > > insert them for you and send back new commits) and you need to push to > > > magic refs to get around various limitations (now a Gerrit server should > > > be able to communicate that pushing to 'master' doesn't update master > > > but instead creates a refs/changes/ ref). > > > > > > Before actually moving to write any code I'm hoping to get some feedback > > > on if we think this is an acceptable base design for push (other > > > features like atomic-push, signed-push, etc can be added as > > > capabilities), so any comments are appreciated. > > > > > > Documentation/technical/protocol-v2.txt | 76 + > > > 1 file changed, 76 insertions(+) > > > > Pinging this thread again to hopefully reach some more people for > > commentary. > > Could you send a v2 that covers all the push features in pack version > 1? I see some are discussed but it's probably good to summarize in > this document too. I can mention the ones we want to implement, but I expect that a push v2 would not require that all features in the current push are supported out of the box. Some servers may not want to support signed-push, etc. Also I don't want to have to implement every single feature that exists before getting something merged. This way follow on series can be written to implement those as new features to push v2. > > A few other comments > > If I remember correctly, we always update the remote refs locally > after a push, assuming that the next 'fetch' will do the same anyway. > This is not true for special refs (like those from gerrit). Looking > from this I don't think it can say "yes we have received your pack and > stored it "somewhere" but there's no visible ref created for it" so > that we can skip the local remote ref update? This is one of the pain points for gerrit and one of the reasons why they have this funky push syntax "push origin HEAD:refs/for/master". Because its not a remote tracking branch for the current branch, we don't update (or create) a local branch "for/master" under the "refs/remotes/origin" namespace (at least that's how i understand it). So in order to support the server doing more things (rebasing, or creating new branches) based on what is pushed the status that the server sends in response needs to be more fluid so that the server can describe what it did in a way that the client can either: update the remote tracking branches, or not (and in this case maybe do what you suggest down below). > > Is it simpler to tell a push client at the end that "yes there's new > stuff now on the server, do another fetch", sort of like HTTP > redirect, then the client can switch to fetch protocol to get the new > stuff that the server has created (e.g. rebase stuff)? I assume we > could reuse the same connection for both push and fetch if needed. > This way both fetch and push send packs in just one direction. I really, really like this suggestion. Thank you for your input. This would actually make the protocol much simpler by keeping push for pushing packs and fetch for fetching packs. And since my plan is to have the status-report for push include all the relevant ref changes that the server made, if there are any that don't correspond with what was pushed (either the server rebased a change or created a new ref I didn't) then we can skip the ref-advertisement and go straight to fetching those refs. And yes, since protocol v2 is command based we could reuse the existing connection and simply send a "fetch" request after our "push" one. This does mean that it'll be an extra round trip than what I originally had in mind, but it should be simpler. -- Brandon Williams
Re: [PATCH v4 00/21] Add `range-diff`, a `tbdiff` lookalike
On Mon, Jul 23, 2018 at 2:49 PM Junio C Hamano wrote: > > Stefan Beller writes: > > > On Sat, Jul 21, 2018 at 3:04 PM Johannes Schindelin via GitGitGadget > > wrote: > > > >> Side note: I work on implementing range-diff not only to make life easier > >> for reviewers who have to suffer through v2, v3, ... of my patch series, > >> but also to verify my changes before submitting a new iteration. And also, > >> maybe even more importantly, I plan to use it to verify my merging-rebases > >> of Git for > >> Windows (for which I previously used to redirect the > >> pre-rebase/post-rebase diffs vs upstream and then compare them using `git > >> diff --no-index`). And of course any interested person can see what > >> changes were necessary e.g. in the merging-rebase of Git for Windows onto > >> v2.17.0 by running a command like: > > > > Thanks for making tools that makes the life of a Git developer easier! > > (Just filed https://github.com/gitgitgadget/gitgitgadget/issues/26 > > which asks to break lines for this cover letter) > > Thanks. These cover letters are unreadable without W Q > (gnus-article-fill-long-lines) While I had some comments on how I dislike some aspects of the implementation, I think it proves its usefulness by my usage, so I would suggest to merge it down to next as-is (and as soon as possible); deferring the issues in the implementation to later. I found running the range-diff on origin/pu to be a pleasant experience, although that still highlights the issues of in-exact coloring (the colors are chosen by the first two characters of the diff, which leads to mis-coloring of diff headers of the inner diff in the outer diff. But despite the imperfection, I strongly urge to consider the series as-is as good enough for inclusion. Thanks, Stefan Thanks, Stefan
Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter
On Mon, Jul 23, 2018 at 9:59 PM Eric Sunshine wrote: > > On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen wrote: > > On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine > > wrote: > > > diff --git a/Documentation/git-format-patch.txt > > > b/Documentation/git-format-patch.txt > > > index f8a061794d..e7f404be3d 100644 > > > --- a/Documentation/git-format-patch.txt > > > +++ b/Documentation/git-format-patch.txt > > > @@ -24,6 +24,7 @@ SYNOPSIS > > >[--to=] [--cc=] > > >[--[no-]cover-letter] [--quiet] [--notes[=]] > > >[--interdiff=] > > > + [--range-diff=] > > > > I wonder if people will use both --interdiff= and > > --range-diff= often enough to justify a shortcut > > "--all-kinds-of-diff=" so that we don't have to type > > twice. But I guess we don't have to worry about this right now. > > My original thought was that --interdiff and --range-diff would be > mutually exclusive, however, I quickly realized that some people might > like to use both options together since each format has its strengths > and weaknesses. (I've used both types of diffs together when preparing > rerolls of my own series and found that, together, they provided a > better picture of the reroll than either would have alone.) I actually had another question that I answered myself: how do I know which one to choose? There's no preview option (and I'm lazy, I don't want to do separate diff commands myself). So my answer was "choose both, then delete the one that does not look good (and explain it in the cover too when I delete it)" > And, as you note, it's something that can be added later if > warranted (plus, this series is already long and I'd like to avoid > making it longer for something like this whose value is only > speculative). Yes of course. We can revisit this later. -- Duy
Re: [PATCH v4] Makefile: add a DEVOPTS flag to get pedantic compilation
On 25.07.18 18:57, Junio C Hamano wrote: > Beat Bolli writes: > >> In the interest of code hygiene, make it easier to compile Git with the >> flag -pedantic. >> >> Pure pedantic compilation with GCC 7.3 results in one warning per use of >> the translation macro `N_`: >> >> warning: array initialized from parenthesized string constant >> [-Wpedantic] >> >> Therefore also disable the parenthesising of i18n strings with >> -DUSE_PARENS_AROUND_GETTEXT_N=0. >> >> Signed-off-by: Beat Bolli >> --- > > Hmph, what did you change between v3 and v4? Just the commit text. In v3, it still said =No instead of =0. >> diff --git a/Makefile b/Makefile >> index 0cb6590f24..2bfc051652 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -484,6 +484,12 @@ all:: > > The postimage of this hunk is supposed to be 11 lines long, as you > have five additional line in the middle of 6 original context lines. > Where did this 12 come from? I am only interested in finding out if > our patch generation tool(s) have some bugs with this question. > > If this is only because you hand-edit your patch, then we have no > tool breakage to worry about, but please refrain from doing so in > the future (instead always go back to the commit, amend it, and > re-run format-patch). > > Thanks. You got me there :-/ Won't happen again, sorry. >> #The DEVELOPER mode enables -Wextra with a few exceptions. By >> #setting this flag the exceptions are removed, and all of >> #-Wextra is used. >> +# >> +#pedantic: >> +# >> +#Enable -pedantic compilation. This also disables >> +#USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings. >> >> GIT-VERSION-FILE: FORCE >> @$(SHELL_PATH) ./GIT-VERSION-GEN >
Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter
Eric Sunshine writes: > On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen wrote: >> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine >> wrote: >> > diff --git a/Documentation/git-format-patch.txt >> > b/Documentation/git-format-patch.txt >> > index f8a061794d..e7f404be3d 100644 >> > --- a/Documentation/git-format-patch.txt >> > +++ b/Documentation/git-format-patch.txt >> > @@ -24,6 +24,7 @@ SYNOPSIS >> >[--to=] [--cc=] >> >[--[no-]cover-letter] [--quiet] [--notes[=]] >> >[--interdiff=] >> > + [--range-diff=] >> >> I wonder if people will use both --interdiff= and >> --range-diff= often enough to justify a shortcut >> "--all-kinds-of-diff=" so that we don't have to type >> twice. But I guess we don't have to worry about this right now. > > My original thought was that --interdiff and --range-diff would be > mutually exclusive, however, I quickly realized that some people might > like to use both options together since each format has its strengths > and weaknesses. (I've used both types of diffs together when preparing > rerolls of my own series and found that, together, they provided a > better picture of the reroll than either would have alone.) > > Based upon experience on this mailing list, I'd guess that most people > would use only one or the other, though that doesn't speak for other > projects. And, as you note, it's something that can be added later if > warranted (plus, this series is already long and I'd like to avoid > making it longer for something like this whose value is only > speculative). A few random thoughts. * I find it somewhat disturbing that use of dash is inconsistent between "--interdiff=" and "--range-diff=". * Perhaps "--interdiff= --range-diff" may be a possible way to say "use the same and show both"? Do we want "--range-diff= --interdiff" to mean the same two meta-diff but shown in different order? * Do we expect that we may find a third-kind of "meta-diff" that sits next to interdiff and range-diff in the future? I *think* two separate options "--interdiff=..." and "--range-diff=..." is still a good way forward, and we'd add "--frotzdiff=..." when such a third-kind of "meta-diff" turns out to be useful, without fearing proliferation of options, and that would be OK, but I am just thinking aloud to see if other people have better ideas.
Re: [PATCH 2/2] doc hash-function-transition: pick SHA-256 as NewHash
Hi,
Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason writes:
>> The consensus on the mailing list seems to be that SHA-256 should be
>> picked as our NewHash, see the "Hash algorithm analysis" thread as of
>> [1]. Linus has come around to this choice and suggested Junio make the
>> final pick, and he's endorsed SHA-256 [3].
I think this commit message focuses too much on the development
process, in a way that makes it not necessary useful to the target
audience that would be finding it with "git blame" or "git log". It's
also not self-contained, which makes it less useful in the same way.
In other words, the commit message should be speaking for the project,
not speaking about the project. I would be tempted to say something as
simple as
hash-function-transition: pick SHA-256 as NewHash
The project has decided.
Signed-off-by: Ævar Arnfjörð Bjarmason
and let any Acked-bys on the message speak for themselves.
Alternatively, the commit message could include a summary of the
discussion:
From a security perspective, it seems that SHA-256, BLAKE2, SHA3-256,
K12, and so on are all believed to have similar security properties.
All are good options from a security point of view.
SHA-256 has a number of advantages:
* It has been around for a while, is widely used, and is supported by
just about every single crypto library (OpenSSL, mbedTLS, CryptoNG,
SecureTransport, etc).
* When you compare against SHA1DC, most vectorized SHA-256
implementations are indeed faster, even without acceleration.
* If we're doing signatures with OpenPGP (or even, I suppose, CMS),
we're going to be using SHA-2, so it doesn't make sense to have our
security depend on two separate algorithms when either one of them
alone could break the security when we could just depend on one.
So SHA-256 it is.
[...]
>> @@ -125,19 +122,19 @@ Detailed Design
>> ---
>> Repository format extension
>> ~~~
>> -A NewHash repository uses repository format version `1` (see
>> +A SHA-256 repository uses repository format version `1` (see
>> Documentation/technical/repository-version.txt) with extensions
>> `objectFormat` and `compatObjectFormat`:
>>
>> [core]
>> repositoryFormatVersion = 1
>> [extensions]
>> -objectFormat = newhash
>> +objectFormat = sha256
>> compatObjectFormat = sha1
>
> Whenever we said SHA1, somebody came and told us that the name of
> the hash is SHA-1 (with dash). Would we be nitpicker-prone in the
> same way with "sha256" here?
Regardless of how we spell it in prose, I think `sha256` as an
identifier in configuration is the spelling people will expect. For
example, gpg ("gpg --version") calls it SHA256.
[...]
>> Selection of a New Hash
>> ---
>> @@ -611,6 +608,10 @@ collisions in 2^69 operations. In August they published
>> details.
>> Luckily, no practical demonstrations of a collision in full SHA-1 were
>> published until 10 years later, in 2017.
>>
>> +It was decided that Git needed to transition to a new hash
>> +function. Initially no decision was made as to what function this was,
>> +the "NewHash" placeholder name was picked to describe it.
>> +
>> The hash function NewHash to replace SHA-1 should be stronger than
>> SHA-1 was: we would like it to be trustworthy and useful in practice
>> for at least 10 years.
>
> This sentence needs a bit of updating to match the new paragraph
> inserted above. "should be stronger" is something said by those
> who are still looking for one and/or trying to decide.
For what it's worth, I would be in favor of modifying the section
more heavily. For example:
Choice of Hash
--
In early 2005, around the time that Git was written, Xiaoyun Wang,
Yiqun Lisa Yin, and Hongbo Yu announced an attack finding SHA-1
collisions in 2^69 operations. In August they published details.
Luckily, no practical demonstrations of a collision in full SHA-1 were
published until 10 years later, in 2017.
Git v2.13.0 and later subsequently moved to a hardened SHA-1
implementation by default that mitigates the SHAttered attack, but
SHA-1 is still believed to be weak.
The hash to replace this hardened SHA-1 should be stronger than SHA-1
was: we would like it to be trustworthy and useful in practice
for at least 10 years.
Some other relevant properties:
1. A 256-bit hash (long enough to match common security practice; not
excessively long to hurt performance and disk usage).
2. High quality implementations should be widely available (e.g., in
OpenSSL and Apple CommonCrypto).
3. The hash function's properties should match Git's needs (e.g. Git
requires collision and 2nd preimage resistance and does not require
length extension resistance).
4. As a tiebreaker, the hash should be fast to compute (fortunately
many contenders are faster than SHA-1).
We choose SHA-256.
Changes:
- retitled
Re: [PATCH v4] Makefile: add a DEVOPTS flag to get pedantic compilation
Beat Bolli writes: > In the interest of code hygiene, make it easier to compile Git with the > flag -pedantic. > > Pure pedantic compilation with GCC 7.3 results in one warning per use of > the translation macro `N_`: > > warning: array initialized from parenthesized string constant [-Wpedantic] > > Therefore also disable the parenthesising of i18n strings with > -DUSE_PARENS_AROUND_GETTEXT_N=0. > > Signed-off-by: Beat Bolli > --- Hmph, what did you change between v3 and v4? > diff --git a/Makefile b/Makefile > index 0cb6590f24..2bfc051652 100644 > --- a/Makefile > +++ b/Makefile > @@ -484,6 +484,12 @@ all:: The postimage of this hunk is supposed to be 11 lines long, as you have five additional line in the middle of 6 original context lines. Where did this 12 come from? I am only interested in finding out if our patch generation tool(s) have some bugs with this question. If this is only because you hand-edit your patch, then we have no tool breakage to worry about, but please refrain from doing so in the future (instead always go back to the commit, amend it, and re-run format-patch). Thanks. > #The DEVELOPER mode enables -Wextra with a few exceptions. By > #setting this flag the exceptions are removed, and all of > #-Wextra is used. > +# > +#pedantic: > +# > +#Enable -pedantic compilation. This also disables > +#USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings. > > GIT-VERSION-FILE: FORCE > @$(SHELL_PATH) ./GIT-VERSION-GEN
Re: [PATCH 2/2] doc hash-function-transition: pick SHA-256 as NewHash
Ævar Arnfjörð Bjarmason writes: > @@ -125,19 +122,19 @@ Detailed Design > --- > Repository format extension > ~~~ > -A NewHash repository uses repository format version `1` (see > +A SHA-256 repository uses repository format version `1` (see > Documentation/technical/repository-version.txt) with extensions > `objectFormat` and `compatObjectFormat`: > > [core] > repositoryFormatVersion = 1 > [extensions] > - objectFormat = newhash > + objectFormat = sha256 > compatObjectFormat = sha1 Whenever we said SHA1, somebody came and told us that the name of the hash is SHA-1 (with dash). Would we be nitpicker-prone in the same way with "sha256" here? > @@ -155,36 +152,36 @@ repository extensions. > Object names > > Objects can be named by their 40 hexadecimal digit sha1-name or 64 > -hexadecimal digit newhash-name, plus names derived from those (see > +hexadecimal digit sha256-name, plus names derived from those (see > gitrevisions(7)). Seeing this hunk makes me respond to the above question with another question: "having to write sha-256-name, sha-1-name, gpgsig-sha-256, and sha-256-content is sort of ugly, no?" I guess names with two dashes are not _too_ bad, so I dunno. > Selection of a New Hash > --- > @@ -611,6 +608,10 @@ collisions in 2^69 operations. In August they published > details. > Luckily, no practical demonstrations of a collision in full SHA-1 were > published until 10 years later, in 2017. > > +It was decided that Git needed to transition to a new hash > +function. Initially no decision was made as to what function this was, > +the "NewHash" placeholder name was picked to describe it. > + > The hash function NewHash to replace SHA-1 should be stronger than > SHA-1 was: we would like it to be trustworthy and useful in practice > for at least 10 years. This sentence needs a bit of updating to match the new paragraph inserted above. "should be stronger" is something said by those who are still looking for one and/or trying to decide. Perhaps something like this? ... the "NewHash" placeholder name was used to describe it. We wanted to choose a hash function to replace SHA-1 that is stronger than SHA-1 was, and would like it to be trustworthy and useful in practice for at least 10 years. Some other relevant properties we wanted in NewHash are: > @@ -630,14 +631,19 @@ Some other relevant properties: > 4. As a tiebreaker, the hash should be fast to compute (fortunately > many contenders are faster than SHA-1). > > -Some hashes under consideration are SHA-256, SHA-512/256, SHA-256x16, > +Some hashes under consideration were SHA-256, SHA-512/256, SHA-256x16, > K12, and BLAKE2bp-256. > > +Eventually in July 2018 SHA-256 was chosen to be the NewHash. See the > +thread starting at <20180609224913.gc38...@genre.crustytoothpaste.net> > +for the discussion > +(https://public-inbox.org/git/20180609224913.gc38...@genre.crustytoothpaste.net/) > +
Re: [PATCH v1 0/3] [RFC] Speeding up checkout (and merge, rebase, etc)
On 7/24/2018 11:13 AM, Duy Nguyen wrote:
On Mon, Jul 23, 2018 at 04:51:38PM -0400, Ben Peart wrote:
What's the current state of the index before this checkout?
This was after running "git checkout" multiple times so there was really
nothing for git to do.
Hmm.. this means cache-tree is fully valid, unless you have changes in
index. We're quite aggressive in repairing cache-tree since aecf567cbf
(cache-tree: create/update cache-tree on checkout - 2014-07-05). If we
have very good cache-tree records and still spend 33s on
traverse_trees, maybe there's something else.
I'm not at all familiar with the cache-tree and couldn't find any
documentation on it other than index-format.txt which says "it helps
speed up tree object generation for a new commit."
I guess you have the starting points you need after Jeff's and Junio's
explanation (and it would be great if cache-tree could actually be for
for this two-way merge). But to make it easier for new people in
future, maybe we should add this?
This is basically a ripoff of Junio's explanation with starting points
(write-tree and index-format.txt). I wanted to incorporate some pieces
from Jeff's too but I think Junio's already covered it well.
I definitely like capturing this in the code or documentation somewhere.
Given I checked the header file for any hints on the design, I think
that is a reasonable place to put it.
-- 8< --
Subject: [PATCH] cache-tree.h: more description of what it is and what's it
used for
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache-tree.h | 29 +
1 file changed, 29 insertions(+)
diff --git a/cache-tree.h b/cache-tree.h
index cfd5328cc9..d25a800a72 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -5,6 +5,35 @@
#include "tree.h"
#include "tree-walk.h"
+/*
+ * cache-tree is an index extension that records tree object names for
+ * subdirectories you see in the index. It is mainly used for
+ * generating trees from the index before you create a new commit (see
+ * builtin/write-tree.c as starting point) but it's also used in "git
+ * diff-index --cached $TREE" as an optimization. See index-format.txt
+ * for on-disk format.
+ *
+ * Every time you write the contents of the index as a tree object, we
I had to read this a couple of times to figure out what was meant by
"write the contents of the index as a tree object." Maybe it was just
me but how about something like:
"Every time you write a new tree object from the index you need to
collect the object name for each top-level path and write a new
top-level tree object out and then do the same recursively for any
subdirectory."
+ * need to collect the object name for each top-level paths and write
+ * a new top-level tree object out, after doing the same recursively
+ * for any modified subdirectory. Whenever you add, remove or modify a
+ * path in the index, the cache-tree entry for enclosing directories
+ * are invalidated, so a cache-tree entry that is still valid means
+ * that all the paths in the index under that directory match the
+ * contents of the tree object that the cache-tree entry holds.
+ *
+ * And that property is used by "diff-index --cached $TREE" that is
+ * run internally. When we find that the subdirectory "D"'s
+ * cache-tree entry is valid in the index, and the tree object
+ * recorded in the cache-tree for that subdirectory matches the
+ * subtree D in the tree object $TREE, then "diff-index --cached"
+ * ignores the entire subdirectory D (which saves relatively little in
+ * the index as it only needs to scan what is already in the memory
+ * forward, but on the $TREE traversal side, it does not have to even
+ * open a subtree, that can save a lot), and with a well-populated
+ * cache-tree, it can save a significant processing.
+ */
+
struct cache_tree;
struct cache_tree_sub {
struct cache_tree *cache_tree;
Re: [RFC] push: add documentation on push v2
On Tue, Jul 24, 2018 at 9:29 PM Brandon Williams wrote: > > On 07/17, Brandon Williams wrote: > > Signed-off-by: Brandon Williams > > --- > > > > Since introducing protocol v2 and enabling fetch I've been thinking > > about what its inverse 'push' would look like. After talking with a > > number of people I have a longish list of things that could be done to > > improve push and I think I've been able to distill the core features we > > want in push v2. Thankfully (due to the capability system) most of the > > other features/improvements can be added later with ease. > > > > What I've got now is a rough design for a more flexible push, more > > flexible because it allows for the server to do what it wants with the > > refs that are pushed and has the ability to communicate back what was > > done to the client. The main motivation for this is to work around > > issues when working with Gerrit and other code-review systems where you > > need to have Change-Ids in the commit messages (now the server can just > > insert them for you and send back new commits) and you need to push to > > magic refs to get around various limitations (now a Gerrit server should > > be able to communicate that pushing to 'master' doesn't update master > > but instead creates a refs/changes/ ref). > > > > Before actually moving to write any code I'm hoping to get some feedback > > on if we think this is an acceptable base design for push (other > > features like atomic-push, signed-push, etc can be added as > > capabilities), so any comments are appreciated. > > > > Documentation/technical/protocol-v2.txt | 76 + > > 1 file changed, 76 insertions(+) > > Pinging this thread again to hopefully reach some more people for > commentary. Could you send a v2 that covers all the push features in pack version 1? I see some are discussed but it's probably good to summarize in this document too. A few other comments If I remember correctly, we always update the remote refs locally after a push, assuming that the next 'fetch' will do the same anyway. This is not true for special refs (like those from gerrit). Looking from this I don't think it can say "yes we have received your pack and stored it "somewhere" but there's no visible ref created for it" so that we can skip the local remote ref update? Is it simpler to tell a push client at the end that "yes there's new stuff now on the server, do another fetch", sort of like HTTP redirect, then the client can switch to fetch protocol to get the new stuff that the server has created (e.g. rebase stuff)? I assume we could reuse the same connection for both push and fetch if needed. This way both fetch and push send packs in just one direction. -- Duy
Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack
On Wed, Jul 25, 2018 at 02:14:35PM +0200, SZEDER Gábor wrote: >> +# sometimes there is fatal error buit the result is still 200 > > s/buit/but/ Thanks, will fix >> +if grep 'fatal:' act.err >> +then >> +return 1 >> +fi > > I just happened to stumble upon a failure because of 'fatal: the > remote end hung up unexpectedly' in the test 'push plain'. Did it happen once or repeated? It is rather strange, that one shoud not fail. Which OS it was? There have been doubds that a random incoming signal can trigger such a failure. > What does that "sometimes" in the above comment mean, and how often > does such a failure happen? I see these patches are in 'pu' for over > a month now, so based on the number of reflog entries since then it > happened once from about 30-35 builds on Travis CI so far. "sometimes" here means "for some kinds of fatal error failure", there is nothing random in it. > > + "$TEST_DIRECTORY"/t5562/invoke-with-content-length.pl > > fetch_body git http-backend >act.out 2>act.err && > > Don't save the standard error of the whole shell function. > When running the test with /bin/sh and '-x' tracing, then the trace of > commands executed in the function will be included in the standard > error as well, which may interfere with later verification (though in > this case it doesn't seem like it would cause any issues). > > Please limit the redirections to the relevant command's output. AFAICT > all invocations of 'test_http_env' in these tests have their stdout and > stderr redirected to the same pair of files, so perhaps you could > simply move all these redirections inside the function. Thanks, I'll try to fix it >> +! verify_http_result "200 OK" > > ... this function would return error (because of that 'if grep fatal: > ...' statement) without even looking at the status, but the test would > still succeed. Is that really the desired behavior here? Yes, it is a desired behavior. A failure is expected here, and the failure does not show up as non-200 status, as described above.
Re: Using Git for applications other than code development
On Wed, Jul 25, 2018 at 01:02:16PM +, David Hind wrote: > I work for a company that is looking to adopt VCS and I like sound of > Git (although I have no experience of using VCS). My question is, > everything seems to be directed towards code developers. Can I use Git > to do revision control for other types of design document? For example > electrical circuit designs, circuit PCB designs etc.? In addition to what Randall said (I would affirm he has presented a comprehensive and correct picture), I'd make one minor note: Git is a distributed VCS, and this requires certain pondering. Even though you will almost certainly have one central (also colloquially called "rendez-vouz") repository where everyone pushes their changes to, and fetches them from, Git allows recording any number of so-called "commits" — atomic, from a logical standpoint, changes to the project — before sharing them with the rest of the team. For software development, this _is_ blessing; for other kinds of development it may be not so good — with the need to somehow resolve possible conflicts in series of changes made in parallel by multiple developers to the same content being supposedly the main impediment. With software development it's simple: it's still done by writing textual files (even tools which generate code automatically tend to generate something textual these days), and conflicts in textual files are relatively easy to represent (even without resorting to specialized tools). Now, say, let's look at gamedev where a part of the team are artists which work on "assets" — such as 3D-models and textures. Two conflicting changes in the same texture are harder to reconcile. Some (most?) centralized VCSes (as opposed to distibuted) support explicit "locking" of certain files - which works like sort of claim "I'm working on this file, don't touch it". Git does not have locking, and if you think it may benefit your workflow then may be other options might suite you better.
[PATCH 1/1] add hook pre-p4-submit
Hook pre-p4-submit is executed before git-p4 actually submits code.
If the hook exits with non-zero value, submit process will abort.
Signed-off-by: Chen Bin
---
Documentation/git-p4.txt | 7 +++
Documentation/githooks.txt | 7 +++
git-p4.py | 18 +-
t/t9800-git-p4-basic.sh| 22 ++
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index f0de3b891..73f7a2691 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -374,6 +374,13 @@ These options can be used to modify 'git p4 submit'
behavior.
been submitted. Implies --disable-rebase. Can also be set with
git-p4.disableP4Sync. Sync with origin/master still goes ahead if possible.
+Hook for submit
+~~~
+Hook `pre-p4-submit` is executed if it exists and is executable.
+Exiting with non-zero status from this script cause `git-p4 submit` to abort.
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be changed.
+See githooks(5) manpage for details about hooks.
+
Rebase options
~~
These options can be used to modify 'git p4 rebase' behavior.
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index e3c283a17..5148627b4 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -485,6 +485,13 @@ The exit status determines whether git will use the data
from the
hook to limit its search. On error, it will fall back to verifying
all files and folders.
+pre-p4-submit
+~
+
+This hook is invoked by `git-p4 submit`. It takes no parameter, and
+exiting with non-zero status from this script causes `git-p4 submit`
+to abort. Run `git-p4 submit --help` for details.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/git-p4.py b/git-p4.py
index b449db1cc..bbb0d987e 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1494,7 +1494,12 @@ def __init__(self):
optparse.make_option("--disable-p4sync",
dest="disable_p4sync", action="store_true",
help="Skip Perforce sync of p4/master
after submit or shelve"),
]
-self.description = "Submit changes from git to the perforce depot."
+self.description = """Submit changes from git to the perforce depot.\n
+Hook `pre-p4-submit` is executed if it exists and is executable.
+Exiting with non-zero status from this script cause `git-p4 submit` to
abort.
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
changed.
+See githooks(5) manpage for details about hooks."""
+
self.usage += " [name of git branch to submit into perforce depot]"
self.origin = ""
self.detectRenames = False
@@ -2303,6 +2308,17 @@ def run(self, args):
sys.exit("number of commits (%d) must match number of shelved
changelist (%d)" %
(len(commits), num_shelves))
+# locate hook
+hooks_path = gitConfig("core.hooksPath")
+if len(hooks_path) > 0:
+hook_file = os.path.join(hooks_path, "pre-p4-submit")
+else:
+hook_file = os.path.join(read_pipe("git rev-parse
--git-dir").strip(), "hooks", "pre-p4-submit")
+
+# Execute hook. If it exits with non-zero value, do NOT continue.
+if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
subprocess.call([hook_file]) != 0:
+sys.exit(1)
+
#
# Apply the commits, one at a time. On failure, ask if should
# continue to try the rest of the patches, or quit.
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 4849edc4e..48b768fa7 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should
display error' '
)
'
+# Test following scenarios:
+# - Without hook ".git/hooks/pre-p4-submit", submit should continue
+# - With hook returning 0, submit should continue
+# - With hook returning 1, submit should abort
+test_expect_success 'run hook pre-p4-submit before submit' '
+ test_when_finished cleanup_git &&
+ git p4 clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ echo "hello world" >hello.txt &&
+ git add hello.txt &&
+ git commit -m "add hello.txt" &&
+ git config git-p4.skipSubmitEdit true &&
+ git p4 submit --dry-run | grep "Would apply" &&
+ mkdir -p .git/hooks &&
+ : >.git/hooks/pre-p4-submit && chmod +x
.git/hooks/pre-p4-submit &&
+ git p4 submit --dry-run | grep "Would apply" &&
+ echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
+ git p4 submit --dry-run | grep "Would apply" || echo "Abort
submit"
+ )
+'
+
test_expect_success 'submit from detached head' '
test_when_finished cleanup_git
RE: Using Git for applications other than code development
Hi David, I have used git over the past 3 years in a manufacturing environment to manage component designs in a CAD/factory automation setting. There are a few main factors that you need to consider: 1. You will need an external tool like Git for Windows, GitHub Client or SourceTree for performing git operations on your workstations because your design software is unlikely to support any VCS directly. 2. Your design software probably needs to store its designs in some text form. This will allow a lot of the advanced git functions, like annotate, to work nicely. This is not a requirement as git is happy to manage binaries (your renderings, for example). 3. You might need to figure out a way to interpret changes when there are conflicts between designers. This either means learning the underlying format (auto-lisp??) or making choices of whose design is going to take priority without trying to merge changes or otherwise resolve conflicts. 4. Lastly (but really there are more you will encounter), you will need to decide on either a local shared repository (GitHub Enterprise, BitBucket Server, GitLab, etc.) or a similar cloud solution (same names). There are costs for each, usually depending on your team size. The costs are pretty small for small teams and more than worth it, IMHO. 5. I have been repeatedly surprised at how many hardware component designers actually have git experience (followed lastly deliberately). Git is generally a good fit for advanced manufacturing. I'm including a discussion of git in a seminar I am giving at IWF Atlanta next month. Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much. From: git-ow...@vger.kernel.org On Behalf Of David Hind Sent: July 25, 2018 9:02 AM To: 'git@vger.kernel.org' Subject: Using Git for applications other than code development Hi, I work for a company that is looking to adopt VCS and I like sound of Git (although I have no experience of using VCS). My question is, everything seems to be directed towards code developers. Can I use Git to do revision control for other types of design document? For example electrical circuit designs, circuit PCB designs etc.? Thanks! David Dynex Semiconductor Limited. Registered in England and Wales: No 3824626 Registered Office: Doddington Road, Lincoln, LN6 3LF, United Kingdom This e-mail and any attachments are confidential and may be privileged. If you are not the intended recipient please notify the sender immediately, delete the email from your system and do not disclose the contents to another person, use it for any purpose or store or copy the information in any medium. Whilst we run anti-virus software on all internet emails we do not accept responsibility for viruses and advise that in keeping with good computing practice you should ensure this email and any attachments are virus free.
Using Git for applications other than code development
Hi, I work for a company that is looking to adopt VCS and I like sound of Git (although I have no experience of using VCS). My question is, everything seems to be directed towards code developers. Can I use Git to do revision control for other types of design document? For example electrical circuit designs, circuit PCB designs etc.? Thanks! David Dynex Semiconductor Limited. Registered in England and Wales: No 3824626 Registered Office: Doddington Road, Lincoln, LN6 3LF, United Kingdom This e-mail and any attachments are confidential and may be privileged. If you are not the intended recipient please notify the sender immediately, delete the email from your system and do not disclose the contents to another person, use it for any purpose or store or copy the information in any medium. Whilst we run anti-virus software on all internet emails we do not accept responsibility for viruses and advise that in keeping with good computing practice you should ensure this email and any attachments are virus free.
Re: [PATCH v1] config.c: fix msvc compile error
On 7/24/2018 3:56 PM, Taylor Blau wrote: On Tue, Jul 24, 2018 at 03:30:10PM +, g...@jeffhostetler.com wrote: From: Jeff Hostetler In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added to builtin/config.c to define a new function and a forward declaration for an array of unknown size. This causes a compile error under MSVC. Thanks for spending time fixing that. fb0dc3bac1 (builtin/config.c: support `--type=` as preferred alias for `--`, 2018-04-18) is from me, so I owe you an extra thanks for patching up mistakes :-). As others have noted in this thread, another patch was sent into similar effect, which has already been queued, and I agree that we should prefer that, since it's further along. Thanks, Taylor Yes, the other version is further along. Let's take it. Jeff
Re: [PATCH v1] msvc: fix non-standard escape sequence in source
On 7/24/2018 2:13 PM, Beat Bolli wrote: Hi Jeff On 24.07.18 16:42, g...@jeffhostetler.com wrote: From: Jeff Hostetler Replace non-standard "\e" escape sequence with "\x1B". This was already fixed in <20180708144342.11922-4-dev+...@drbeat.li>. Cheers, Beat Thanks for the pointer. I see that commit is in 'next'. I was only looking in 'master'. Thanks, Jeff
Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack
[Hrm, this time with hopefully proper In-Reply-To: header.
Sorry for the double post.]
> Push passes to another commands, as described in
> https://public-inbox.org/git/20171129032214.gb32...@sigill.intra.peff.net/
>
> As it gets complicated to correctly track the data length, instead transfer
> the data through parent process and cut the pipe as the specified length is
> reached. Do it only when CONTENT_LENGTH is set, otherwise pass the input
> directly to the forked commands.
>
> Add tests for cases:
>
> * CONTENT_LENGTH is set, script's stdin has more data, with all combinations
> of variations: fetch or push, plain or compressed body, correct or truncated
> input.
>
> * CONTENT_LENGTH is specified to a value which does not fit into ssize_t.
>
> Helped-by: Junio C Hamano
> Signed-off-by: Max Kirillov
> ---
> help.c | 1 +
> http-backend.c | 32 -
> t/t5562-http-backend-content-length.sh | 169 +
> t/t5562/invoke-with-content-length.pl | 37 ++
> 4 files changed, 237 insertions(+), 2 deletions(-)
> create mode 100755 t/t5562-http-backend-content-length.sh
> create mode 100755 t/t5562/invoke-with-content-length.pl
> diff --git a/t/t5562-http-backend-content-length.sh
> b/t/t5562-http-backend-content-length.sh
> new file mode 100755
> index 00..8040d80e04
> --- /dev/null
> +++ b/t/t5562-http-backend-content-length.sh
> @@ -0,0 +1,169 @@
> +#!/bin/sh
> +
> +test_description='test git-http-backend respects CONTENT_LENGTH'
> +. ./test-lib.sh
> +
> +test_lazy_prereq GZIP 'gzip --version'
> +
> +verify_http_result() {
> + # sometimes there is fatal error buit the result is still 200
s/buit/but/
> + if grep 'fatal:' act.err
> + then
> + return 1
> + fi
I just happened to stumble upon a failure because of 'fatal: the
remote end hung up unexpectedly' in the test 'push plain'.
What does that "sometimes" in the above comment mean, and how often
does such a failure happen? I see these patches are in 'pu' for over
a month now, so based on the number of reflog entries since then it
happened once from about 30-35 builds on Travis CI so far.
I don't really like the idea of adding a bunch of flaky test cases...
we have enough of them already, unfortunately.
> +
> + if ! grep "Status" act.out >act
> + then
> + printf "Status: 200 OK\r\n" >act
> + fi
> + printf "Status: $1\r\n" >exp &&
> + test_cmp exp act
> +}
> +
> +test_http_env() {
> + handler_type="$1"
> + shift
> + env \
> + CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
> + QUERY_STRING="/repo.git/git-$handler_type-pack" \
> + PATH_TRANSLATED="$PWD/.git/git-$handler_type-pack" \
> + GIT_HTTP_EXPORT_ALL=TRUE \
> + REQUEST_METHOD=POST \
> + "$@"
> +}
> +
> +ssize_b100dots() {
> + # hardcoded ((size_t) SSIZE_MAX) + 1
> + case "$(build_option sizeof-size_t)" in
> + 8) echo 9223372036854775808;;
> + 4) echo 2147483648;;
> + *) die "Unexpected ssize_t size: $(build_option sizeof-size_t)";;
> + esac
> +}
> +
> +test_expect_success 'setup' '
> + git config http.receivepack true &&
> + test_commit c0 &&
> + test_commit c1 &&
> + hash_head=$(git rev-parse HEAD) &&
> + hash_prev=$(git rev-parse HEAD~1) &&
> + printf "want %s" "$hash_head" | packetize >fetch_body &&
> + printf >>fetch_body &&
> + printf "have %s" "$hash_prev" | packetize >>fetch_body &&
> + printf done | packetize >>fetch_body &&
> + test_copy_bytes 10 fetch_body.trunc &&
> + hash_next=$(git commit-tree -p HEAD -m next HEAD^{tree}) &&
> + printf "%s %s refs/heads/newbranch\\0report-status\\n" "$_z40"
> "$hash_next" | packetize >push_body &&
> + printf >>push_body &&
> + echo "$hash_next" | git pack-objects --stdout >>push_body &&
> + test_copy_bytes 10 push_body.trunc &&
> + : >empty_body
> +'
> +
> +test_expect_success GZIP 'setup, compression related' '
> + gzip -k fetch_body &&
> + test_copy_bytes 10 fetch_body.gz.trunc &&
> + gzip -k push_body &&
> + test_copy_bytes 10 push_body.gz.trunc
> +'
> +
> +test_expect_success 'fetch plain' '
> + test_http_env upload \
> + "$TEST_DIRECTORY"/t5562/invoke-with-content-length.pl
> fetch_body git http-backend >act.out 2>act.err &&
Don't save the standard error of the whole shell function.
When running the test with /bin/sh and '-x' tracing, then the trace of
commands executed in the function will be included in the standard
error as well, which may interfere with later verification (though in
this case it doesn't seem like it would cause any issues).
Please limit the redirections to the relevant command's output. AFAICT
all invocations of 'test_http_env' in these tests have their stdout and
stderr redirected to the same pair of files, so perhaps you
Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack
> Push passes to another commands, as described in
> https://public-inbox.org/git/20171129032214.gb32...@sigill.intra.peff.net/
>
> As it gets complicated to correctly track the data length, instead transfer
> the data through parent process and cut the pipe as the specified length is
> reached. Do it only when CONTENT_LENGTH is set, otherwise pass the input
> directly to the forked commands.
>
> Add tests for cases:
>
> * CONTENT_LENGTH is set, script's stdin has more data, with all combinations
> of variations: fetch or push, plain or compressed body, correct or truncated
> input.
>
> * CONTENT_LENGTH is specified to a value which does not fit into ssize_t.
>
> Helped-by: Junio C Hamano
> Signed-off-by: Max Kirillov
> ---
> help.c | 1 +
> http-backend.c | 32 -
> t/t5562-http-backend-content-length.sh | 169 +
> t/t5562/invoke-with-content-length.pl | 37 ++
> 4 files changed, 237 insertions(+), 2 deletions(-)
> create mode 100755 t/t5562-http-backend-content-length.sh
> create mode 100755 t/t5562/invoke-with-content-length.pl
> diff --git a/t/t5562-http-backend-content-length.sh
> b/t/t5562-http-backend-content-length.sh
> new file mode 100755
> index 00..8040d80e04
> --- /dev/null
> +++ b/t/t5562-http-backend-content-length.sh
> @@ -0,0 +1,169 @@
> +#!/bin/sh
> +
> +test_description='test git-http-backend respects CONTENT_LENGTH'
> +. ./test-lib.sh
> +
> +test_lazy_prereq GZIP 'gzip --version'
> +
> +verify_http_result() {
> + # sometimes there is fatal error buit the result is still 200
s/buit/but/
> + if grep 'fatal:' act.err
> + then
> + return 1
> + fi
I just happened to stumble upon a failure because of 'fatal: the
remote end hung up unexpectedly' in the test 'push plain'.
What does that "sometimes" in the above comment mean, and how often
does such a failure happen? I see these patches are in 'pu' for over
a month now, so based on the number of reflog entries since then it
happened once from about 30-35 builds on Travis CI so far.
I don't really like the idea of adding a bunch of flaky test cases...
we have enough of them already, unfortunately.
> +
> + if ! grep "Status" act.out >act
> + then
> + printf "Status: 200 OK\r\n" >act
> + fi
> + printf "Status: $1\r\n" >exp &&
> + test_cmp exp act
> +}
> +
> +test_http_env() {
> + handler_type="$1"
> + shift
> + env \
> + CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
> + QUERY_STRING="/repo.git/git-$handler_type-pack" \
> + PATH_TRANSLATED="$PWD/.git/git-$handler_type-pack" \
> + GIT_HTTP_EXPORT_ALL=TRUE \
> + REQUEST_METHOD=POST \
> + "$@"
> +}
> +
> +ssize_b100dots() {
> + # hardcoded ((size_t) SSIZE_MAX) + 1
> + case "$(build_option sizeof-size_t)" in
> + 8) echo 9223372036854775808;;
> + 4) echo 2147483648;;
> + *) die "Unexpected ssize_t size: $(build_option sizeof-size_t)";;
> + esac
> +}
> +
> +test_expect_success 'setup' '
> + git config http.receivepack true &&
> + test_commit c0 &&
> + test_commit c1 &&
> + hash_head=$(git rev-parse HEAD) &&
> + hash_prev=$(git rev-parse HEAD~1) &&
> + printf "want %s" "$hash_head" | packetize >fetch_body &&
> + printf >>fetch_body &&
> + printf "have %s" "$hash_prev" | packetize >>fetch_body &&
> + printf done | packetize >>fetch_body &&
> + test_copy_bytes 10 fetch_body.trunc &&
> + hash_next=$(git commit-tree -p HEAD -m next HEAD^{tree}) &&
> + printf "%s %s refs/heads/newbranch\\0report-status\\n" "$_z40"
> "$hash_next" | packetize >push_body &&
> + printf >>push_body &&
> + echo "$hash_next" | git pack-objects --stdout >>push_body &&
> + test_copy_bytes 10 push_body.trunc &&
> + : >empty_body
> +'
> +
> +test_expect_success GZIP 'setup, compression related' '
> + gzip -k fetch_body &&
> + test_copy_bytes 10 fetch_body.gz.trunc &&
> + gzip -k push_body &&
> + test_copy_bytes 10 push_body.gz.trunc
> +'
> +
> +test_expect_success 'fetch plain' '
> + test_http_env upload \
> + "$TEST_DIRECTORY"/t5562/invoke-with-content-length.pl
> fetch_body git http-backend >act.out 2>act.err &&
Don't save the standard error of the whole shell function.
When running the test with /bin/sh and '-x' tracing, then the trace of
commands executed in the function will be included in the standard
error as well, which may interfere with later verification (though in
this case it doesn't seem like it would cause any issues).
Please limit the redirections to the relevant command's output. AFAICT
all invocations of 'test_http_env' in these tests have their stdout and
stderr redirected to the same pair of files, so perhaps you could
simply move all these redirections inside the function.
> + verify_http_result "200
Re: [PATCH 1/1] add hook pre-p4-submit
Thanks for you review. I will update documentation asap.
On Wed, Jul 25, 2018 at 7:14 PM, Ævar Arnfjörð Bjarmason
wrote:
>
> On Wed, Jul 25 2018, Luke Diamand wrote:
>
>> On 23 July 2018 at 12:27, Chen Bin wrote:
>>> Hook pre-p4-submit is executed before git-p4 actually submits code.
>>> If the hook exits with non-zero value, submit process will abort.
>>
>>
>> Looks good to me - could you add some documentation please
>> (Documentation/git-p4.txt).
>>
>> Thanks!
>> Luke
>
> This looks correct (and you'd know better), but I was surprised that we
> wouldn't just document this in githooks(5), but looking at git-p4 I see
> that we have its config documented there, not in git-config(1) (ditto
> some git-svn config stuff).
>
> Shouldn't we at least update githooks & git-config to say that the
> config / hooks documentation for these utilities can be found there?
>
>>>
>>> Signed-off-by: Chen Bin
>>> ---
>>> git-p4.py | 6 ++
>>> t/t9800-git-p4-basic.sh | 22 ++
>>> 2 files changed, 28 insertions(+)
>>>
>>> diff --git a/git-p4.py b/git-p4.py
>>> index b449db1cc..69ee9ce41 100755
>>> --- a/git-p4.py
>>> +++ b/git-p4.py
>>> @@ -2303,6 +2303,12 @@ def run(self, args):
>>> sys.exit("number of commits (%d) must match number of shelved
>>> changelist (%d)" %
>>> (len(commits), num_shelves))
>>>
>>> +# locate hook at `.git/hooks/pre-p4-submit`
>>> +hook_file = os.path.join(read_pipe("git rev-parse
>>> --git-dir").strip(), "hooks", "pre-p4-submit")
>>> +# Execute hook. If it exit with non-zero value, do NOT continue.
>>> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
>>> subprocess.call([hook_file]) != 0:
>>> +sys.exit(1)
>>> +
>>> #
>>> # Apply the commits, one at a time. On failure, ask if should
>>> # continue to try the rest of the patches, or quit.
>>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
>>> index 4849edc4e..48b768fa7 100755
>>> --- a/t/t9800-git-p4-basic.sh
>>> +++ b/t/t9800-git-p4-basic.sh
>>> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT
>>> should display error' '
>>> )
>>> '
>>>
>>> +# Test following scenarios:
>>> +# - Without hook ".git/hooks/pre-p4-submit", submit should continue
>>> +# - With hook returning 0, submit should continue
>>> +# - With hook returning 1, submit should abort
>>> +test_expect_success 'run hook pre-p4-submit before submit' '
>>> + test_when_finished cleanup_git &&
>>> + git p4 clone --dest="$git" //depot &&
>>> + (
>>> + cd "$git" &&
>>> + echo "hello world" >hello.txt &&
>>> + git add hello.txt &&
>>> + git commit -m "add hello.txt" &&
>>> + git config git-p4.skipSubmitEdit true &&
>>> + git p4 submit --dry-run | grep "Would apply" &&
>>> + mkdir -p .git/hooks &&
>>> + : >.git/hooks/pre-p4-submit && chmod +x
>>> .git/hooks/pre-p4-submit &&
>>> + git p4 submit --dry-run | grep "Would apply" &&
>>> + echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
>>> + git p4 submit --dry-run | grep "Would apply" || echo "Abort
>>> submit"
>>> + )
>>> +'
>>> +
>>> test_expect_success 'submit from detached head' '
>>> test_when_finished cleanup_git &&
>>> git p4 clone --dest="$git" //depot &&
>>> --
>>> 2.18.0
>>>
--
help me, help you.
Re: [PATCH] git-rebase--interactive.sh: Remove superfluous tab in rebase
On Wed, Jul 25 2018, Daniel Baumann wrote:
> the attached patch removes a superfluous tab indentation in the
> interactive git-rebase messages.
>
> It would be nice to get rid of this to avoid visual disturbance with
> whitespace highlighting editors.
The code you're modifying doesn't exist in the "pu" branch since
249d626f2c ("rebase--interactive: rewrite append_todo_help() in C",
2018-07-10), rebase is being rewritten in C. From looking at it it seems
we no longer have this problem, but perhaps you'd like to check that out
for yourself?
[PATCH] git-rebase--interactive.sh: Remove superfluous tab in rebase
Hi, the attached patch removes a superfluous tab indentation in the interactive git-rebase messages. It would be nice to get rid of this to avoid visual disturbance with whitespace highlighting editors. Regards, Daniel >From 353eb1c2c44d5117a3fad1323ca6852b13e4c86a Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Wed, 25 Jul 2018 11:58:40 +0200 Subject: [PATCH] git-rebase--interactive.sh: Remove superfluous tab in rebase file Removing superfluous tab indentation in interactive git-rebase file to avoid visual disturbance with whitespace highlighting editors. Signed-off-by: Daniel Baumann --- git-rebase--interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 299ded213..0bf9eefad 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -224,7 +224,7 @@ EOF gettext " However, if you remove everything, the rebase will be aborted. - " | git stripspace --comment-lines >>"$todo" +" | git stripspace --comment-lines >>"$todo" if test -z "$keep_empty" then -- 2.18.0
[RFC] notes..rewriteMode configuration option
Hi all, The project I work on currently heavily uses git. We would like to add some tools based around detecting how branches change over time for code review, so that users can compare commits as they change. We'd like to do this via adding metadata in git notes, so that we can detect when commits are ammended or squashed. While digging around in the `git notes` help, I noticed there is support for `notes..mergeStrategy` but not `notes..rewriteMode` configuration options. I was wondering if you would be interested in a patch that adds support for a `notes..rewriteMode` configuration option that would take precedence for doing note rewrites for the given ref (like how `notes..mergeStrategy` takes precedence for merges). Regards, Barret Rennie
Re: [PATCH 1/1] add hook pre-p4-submit
On Wed, Jul 25 2018, Luke Diamand wrote:
> On 23 July 2018 at 12:27, Chen Bin wrote:
>> Hook pre-p4-submit is executed before git-p4 actually submits code.
>> If the hook exits with non-zero value, submit process will abort.
>
>
> Looks good to me - could you add some documentation please
> (Documentation/git-p4.txt).
>
> Thanks!
> Luke
This looks correct (and you'd know better), but I was surprised that we
wouldn't just document this in githooks(5), but looking at git-p4 I see
that we have its config documented there, not in git-config(1) (ditto
some git-svn config stuff).
Shouldn't we at least update githooks & git-config to say that the
config / hooks documentation for these utilities can be found there?
>>
>> Signed-off-by: Chen Bin
>> ---
>> git-p4.py | 6 ++
>> t/t9800-git-p4-basic.sh | 22 ++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index b449db1cc..69ee9ce41 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2303,6 +2303,12 @@ def run(self, args):
>> sys.exit("number of commits (%d) must match number of shelved
>> changelist (%d)" %
>> (len(commits), num_shelves))
>>
>> +# locate hook at `.git/hooks/pre-p4-submit`
>> +hook_file = os.path.join(read_pipe("git rev-parse
>> --git-dir").strip(), "hooks", "pre-p4-submit")
>> +# Execute hook. If it exit with non-zero value, do NOT continue.
>> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
>> subprocess.call([hook_file]) != 0:
>> +sys.exit(1)
>> +
>> #
>> # Apply the commits, one at a time. On failure, ask if should
>> # continue to try the rest of the patches, or quit.
>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
>> index 4849edc4e..48b768fa7 100755
>> --- a/t/t9800-git-p4-basic.sh
>> +++ b/t/t9800-git-p4-basic.sh
>> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should
>> display error' '
>> )
>> '
>>
>> +# Test following scenarios:
>> +# - Without hook ".git/hooks/pre-p4-submit", submit should continue
>> +# - With hook returning 0, submit should continue
>> +# - With hook returning 1, submit should abort
>> +test_expect_success 'run hook pre-p4-submit before submit' '
>> + test_when_finished cleanup_git &&
>> + git p4 clone --dest="$git" //depot &&
>> + (
>> + cd "$git" &&
>> + echo "hello world" >hello.txt &&
>> + git add hello.txt &&
>> + git commit -m "add hello.txt" &&
>> + git config git-p4.skipSubmitEdit true &&
>> + git p4 submit --dry-run | grep "Would apply" &&
>> + mkdir -p .git/hooks &&
>> + : >.git/hooks/pre-p4-submit && chmod +x
>> .git/hooks/pre-p4-submit &&
>> + git p4 submit --dry-run | grep "Would apply" &&
>> + echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
>> + git p4 submit --dry-run | grep "Would apply" || echo "Abort
>> submit"
>> + )
>> +'
>> +
>> test_expect_success 'submit from detached head' '
>> test_when_finished cleanup_git &&
>> git p4 clone --dest="$git" //depot &&
>> --
>> 2.18.0
>>
Re: [PATCH 1/1] add hook pre-p4-submit
On 23 July 2018 at 12:27, Chen Bin wrote:
> Hook pre-p4-submit is executed before git-p4 actually submits code.
> If the hook exits with non-zero value, submit process will abort.
Looks good to me - could you add some documentation please
(Documentation/git-p4.txt).
Thanks!
Luke
>
> Signed-off-by: Chen Bin
> ---
> git-p4.py | 6 ++
> t/t9800-git-p4-basic.sh | 22 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/git-p4.py b/git-p4.py
> index b449db1cc..69ee9ce41 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2303,6 +2303,12 @@ def run(self, args):
> sys.exit("number of commits (%d) must match number of shelved
> changelist (%d)" %
> (len(commits), num_shelves))
>
> +# locate hook at `.git/hooks/pre-p4-submit`
> +hook_file = os.path.join(read_pipe("git rev-parse
> --git-dir").strip(), "hooks", "pre-p4-submit")
> +# Execute hook. If it exit with non-zero value, do NOT continue.
> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
> subprocess.call([hook_file]) != 0:
> +sys.exit(1)
> +
> #
> # Apply the commits, one at a time. On failure, ask if should
> # continue to try the rest of the patches, or quit.
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> index 4849edc4e..48b768fa7 100755
> --- a/t/t9800-git-p4-basic.sh
> +++ b/t/t9800-git-p4-basic.sh
> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should
> display error' '
> )
> '
>
> +# Test following scenarios:
> +# - Without hook ".git/hooks/pre-p4-submit", submit should continue
> +# - With hook returning 0, submit should continue
> +# - With hook returning 1, submit should abort
> +test_expect_success 'run hook pre-p4-submit before submit' '
> + test_when_finished cleanup_git &&
> + git p4 clone --dest="$git" //depot &&
> + (
> + cd "$git" &&
> + echo "hello world" >hello.txt &&
> + git add hello.txt &&
> + git commit -m "add hello.txt" &&
> + git config git-p4.skipSubmitEdit true &&
> + git p4 submit --dry-run | grep "Would apply" &&
> + mkdir -p .git/hooks &&
> + : >.git/hooks/pre-p4-submit && chmod +x
> .git/hooks/pre-p4-submit &&
> + git p4 submit --dry-run | grep "Would apply" &&
> + echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
> + git p4 submit --dry-run | grep "Would apply" || echo "Abort
> submit"
> + )
> +'
> +
> test_expect_success 'submit from detached head' '
> test_when_finished cleanup_git &&
> git p4 clone --dest="$git" //depot &&
> --
> 2.18.0
>
Re: [PATCH 01/10] t: add tool to translate hash-related values
On Tue, Jul 24, 2018 at 6:17 PM brian m. carlson
wrote:
> On Mon, Jun 11, 2018 at 03:47:43AM -0400, Eric Sunshine wrote:
> > Here's what I had envisioned when reading your emails about OID lookup
> > table functionality:
> >
> > --- >8 ---
> > test_oid_cache () {
> > while read tag rest
> > do
> > case $tag in \#*) continue ;; esac
> >
> > for x in $rest
> > do
> > k=${x%:*}
> > v=${x#*:}
> > if test "$k" = $test_hash_algo
> > then
> > eval "test_oid_$tag=$v"
> > break
> > fi
> > done
> > done
> > }
> >
> > test_oid () {
> > if test $# -gt 1
> > then
> > test_oid_cache <<-EOF
> > $*
> > EOF
> > fi
> > eval "echo \$test_oid_$1"
> > }
> > --- >8 ---
>
> I'd like to use this as a basis for my v2, but I would need your
> sign-off in order to do that. Would that be okay?
You can have my sign-off for the code above.
[PATCH 1/2] doc hash-function-transition: note the lack of a changelog
The changelog embedded in the document pre-dates the addition of the
document to git.git (it used to be a Google Doc), so it only goes up
to 752414ae43 ("technical doc: add a design doc for hash function
transition", 2017-09-27).
Since then I made some small edits to it, which would have been worthy
of including in this changelog (but weren't). Instead of amending it
to include these, just note that future changes will be noted in the
log.
Signed-off-by: Ævar Arnfjörð Bjarmason
---
Documentation/technical/hash-function-transition.txt | 6 ++
1 file changed, 6 insertions(+)
diff --git a/Documentation/technical/hash-function-transition.txt
b/Documentation/technical/hash-function-transition.txt
index 4ab6cd1012..5ee4754adb 100644
--- a/Documentation/technical/hash-function-transition.txt
+++ b/Documentation/technical/hash-function-transition.txt
@@ -814,6 +814,12 @@ Incorporated suggestions from jonathantanmy and sbeller:
* avoid loose object overhead by packing more aggressively in
"git gc --auto"
+Later history:
+
+ See the history of this file in git.git for the history of subsequent
+ edits. This document history is no longer being maintained as it
+ would now be superfluous to the commit log
+
[1]
http://public-inbox.org/git/ca+55afzjtejicjv0e43+9or3qujk2pifilqemytolpyjwe6...@mail.gmail.com/
[2]
http://public-inbox.org/git/ca+55afz+gkasdz24zmepques1xps9bp_s8o7q4wq7lv7x5-...@mail.gmail.com/
[3]
http://public-inbox.org/git/20170306084353.nrns455dvkdsf...@sigill.intra.peff.net/
--
2.17.0.290.gded63e768a
[PATCH 2/2] doc hash-function-transition: pick SHA-256 as NewHash
The consensus on the mailing list seems to be that SHA-256 should be picked as our NewHash, see the "Hash algorithm analysis" thread as of [1]. Linus has come around to this choice and suggested Junio make the final pick, and he's endorsed SHA-256 [3]. This follow-up change changes occurrences of "NewHash" to "SHA-256" (or "sha256", depending on the context). The "Selection of a New Hash" section has also been changed to note that historically we used the the "NewHash" name while we didn't know what the new hash function would be. This leaves no use of "NewHash" anywhere in git.git except in the aforementioned section (and as a variable name in t/t9700/test.pl, but that use from 2008 has nothing to do with this transition plan). 1. https://public-inbox.org/git/20180720215220.gb18...@genre.crustytoothpaste.net/ 2. https://public-inbox.org/git/ca+55afwse9bf8e0hlk9pp3fvd5lavy5grdsv3fbntgzekja...@mail.gmail.com/ 3. https://public-inbox.org/git/xmqqzhygwd5o@gitster-ct.c.googlers.com/ Signed-off-by: Ævar Arnfjörð Bjarmason --- .../technical/hash-function-transition.txt| 186 +- 1 file changed, 96 insertions(+), 90 deletions(-) diff --git a/Documentation/technical/hash-function-transition.txt b/Documentation/technical/hash-function-transition.txt index 5ee4754adb..8fb2d4b498 100644 --- a/Documentation/technical/hash-function-transition.txt +++ b/Documentation/technical/hash-function-transition.txt @@ -59,14 +59,11 @@ that are believed to be cryptographically secure. Goals - -Where NewHash is a strong 256-bit hash function to replace SHA-1 (see -"Selection of a New Hash", below): - -1. The transition to NewHash can be done one local repository at a time. +1. The transition to SHA-256 can be done one local repository at a time. a. Requiring no action by any other party. - b. A NewHash repository can communicate with SHA-1 Git servers + b. A SHA-256 repository can communicate with SHA-1 Git servers (push/fetch). - c. Users can use SHA-1 and NewHash identifiers for objects + c. Users can use SHA-1 and SHA-256 identifiers for objects interchangeably (see "Object names on the command line", below). d. New signed objects make use of a stronger hash function than SHA-1 for their security guarantees. @@ -79,7 +76,7 @@ Where NewHash is a strong 256-bit hash function to replace SHA-1 (see Non-Goals - -1. Add NewHash support to Git protocol. This is valuable and the +1. Add SHA-256 support to Git protocol. This is valuable and the logical next step but it is out of scope for this initial design. 2. Transparently improving the security of existing SHA-1 signed objects. @@ -87,26 +84,26 @@ Non-Goals repository. 4. Taking the opportunity to fix other bugs in Git's formats and protocols. -5. Shallow clones and fetches into a NewHash repository. (This will - change when we add NewHash support to Git protocol.) -6. Skip fetching some submodules of a project into a NewHash - repository. (This also depends on NewHash support in Git +5. Shallow clones and fetches into a SHA-256 repository. (This will + change when we add SHA-256 support to Git protocol.) +6. Skip fetching some submodules of a project into a SHA-256 + repository. (This also depends on SHA-256 support in Git protocol.) Overview We introduce a new repository format extension. Repositories with this -extension enabled use NewHash instead of SHA-1 to name their objects. +extension enabled use SHA-256 instead of SHA-1 to name their objects. This affects both object names and object content --- both the names of objects and all references to other objects within an object are switched to the new hash function. -NewHash repositories cannot be read by older versions of Git. +SHA-256 repositories cannot be read by older versions of Git. -Alongside the packfile, a NewHash repository stores a bidirectional -mapping between NewHash and SHA-1 object names. The mapping is generated +Alongside the packfile, a SHA-256 repository stores a bidirectional +mapping between SHA-256 and SHA-1 object names. The mapping is generated locally and can be verified using "git fsck". Object lookups use this -mapping to allow naming objects using either their SHA-1 and NewHash names +mapping to allow naming objects using either their SHA-1 and SHA-256 names interchangeably. "git cat-file" and "git hash-object" gain options to display an object @@ -116,7 +113,7 @@ object database so that they can be named using the appropriate name (using the bidirectional hash mapping). Fetches from a SHA-1 based server convert the fetched objects into -NewHash form and record the mapping in the bidirectional mapping table +SHA-256 form and record the mapping in the bidirectional mapping table (see below for details). Pushes to a SHA-1 based server convert the objects being pushed into sha1 form so the server does not have to be aware of the hash function the
[PATCH 0/2] document that NewHash is now SHA-256
On Tue, Jul 24 2018, Junio C Hamano wrote: > Linus Torvalds writes: > >> On Tue, Jul 24, 2018 at 12:01 PM Edward Thomson >> wrote: >>> >>> Switching gears, if I look at this from the perspective of the libgit2 >>> project, I would also prefer SHA-256 or SHA3 over blake2b. To support >>> blake2b, we'd have to include - and support - that code ourselves. But >>> to support SHA-256, we would simply use the system's crypto libraries >>> that we already take a dependecy on (OpenSSL, mbedTLS, CryptoNG, or >>> SecureTransport). >> >> I think this is probably the single strongest argument for sha256. >> "It's just there". > > Yup. I actually was leaning toward saying "all of them are OK in > practice, so the person who is actually spear-heading the work gets > to choose", but if we picked SHA-256 now, that would not be a choice > that Brian has to later justify for choosing against everybody > else's wishes, which makes it the best choice ;-) Looks like it's settled then. I thought I'd do the grunt work of updating the relevant documentation so we can officially move on from the years-long NewHash discussion. Ævar Arnfjörð Bjarmason (2): doc hash-function-transition: note the lack of a changelog doc hash-function-transition: pick SHA-256 as NewHash .../technical/hash-function-transition.txt| 192 ++ 1 file changed, 102 insertions(+), 90 deletions(-) -- 2.17.0.290.gded63e768a
Re: [PATCH] diff: --color-moved: rename "dimmed_zebra" to "dimmed-zebra"
On Tue, Jul 24, 2018 at 6:09 PM Jonathan Nieder wrote: > Eric Sunshine wrote: > > Subject: diff: --color-moved: rename "dimmed_zebra" to "dimmed-zebra" > > It would be clearer to say something like > > diff --color-moved: add "dimmed-zebra" synonym for "dimmed_zebra" That could work, although doesn't really convey that "dimmed_zebra" is undesirable. Perhaps: diff --color-moved: add "dimmed-zebra"; deprecate "dimmed_zebra" Perhaps, not worth a re-roll, though(?). > > The --color-moved "dimmed_zebra" mode (with an underscore) is an > > anachronism. Most options and modes are hyphenated. It is more difficult > > to type and somewhat more difficult to read than those which are > > hyphenated. Therefore, rename it to "dimmed-zebra", and nominally > > deprecate "dimmed_zebra". > > Hm. Looks like dimmed_zebra was introduced in v2.15.0-rc0~16^2~2 > (2017-06-30), so it has been around for a while (about a year). But I > would like to be able to simplify by getting rid of it. > > Is there anything we can do to make it possible to remove eventually? > For example, should we (eventually, after dimmed-zebra has existed > for some time) start warning when people use dimmed_zebra to encourage > them to use dimmed-zebra instead? Wanting to simplify by eventually getting rid of the old name is understandable, but my hope is that we don't have to do so. Given that this nominal deprecation involves one short sentence in the documentation and one strcmp() for backward compatibility, it seems unlikely to become a maintenance burden. In contrast to "git branch -l", short for --create-reflog, but which people intuitively expect to mean --list, "freeing up" this name for some other purpose is unnecessary, so a concrete deprecation may be overkill. As well, the extra burden on Junio to hang onto deprecation and eventual removal patches far into the future seems unwarranted. > > +dimmed-zebra:: > > Similar to 'zebra', but additional dimming of uninteresting parts > > of moved code is performed. The bordering lines of two adjacent > > blocks are considered interesting, the rest is uninteresting. > > + `dimmed_zebra` is a deprecated synonym. > > Thanks for including that note. It means that when people see > dimmed_zebra in scripts or configuration they won't have to be > mystified about why it works. > > I don't have any good ideas about deprecating more aggressively, and > the patch looks good, so > > Reviewed-by: Jonathan Nieder Thanks for the review.
Re: [PATCH 7/9] vscode: use 8-space tabs, no trailing ws, etc for Git's source code
Am 23.07.2018 um 15:52 schrieb Johannes Schindelin via GitGitGadget:
From: Johannes Schindelin
This adds a couple settings for the .c/.h files so that it is easier to
conform to Git's conventions while editing the source code.
Signed-off-by: Johannes Schindelin
---
contrib/vscode/init.sh | 8
1 file changed, 8 insertions(+)
diff --git a/contrib/vscode/init.sh b/contrib/vscode/init.sh
index face115e8..29f2a729d 100755
--- a/contrib/vscode/init.sh
+++ b/contrib/vscode/init.sh
@@ -21,6 +21,14 @@ cat >.vscode/settings.json.new <<\EOF ||
"editor.wordWrap": "wordWrapColumn",
"editor.wordWrapColumn": 72
},
+"[c]": {
+"editor.detectIndentation": false,
+"editor.insertSpaces": false,
+"editor.tabSize": 8,
+"editor.wordWrap": "wordWrapColumn",
+"editor.wordWrapColumn": 80,
+"files.trimTrailingWhitespace": true
+},
I am a VS Code user, but I haven't used these settings before.
With these settings, does the editor break lines while I am typing? Or
does it just insert a visual cue that tells where I should insert a line
break? If the former, it would basically make the editor unusable for my
taste. I want to have total control over the code I write. The 80 column
limit is just a recommendation, not a hard requirement.
"files.associations": {
"*.h": "c",
"*.c": "c"
-- Hannes
Re: [PATCH 2/2] remote-odb: un-inline function remote_odb_reinit
Hi, On Wed, Jul 25, 2018 at 12:03 AM, Beat Bolli wrote: > > On 24.07.18 23:59, Jonathan Nieder wrote: >> >> Beat Bolli wrote: >>> -inline void remote_odb_reinit(void) >>> +void remote_odb_reinit(void) >> >> This looks like an oversight in >> https://public-inbox.org/git/20180713174959.16748-6-chrisc...@tuxfamily.org/: >> there isn't any reason for this function to be inline. >> >> Christian, can you squash it in on your next reroll? > > That would probably make sense. I didn't check how mature the topics > were that caused errors. Ok, it is in the next version I will send. Thanks, Christian.
