What's cooking in git.git (Jan 2015, #04; Wed, 21)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. First release candidate 2.3-rc1 has been tagged. Please spend some time to find and fix regressions, instead of spending all time having fun with new and shiny toys ;-) 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"] * jk/http-push-symref-fix (2015-01-14) 1 commit + http-push: trim trailing newline from remote symref Using newer libCURL (or old one with security fixes) exposes this old breakage. -- [New Topics] * ak/typofixes (2015-01-21) 2 commits - t/lib-terminal.sh: fix typo - pack-bitmap: fix typo Will merge to 'next'. * jc/apply-ws-fix-expands (2015-01-16) 4 commits - apply: detect and mark whitespace errors in context lines when fixing - apply: count the size of postimage correctly - typofix - apply: make update_pre_post_images() sanity check the given postlen Needs rerolling and adding tests from Kyle J. McKay. * jc/coding-guidelines (2015-01-15) 1 commit - CodingGuidelines: clarify C #include rules Will merge to 'next'. * jc/pretty-format-doc (2015-01-15) 1 commit - "log --pretty" documentation: do not forget "tformat:" Will merge to 'next'. * jc/unused-symbols (2015-01-15) 8 commits - shallow.c: make check_shallow_file_for_update() static - remote.c: make clear_cas_option() static - urlmatch.c: make match_urls() static - revision.c: make save_parents() and free_saved_parents() static - line-log.c: make line_log_data_init() static - pack-bitmap.c: make pack_bitmap_filename() static - prompt.c: remove git_getpass() nobody uses - http.c: make finish_active_slot() and handle_curl_result() static Will merge to 'next'. * jk/sanity (2015-01-16) 3 commits - tests: SANITY requires POSIXPERM - tests: correct misuses of POSIXPERM - t/lib-httpd: switch SANITY check for NOT_ROOT Saw Torsten's report on Cygwin, but I couldn't tell if it was about existing breakages or new one introduced by this series. * js/fsck-opt (2015-01-21) 19 commits - fsck: support ignoring objects in `git fsck` via fsck.skiplist - fsck: git receive-pack: support excluding objects from fsck'ing - fsck: introduce `git fsck --quick` - fsck: support demoting errors to warnings - fsck: document the new receive.fsck.* options - fsck: allow upgrading fsck warnings to errors - fsck: optionally ignore specific fsck issues completely - fsck: disallow demoting grave fsck errors to warnings - fsck: add a simple test for receive.fsck.* - fsck: make fsck_tag() warn-friendly - fsck: handle multiple authors in commits specially - fsck: make fsck_commit() warn-friendly - fsck: make fsck_ident() warn-friendly - fsck: report the ID of the error/warning - fsck: allow demoting errors to warnings via receive.fsck.warn = - fsck: offer a function to demote fsck errors to warnings - fsck: provide a function to parse fsck message IDs - fsck: introduce identifiers for fsck messages - fsck: introduce fsck options Need extra set of eyes to review this. * ld/p4-exclude-in-sync (2015-01-20) 1 commit - git-p4: support excluding paths on sync Will merge to 'next'. * tb/connect-ipv6-parse-fix (2015-01-20) 3 commits - t5500: Show user name and host in diag-url - t5601: Add more test cases for IPV6 - connect.c: Improve parsing of literal IPV6 addresses Need extra set of eyes to review this. * sb/atomic-push-fix (2015-01-21) 5 commits - refs.c: enable large transactions - refs.c: have a write_sha1_to_lock_file wrapper - refs.c: remove lock_fd from struct ref_lock - t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE - update-ref: test handling large transactions properly (this branch uses mh/reflog-expire and sb/atomic-push.) I had to wiggle this in and am not confident I did it correctly. Need extra set of eyes to review this. -- [Stalled] * jn/doc-api-errors (2014-12-04) 1 commit - doc: document error handling functions and conventions For discussion. * ye/http-accept-language (2015-01-21) 1 commit . http: add Accept-Language header if possible Not quite there yet. * pw/remote-set-url-fetch (2014-11-26) 1 commit - remote: add --fetch and --both options to set-url Expecting a reroll. * ms/submodule-update-config-doc (2014-11-03) 1 commit - submodule: clarify documentation for update subcommand Needs a reroll ($gmane/259037). * je/quiltimport-no-fuzz (2014-10-21) 2 commits - git-quiltimport: flip the default not to allow fuzz - git-quiltimport.sh: allow declining fuzz with --exact option "quiltimport" drove "git apply" always with -C1 option to reduce context of the patch in order to give more chance to som
Re: [PATCH v7 1/1] http: Add Accept-Language header if possible
Yi EungJun writes:
> +static void write_accept_language(struct strbuf *buf)
> +{
> + /*
> + * MAX_DECIMAL_PLACES must not be larger than 3. If it is larger than
> + * that, q-value will be smaller than 0.001, the minimum q-value the
> + * HTTP specification allows. See
> + * http://tools.ietf.org/html/rfc7231#section-5.3.1 for q-value.
> + */
> + const int MAX_DECIMAL_PLACES = 3;
> + const int MAX_LANGUAGE_TAGS = 1000;
> + const int MAX_ACCEPT_LANGUAGE_HEADER_SIZE = 4000;
> + struct strbuf *language_tags = NULL;
> + int num_langs;
No initial value given to this variable, but...
> + const char *s = get_preferred_languages();
> +
> + /* Don't add Accept-Language header if no language is preferred. */
> + if (!s)
> + return;
> +
> + /*
> + * Split the colon-separated string of preferred languages into
> + * language_tags array.
> + */
> + do {
> + /* increase language_tags array to add new language tag */
> + REALLOC_ARRAY(language_tags, num_langs + 1);
... it is nevertheless used. I think it was meant to start at 0?
> + /* write Accept-Language header into buf */
> + if (num_langs >= 1) {
> + int i;
> + int last_buf_len;
This is uninitialized...
> + int max_q;
> + int decimal_places;
> + char q_format[32];
> +
> +...
> + if (buf->len > MAX_ACCEPT_LANGUAGE_HEADER_SIZE) {
> + strbuf_remove(buf, last_buf_len, buf->len -
> last_buf_len);
... and then it is used here.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] transport-helper: do not request symbolic refs to remote helpers
Mike Hommey writes: > Note the most important part is actually between the parens: that only > happens when the remote helper returns '?' to the `list` command, which > non-git remotes helpers (like git-remote-hg or git-remote-bzr) do. > git-remote-testgit also does, so if you only apply the test parts of the > patch, you'll see that the test fails. > > remote-curl probably doesn't hit the problem because it's not returning > '?' to `list`. Hmm, that suggests that the new codepath should be taken only when the remote helper says '?' (does it mean "I dunno"? where are these documented, by the way?), yes? It wasn't immediately obvious from the patch text that it was the case. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-add--interactive: print message if there are no untracked files
No i don't see any reasons why list_and_choose() shoud give a prompt
without candidates. Will resed patch.
Thank you.
2015-01-22 3:17 GMT+06:00 Junio C Hamano :
> Junio C Hamano writes:
>
>>> sub add_untracked_cmd {
>>> -my @add = list_and_choose({ PROMPT => 'Add untracked' },
>>> - list_untracked());
>>> -if (@add) {
>>> -system(qw(git update-index --add --), @add);
>>> -say_n_paths('added', @add);
>>> +if (system(qw(git ls-files --others --exclude-standard --))) {
>>
>> But this ls-files invocation that knows too much about how
>> list_untracked() computes things does not.
>>
>> Why not
>> ...
>> or something instead?
>
> Actually, is there any case where list_and_choose() should give a
> prompt to choose from zero candidates?
>
> In other words, I am wondering if this affects other callers of
> list_and_choose in any negative way.
>
> git-add--interactive.perl | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 94b988c..46ed9a7 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -519,6 +519,10 @@ sub error_msg {
> sub list_and_choose {
> my ($opts, @stuff) = @_;
> my (@chosen, @return);
> +
> + if (!@stuff) {
> + return @return;
> + }
> my $i;
> my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
>
--
_
0xAX
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i18n: bundle: mark git-bundle usage for translation
Ok, understand. How to be with it? Resend after 2.3? Thank you. 2015-01-22 3:13 GMT+06:00 Junio C Hamano : > Thanks, but please hold this off until the 2.3 final is tagged. > > It is way too late for anything that is not a regression fix at this > point in this cycle, and changes that affect i18n are the last thing > we want to take late in the cycle because l10n people need time to > update the translations. -- _ 0xAX -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] transport-helper: do not request symbolic refs to remote helpers
On Wed, Jan 21, 2015 at 10:46:48PM -0800, Junio C Hamano wrote: > Mike Hommey writes: > > > A typical remote helper will return a `list` of refs containing a symbolic > > ref HEAD, pointing to, e.g. refs/heads/master. In the case of a clone, all > > the refs are being requested through `fetch` or `import`, including the > > symbolic ref. > > > > While this works properly, in some cases of a fetch, like `git fetch url` > > or `git fetch origin HEAD`, or any fetch command involving a symbolic ref > > without also fetching the corresponding ref it points to, the fetch command > > fails with: > > > > fatal: bad object > > error: did not send all necessary objects > > > > (in the case the remote helper returned '?' values to the `list` command). > > Hmph. > > Since the most "typical remote helper" I immediately think of is > remote-curl and "git fetch https://code.googlesource.com/git HEAD" > does not seem to fail that way, I am not sure what to make of the > above. It is unclear if you meant that the above is inherent due to > the way how remote helper protocol works (e.g. there is only one > thing we can associate with a ref and we cannot say "HEAD points at > this commit" at the same time we say "HEAD points at > refs/heads/master"), or just due to broken or lazy implementation of > the remote helpers that are invoked by transport-helper.c interface. Note the most important part is actually between the parens: that only happens when the remote helper returns '?' to the `list` command, which non-git remotes helpers (like git-remote-hg or git-remote-bzr) do. git-remote-testgit also does, so if you only apply the test parts of the patch, you'll see that the test fails. remote-curl probably doesn't hit the problem because it's not returning '?' to `list`. Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] rebase -i: respect core.abbrev for real
"Kirill A. Shutemov" writes:
> I have tried to fix this before: see 568950388be2, but it doesn't
> really work.
>
> I don't know how it happend, but that commit makes interactive rebase to
> respect core.abbrev only during --edit-todo, but not the initial todo
> list edit.
>
> For this time I've included a test-case to avoid this frustration again.
I suspect that the work that introduced expand/collapse could have
gone one step further so that all of the short object names you are
touching in this patch can stay in the full 40-hex name to avoid
ambiguity. That is, keep the internal representation always use the
full object name, and then always call these three:
collapse_todo_ids
git_sequence_editor
expand_todo_ids
in this order.
Wouldn't that make the end result a lot cleaner, I wonder?
>
> Signed-off-by: Kirill A. Shutemov
> ---
> v2: fix &&-chain in the test-case
> ---
> git-rebase--interactive.sh| 4 ++--
> t/t3404-rebase-interactive.sh | 7 +++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index c6a4629cbc2b..1855e12f1ada 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -962,7 +962,7 @@ else
> shortrevisions=$shorthead
> fi
> git rev-list $merges_option --pretty=oneline --abbrev-commit \
> - --abbrev=7 --reverse --left-right --topo-order \
> + --reverse --left-right --topo-order \
> $revisions ${restrict_revision+^$restrict_revision} | \
> sed -n "s/^>//p" |
> while read -r shortsha1 rest
> @@ -1020,7 +1020,7 @@ then
> # just the history of its first-parent for others that
> will
> # be rebasing on top of it
> git rev-list --parents -1 $rev | cut -d' ' -s -f2 >
> "$dropped"/$rev
> - short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
> + short=$(git rev-list -1 --abbrev-commit $rev)
> sane_grep -v "^[a-z][a-z]* $short" <"$todo" >
> "${todo}2" ; mv "${todo}2" "$todo"
> rm "$rewritten"/$rev
> fi
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 8197ed29a9ec..a31f7e0430e1 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
> )
> '
>
> +test_expect_success 'respect core.abbrev' '
> + git config core.abbrev 12 &&
> + set_cat_todo_editor &&
> + test_must_fail git rebase -i HEAD~4 >todo-list &&
> + test 4 = $(grep -c "pick [0-9a-f]\{12,\}" todo-list)
> +'
> +
> test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: add git apply whitespace expansion tests
On Jan 21, 2015, at 14:33, Junio C Hamano wrote: "Kyle J. McKay" writes: So since I've not been able to get test 2 or 3 to core dump (even before 250b3c6c) I tend to believe you are correct in that the code thinks (incorrectly) that the result should fit within the buffer. Thanks; let me steal your tests when I reroll. Awesome. :) But please squash in this tiny change if using the tests verbatim: On Jan 18, 2015, at 02:49, Kyle J. McKay wrote: +# +## create test-N, patchN.patch, expect-N files +# + +# test 1 +printf '\t%s\n' 1 2 3 4 5 6 > before +printf '\t%s\n' 1 2 3 > after +printf '%64s\n' a b c $x >> after This line ^ in test 1 should not have a '$x' in it. It should just be: +printf '%64s\n' a b c >> after The test runs fine currently, but if somehow x should get defined before running the tests, test 1 would fail. All the other '$x' in the other tests are correct. +printf '\t%s\n' 4 5 6 >> after +git diff --no-index before after | \ +sed -e 's/before/test-1/' -e 's/after/test-1/' > patch1.patch +printf '%64s\n' 1 2 3 4 5 6 > test-1 +printf '%64s\n' 1 2 3 a b c 4 5 6 > expect-1 + +# test 2 -Kyle -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] transport-helper: do not request symbolic refs to remote helpers
Mike Hommey writes: > A typical remote helper will return a `list` of refs containing a symbolic > ref HEAD, pointing to, e.g. refs/heads/master. In the case of a clone, all > the refs are being requested through `fetch` or `import`, including the > symbolic ref. > > While this works properly, in some cases of a fetch, like `git fetch url` > or `git fetch origin HEAD`, or any fetch command involving a symbolic ref > without also fetching the corresponding ref it points to, the fetch command > fails with: > > fatal: bad object > error: did not send all necessary objects > > (in the case the remote helper returned '?' values to the `list` command). Hmph. Since the most "typical remote helper" I immediately think of is remote-curl and "git fetch https://code.googlesource.com/git HEAD" does not seem to fail that way, I am not sure what to make of the above. It is unclear if you meant that the above is inherent due to the way how remote helper protocol works (e.g. there is only one thing we can associate with a ref and we cannot say "HEAD points at this commit" at the same time we say "HEAD points at refs/heads/master"), or just due to broken or lazy implementation of the remote helpers that are invoked by transport-helper.c interface. > This is because there is only one ref given to fetch(), and it's not > further resolved to something at the end of fetch_with_import(). There is no get_refs_list() or something similar involved? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 0/5] Fix bug in large transactions
version2: * This applies on top of origin/sb/atomic-push though it will result in a one line merge conflict with origin/jk/lock-ref-sha1-basic-return-errors when merging to origin/next. * It now uses the FILE* pointer instead of file descriptors. This results in a combination of the 2 former patches "refs.c: have a write_in_full_to_lock_file wrapper" and "refs.c: write to a lock file only once" as the wrapper function is more adapted to its consumers * no need to dance around with char *pointers which may leak. * another new patch sneaked into the series: Renaming ULIMIT in t7004 to ULIMIT_STACK_SIZE That said, only the first and third patch are updated from the first version of the patches. The others are new in the sense that rewriting them was cheaper than keeping notes in between. version1: (reported as: git update-ref --stdin : too many open files, 2014-12-20) First a test case is introduced to demonstrate the failure, the patches 2-6 are little refactoring and the last patch fixes the bug and also marks the bugs as resolved in the test suite. Unfortunately this applies on top of origin/next. Any feedback would be welcome! Thanks, Stefan Stefan Beller (5): update-ref: test handling large transactions properly t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE refs.c: remove lock_fd from struct ref_lock refs.c: have a write_sha1_to_lock_file wrapper refs.c: enable large transactions refs.c| 34 ++ t/t1400-update-ref.sh | 28 t/t7004-tag.sh| 4 ++-- 3 files changed, 52 insertions(+), 14 deletions(-) -- 2.2.1.62.g3f15098 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 5/5] refs.c: enable large transactions
By closing the file descriptors after creating the lock file we are not
limiting the size of the transaction by the number of available file
descriptors.
Signed-off-by: Stefan Beller
---
refs.c| 17 +
t/t1400-update-ref.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 2013d37..9d01102 100644
--- a/refs.c
+++ b/refs.c
@@ -3055,11 +3055,18 @@ int is_branch(const char *refname)
static int write_sha1_to_lock_file(struct ref_lock *lock,
const unsigned char *sha1)
{
- if (fdopen_lock_file(lock->lk, "w") < 0
- || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41)
+ if (lock->lk->fd == -1) {
+ if (reopen_lock_file(lock->lk) < 0
+ || fdopen_lock_file(lock->lk, "w") < 0
+ || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41
+ || close_lock_file(lock->lk) < 0)
+ return -1;
+ } else {
+ if (fdopen_lock_file(lock->lk, "w") < 0
+ || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41)
return -1;
- else
- return 0;
+ }
+ return 0;
}
/*
@@ -3761,6 +3768,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update->refname);
goto cleanup;
}
+ /* Do not keep all lock files open at the same time. */
+ close_lock_file(update->lock->lk);
}
/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 47d2fe9..c593a1d 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -979,7 +979,7 @@ run_with_limited_open_files () {
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
@@ -990,7 +990,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large
transaction creating branches
)
'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 3/5] refs.c: remove lock_fd from struct ref_lock
The 'lock_fd' is the same as 'lk->fd'. No need to store it twice so remove
it. You may argue this introduces more coupling as we need to know more
about the internals of the lock file mechanism, but this will be solved in
a later patch.
No functional changes intended.
Signed-off-by: Stefan Beller
---
refs.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 14e52ca..2472e61 100644
--- a/refs.c
+++ b/refs.c
@@ -11,7 +11,6 @@ struct ref_lock {
char *orig_ref_name;
struct lock_file *lk;
unsigned char old_sha1[20];
- int lock_fd;
int force_write;
};
@@ -2259,7 +2258,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int attempts_remaining = 3;
lock = xcalloc(1, sizeof(struct ref_lock));
- lock->lock_fd = -1;
if (mustexist)
resolve_flags |= RESOLVE_REF_READING;
@@ -2335,8 +2333,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
- lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
- if (lock->lock_fd < 0) {
+ if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
if (errno == ENOENT && --attempts_remaining > 0)
/*
* Maybe somebody just deleted one of the
@@ -2904,7 +2901,6 @@ static int close_ref(struct ref_lock *lock)
{
if (close_lock_file(lock->lk))
return -1;
- lock->lock_fd = -1;
return 0;
}
@@ -2912,7 +2908,6 @@ static int commit_ref(struct ref_lock *lock)
{
if (commit_lock_file(lock->lk))
return -1;
- lock->lock_fd = -1;
return 0;
}
@@ -3090,8 +3085,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full(lock->lock_fd, &term, 1) != 1 ||
+ if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
+ write_in_full(lock->lk->fd, &term, 1) != 1 ||
close_ref(lock) < 0) {
int save_errno = errno;
error("Couldn't write %s", lock->lk->filename.buf);
@@ -4047,9 +4042,9 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
status |= error("couldn't write %s: %s", log_file,
strerror(errno));
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
- (write_in_full(lock->lock_fd,
+ (write_in_full(lock->lk->fd,
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock->lock_fd, "\n") != 1 ||
+write_str_in_full(lock->lk->fd, "\n") != 1 ||
close_ref(lock) < 0)) {
status |= error("couldn't write %s",
lock->lk->filename.buf);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 1/5] update-ref: test handling large transactions properly
Signed-off-by: Stefan Beller
---
t/t1400-update-ref.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7b4707b..47d2fe9 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -973,4 +973,32 @@ test_expect_success 'stdin -z delete refs works with
packed and loose refs' '
test_must_fail git rev-parse --verify -q $c
'
+run_with_limited_open_files () {
+ (ulimit -n 32 && "$@")
+}
+
+test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
+
+test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+(
+ for i in $(test_seq 33)
+ do
+ echo "create refs/heads/$i HEAD"
+ done >large_input &&
+ run_with_limited_open_files git update-ref --stdin large_input &&
+ run_with_limited_open_files git update-ref --stdin http://vger.kernel.org/majordomo-info.html
[PATCHv2 2/5] t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE
During creation of the patch series our discussion we could have a
more descriptive name for the prerequisite for the test so it stays
unique when other limits of ulimit are introduced.
Signed-off-by: Stefan Beller
---
t/t7004-tag.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 796e9f7..06b8e0d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1463,10 +1463,10 @@ run_with_limited_stack () {
(ulimit -s 128 && "$@")
}
-test_lazy_prereq ULIMIT 'run_with_limited_stack true'
+test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
# we require ulimit, this excludes Windows
-test_expect_success ULIMIT '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
>expect &&
i=1 &&
while test $i -lt 8000
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 4/5] refs.c: have a write_sha1_to_lock_file wrapper
Now we only have one place where we need to touch the internals of
the lock_file struct.
No functional changes intended.
Signed-off-by: Stefan Beller
---
refs.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 2472e61..2013d37 100644
--- a/refs.c
+++ b/refs.c
@@ -3052,6 +3052,16 @@ int is_branch(const char *refname)
return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
}
+static int write_sha1_to_lock_file(struct ref_lock *lock,
+ const unsigned char *sha1)
+{
+ if (fdopen_lock_file(lock->lk, "w") < 0
+ || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41)
+ return -1;
+ else
+ return 0;
+}
+
/*
* Write sha1 into the ref specified by the lock. Make sure that errno
* is sane on error.
@@ -3059,7 +3069,6 @@ int is_branch(const char *refname)
static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
- static char term = '\n';
struct object *o;
if (!lock) {
@@ -3085,8 +3094,7 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full(lock->lk->fd, &term, 1) != 1 ||
+ if (write_sha1_to_lock_file(lock, sha1) ||
close_ref(lock) < 0) {
int save_errno = errno;
error("Couldn't write %s", lock->lk->filename.buf);
@@ -4042,9 +4050,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
status |= error("couldn't write %s: %s", log_file,
strerror(errno));
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
- (write_in_full(lock->lk->fd,
- sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock->lk->fd, "\n") != 1 ||
+ (write_sha1_to_lock_file(lock, cb.last_kept_sha1) ||
close_ref(lock) < 0)) {
status |= error("couldn't write %s",
lock->lk->filename.buf);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: add support for --dry-run option
On Mon, Jan 19, 2015 at 03:20:51PM +0100, Michael J Gruber wrote: > Alexander Kuleshov schrieb am 17.01.2015 um 08:35: > > This patch adds support -d/--dry-run option for branch(es) deletion. > > If -d/--dry-run option passed to git branch -d branch..., branch(es) > > will not be removed, instead just print list of branches that are > > to be removed. > > > > For example: > > > > $ git branch > > a > > b > > c > > * master > > > > $ git branch -d -n a b c > > delete branch 'a' (261c0d1) > > delete branch 'b' (261c0d1) > > delete branch 'c' (261c0d1) > > Is there a case where deleting "a b c" would not delete "a b c"? Sure: $ cd /tmp/ $ git init foo Initialized empty Git repository in /tmp/foo/.git/ $ cd foo/ $ touch .gitignore $ git add .gitignore $ git commit -m init [master (root-commit) fde5138] init 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 .gitignore $ git checkout -b a Switched to a new branch 'a' $ git branch -d a error: Cannot delete the branch 'a' which you are currently on. $ touch file $ git add file $ git commit -m 'add file' [a e2c2ece] add file 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 file $ git checkout -b b master Switched to a new branch 'b' $ git branch -d a error: The branch 'a' is not fully merged. If you are sure you want to delete it, run 'git branch -D a'. -- Scott Schmit smime.p7s Description: S/MIME cryptographic signature
Re: [BUG?] setting ulimit in test suite broken for me
On Wed, Jan 21, 2015 at 11:03 AM, Jeff King wrote: > > in your debugging statement (and of course use run_with... for the > actual git command you want to limit). > Thanks for that hint, its now part of the bugfix series. Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] refs.c: replace write_str_in_full by write_in_full
On Wed, Jan 21, 2015 at 03:44:36PM -0800, Stefan Beller wrote:
> On Wed, Jan 21, 2015 at 3:38 PM, Jeff King wrote:
> > On Wed, Jan 21, 2015 at 03:23:42PM -0800, Stefan Beller wrote:
> >
> >> There is another occurrence where we could have used write_str_in_full
> >> (line 3107: write_in_full(lock->lk->fd, &term, 1)), so the current state
> >> is inconsistent. This replaces the only occurrence of write_str_in_full
> >> by write_in_full, so we only need to wrap write_in_full in the next patch.
> >
> > I had to read the first sentence a few times to figure out what you
> > meant. But I am not sure it is even relevant. We do not care about the
> > inconsistency.
>
> You're not the first who needs to reread my stuff :/
> I have the impression my English worsened since coming into the USA.
Actually, it was my fault in this case. I read it as "_this_ is another
occurrence", and then I scratched my head wondering what the first
occurrence was (was there a previous change that you should have been
referencing?). I finally got it on the third try. :)
> We do not care about the inconsistency, but we may care about the
> change itself: "write_str_in_full is way better than write_in_full, so
> why the step backwards?" And I am trying to explain that this is not
> a huge step backwards but rather improves consistency.
But you could improve consistency by going the other way, too. :) I
think the point is that you should lead in with the _real_ reason for
the change, not justifications. You can put in the justifications, too,
for the people who say "wait, but couldn't you do this other thing...".
> > It is just "we are about to change how callers of
> > write_in_full in this file behave, the wrapper gets in the way, and it
> > does not add enough value by itself to merit making our future changes
> > in two places".
>
> That's actually true. Though that sounds as if we'd be lazy ("we only
> want to make
> one change, so let's bend over here")
It's not laziness. It's avoiding duplicating logic, which would end up
costing more lines and providing worse maintainability than just
dropping the wrapper, which is after all only saving us a few characters
(and not anything conceptually hard).
> I'll rethink the commit message.
Everything I said above is rather subjective, of course. I do appreciate
you breaking your commits apart and explaining each one in the first
place. IOW, while I have thoughts on improving them (obviously), the
current iteration is not so bad that I would be upset to see it go into
git. Don't waste too much time on it.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv1 0/6] Fix bug in large transactions
On Wed, Jan 21, 2015 at 03:23:39PM -0800, Stefan Beller wrote: > (reported as: git update-ref --stdin : too many open files, 2014-12-20) > > First a test case is introduced to demonstrate the failure, > the patches 2-6 are little refactoring and the last patch > fixes the bug and also marks the bugs as resolved in the > test suite. > > Unfortunately this applies on top of origin/next. Saying "applies on next" is not very useful to Junio. He is not going to branch a topic straight from "next", as merging it to master would pull in all of the topics cooking in "next" (not to mention a bunch of merge commits which are generally never part of "master"). Instead, figure out which topic in next you actually _need_ to build on, and then it can be branched from there. And if there is no such topic, then you should not be building on next, of course. :) But I think you know that part already. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] refs.c: write to a lock file only once
On Wed, Jan 21, 2015 at 03:23:44PM -0800, Stefan Beller wrote:
> + const char *sha1_lf;
>
> if (!lock) {
> errno = EINVAL;
> @@ -3104,8 +3104,9 @@ static int write_ref_sha1(struct ref_lock *lock,
> errno = EINVAL;
> return -1;
> }
> - if (write_in_full_to_lockfile(lock->lk, sha1_to_hex(sha1), 40) != 40 ||
> - write_in_full_to_lockfile(lock->lk, &term, 1) != 1 ||
> +
> + sha1_lf = xstrfmt("%s\n", sha1_to_hex(sha1));
> + if (write_in_full_to_lockfile(lock->lk, sha1_lf, 41) != 41 ||
> close_ref(lock) < 0) {
> int save_errno = errno;
> error("Couldn't write %s", lock->lk->filename.buf);
> @@ -3113,6 +3114,7 @@ static int write_ref_sha1(struct ref_lock *lock,
> errno = save_errno;
> return -1;
> }
> + free((void*)sha1_lf);
It looks like you leak sha1_lf in the error code path here.
It's a shame that we must allocate at all, when we are really just
passing through to a buffer. Lockfiles have a "FILE *" pointer these
days. Could we just allow:
lockfile_printf(lock->lk, "%s\n", sha1_to_hex(sha1));
or similar?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] refs.c: replace write_str_in_full by write_in_full
On Wed, Jan 21, 2015 at 3:38 PM, Jeff King wrote:
> On Wed, Jan 21, 2015 at 03:23:42PM -0800, Stefan Beller wrote:
>
>> There is another occurrence where we could have used write_str_in_full
>> (line 3107: write_in_full(lock->lk->fd, &term, 1)), so the current state
>> is inconsistent. This replaces the only occurrence of write_str_in_full
>> by write_in_full, so we only need to wrap write_in_full in the next patch.
>
> I had to read the first sentence a few times to figure out what you
> meant. But I am not sure it is even relevant. We do not care about the
> inconsistency.
You're not the first who needs to reread my stuff :/
I have the impression my English worsened since coming into the USA.
We do not care about the inconsistency, but we may care about the change itself:
"write_str_in_full is way better than write_in_full, so why the step backwards?"
And I am trying to explain that this is not a huge step backwards but
rather improves
consistency.
> It is just "we are about to change how callers of
> write_in_full in this file behave, the wrapper gets in the way, and it
> does not add enough value by itself to merit making our future changes
> in two places".
That's actually true. Though that sounds as if we'd be lazy ("we only
want to make
one change, so let's bend over here")
I'll rethink the commit message.
Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] refs.c: Have a write_in_full_to_lock_file wrapping write_in_full
On Wed, Jan 21, 2015 at 03:23:43PM -0800, Stefan Beller wrote: > Now we only have one place where we need to touch the internals of > the lock_file struct. I wonder if the point of the previous patch would be more obvious if it were combined with this one. I am OK leaving them separate, though. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] refs.c: replace write_str_in_full by write_in_full
On Wed, Jan 21, 2015 at 03:23:42PM -0800, Stefan Beller wrote: > There is another occurrence where we could have used write_str_in_full > (line 3107: write_in_full(lock->lk->fd, &term, 1)), so the current state > is inconsistent. This replaces the only occurrence of write_str_in_full > by write_in_full, so we only need to wrap write_in_full in the next patch. I had to read the first sentence a few times to figure out what you meant. But I am not sure it is even relevant. We do not care about the inconsistency. It is just "we are about to change how callers of write_in_full in this file behave, the wrapper gets in the way, and it does not add enough value by itself to merit making our future changes in two places". -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] update-ref: Test handling large transactions properly
On Wed, Jan 21, 2015 at 03:23:40PM -0800, Stefan Beller wrote:
> Signed-off-by: Stefan Beller
> ---
> t/t1400-update-ref.sh | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index 6805b9e..ea98b9b 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -1065,4 +1065,32 @@ test_expect_success 'stdin -z delete refs works with
> packed and loose refs' '
> test_must_fail git rev-parse --verify -q $c
> '
>
> +run_with_limited_open_files () {
> + (ulimit -n 32 && "$@")
> +}
> +
> +test_lazy_prereq ULIMIT 'run_with_limited_open_files true'
We already have a ULIMIT prereq in t7004 that does something similar but
different. The two do not conflict as long as they are in separate
scripts, but they would if one gets moved into test-lib.sh. Should we
maybe give these more descriptive names? It is not just about "ulimit",
but the individual limit option. I can imagine a platform where "ulimit
-s" works, but "ulimit -n" does not (or the other way around).
I almost also suggested that the two "ulimit -s" instances share the
same function and lazy prereq, but I think that's probably not a good
idea. One cares about limiting the stack, and the other care about
limiting the cmdline size. The latter _happens_ to be done using "ulimit
-s". That works on Linux, but no clue about elsewhere. I could easily
imagine a platform where there is some other way, and we add a run-time
switch.
> +test_expect_failure ULIMIT 'large transaction creating branches does not
> burst open file limit' '
> +(
> + for i in $(seq 33)
Use test_seq here, for portability.
> +test_expect_failure ULIMIT 'large transaction deleting branches does not
> burst open file limit' '
> +(
> + for i in $(seq 33)
Ditto here.
The rest of the tests looked good to me.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] refs.c: remove lock_fd from struct ref_lock
The 'lock_fd' is the same as 'lk->fd'. No need to store it twice so remove
it. You may argue this introduces more coupling as we need to know more
about the internals of the lock file mechanism, but this will be solved in
a later patch.
No functional changes intended.
Signed-off-by: Stefan Beller
---
refs.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index ab2f2a9..e905f51 100644
--- a/refs.c
+++ b/refs.c
@@ -11,7 +11,6 @@ struct ref_lock {
char *orig_ref_name;
struct lock_file *lk;
unsigned char old_sha1[20];
- int lock_fd;
int force_write;
};
@@ -2262,7 +2261,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int attempts_remaining = 3;
lock = xcalloc(1, sizeof(struct ref_lock));
- lock->lock_fd = -1;
if (mustexist)
resolve_flags |= RESOLVE_REF_READING;
@@ -2338,8 +2336,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
- lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
- if (lock->lock_fd < 0) {
+ if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
last_errno = errno;
if (errno == ENOENT && --attempts_remaining > 0)
/*
@@ -2916,7 +2913,6 @@ static int close_ref(struct ref_lock *lock)
{
if (close_lock_file(lock->lk))
return -1;
- lock->lock_fd = -1;
return 0;
}
@@ -2924,7 +2920,6 @@ static int commit_ref(struct ref_lock *lock)
{
if (commit_lock_file(lock->lk))
return -1;
- lock->lock_fd = -1;
return 0;
}
@@ -3102,8 +3097,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full(lock->lock_fd, &term, 1) != 1 ||
+ if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
+ write_in_full(lock->lk->fd, &term, 1) != 1 ||
close_ref(lock) < 0) {
int save_errno = errno;
error("Couldn't write %s", lock->lk->filename.buf);
@@ -4083,9 +4078,9 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
status |= error("couldn't write %s: %s", log_file,
strerror(errno));
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
- (write_in_full(lock->lock_fd,
+ (write_in_full(lock->lk->fd,
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock->lock_fd, "\n") != 1 ||
+write_str_in_full(lock->lk->fd, "\n") != 1 ||
close_ref(lock) < 0)) {
status |= error("couldn't write %s",
lock->lk->filename.buf);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] refs.c: Have a write_in_full_to_lock_file wrapping write_in_full
Now we only have one place where we need to touch the internals of
the lock_file struct.
No functional changes intended.
Signed-off-by: Stefan Beller
---
refs.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 8281bed..311599b 100644
--- a/refs.c
+++ b/refs.c
@@ -3064,6 +3064,13 @@ int is_branch(const char *refname)
return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
}
+static ssize_t write_in_full_to_lockfile(struct lock_file *lock,
+const void *buf,
+size_t count)
+{
+ return write_in_full(lock->fd, buf, count);
+}
+
/*
* Write sha1 into the ref specified by the lock. Make sure that errno
* is sane on error.
@@ -3097,8 +3104,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full(lock->lk->fd, &term, 1) != 1 ||
+ if (write_in_full_to_lockfile(lock->lk, sha1_to_hex(sha1), 40) != 40 ||
+ write_in_full_to_lockfile(lock->lk, &term, 1) != 1 ||
close_ref(lock) < 0) {
int save_errno = errno;
error("Couldn't write %s", lock->lk->filename.buf);
@@ -4078,9 +4085,9 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
status |= error("couldn't write %s: %s", log_file,
strerror(errno));
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
- (write_in_full(lock->lk->fd,
+ (write_in_full_to_lockfile(lock->lk,
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_in_full(lock->lk->fd, "\n", 1) != 1 ||
+write_in_full_to_lockfile(lock->lk, "\n", 1) != 1 ||
close_ref(lock) < 0)) {
status |= error("couldn't write %s",
lock->lk->filename.buf);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] update-ref: Test handling large transactions properly
Signed-off-by: Stefan Beller
---
t/t1400-update-ref.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 6805b9e..ea98b9b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1065,4 +1065,32 @@ test_expect_success 'stdin -z delete refs works with
packed and loose refs' '
test_must_fail git rev-parse --verify -q $c
'
+run_with_limited_open_files () {
+ (ulimit -n 32 && "$@")
+}
+
+test_lazy_prereq ULIMIT 'run_with_limited_open_files true'
+
+test_expect_failure ULIMIT 'large transaction creating branches does not burst
open file limit' '
+(
+ for i in $(seq 33)
+ do
+ echo "create refs/heads/$i HEAD"
+ done >large_input &&
+ run_with_limited_open_files git update-ref --stdin large_input &&
+ run_with_limited_open_files git update-ref --stdin http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] refs.c: enable large transactions
By closing the file descriptors after creating the lock file we are not
limiting the size of the transaction by the number of available file
descriptors.
Signed-off-by: Stefan Beller
---
refs.c| 14 +-
t/t1400-update-ref.sh | 4 ++--
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 0161667..014ef59 100644
--- a/refs.c
+++ b/refs.c
@@ -3068,7 +3068,17 @@ static ssize_t write_in_full_to_lockfile(struct
lock_file *lock,
const void *buf,
size_t count)
{
- return write_in_full(lock->fd, buf, count);
+ if (lock->fd == -1) {
+ int ret;
+ if (reopen_lock_file(lock) < 0)
+ return -1;
+ ret = write_in_full(lock->fd, buf, count);
+ if (close_lock_file(lock) < 0)
+ return -1;
+ return ret;
+ } else {
+ return write_in_full(lock->fd, buf, count);
+ }
}
/*
@@ -3797,6 +3807,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update->refname);
goto cleanup;
}
+ /* Do not keep all lock files open at the same time. */
+ close_lock_file(update->lock->lk);
}
/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index ea98b9b..751b6dc 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1071,7 +1071,7 @@ run_with_limited_open_files () {
test_lazy_prereq ULIMIT 'run_with_limited_open_files true'
-test_expect_failure ULIMIT 'large transaction creating branches does not burst
open file limit' '
+test_expect_success ULIMIT 'large transaction creating branches does not burst
open file limit' '
(
for i in $(seq 33)
do
@@ -1082,7 +1082,7 @@ test_expect_failure ULIMIT 'large transaction creating
branches does not burst o
)
'
-test_expect_failure ULIMIT 'large transaction deleting branches does not burst
open file limit' '
+test_expect_success ULIMIT 'large transaction deleting branches does not burst
open file limit' '
(
for i in $(seq 33)
do
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] refs.c: write to a lock file only once
Instead of having to call write_in_full_to_lock_file twice get a formatted
string such that we only need to invoke writing to the lock file once.
This is helpful for the next patch when we only open the file descriptors
as needed. The lock file API has a reopen_lock_file which currently
doesn't open for appending.
No functional changes intended.
Signed-off-by: Stefan Beller
---
refs.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 311599b..0161667 100644
--- a/refs.c
+++ b/refs.c
@@ -3078,8 +3078,8 @@ static ssize_t write_in_full_to_lockfile(struct lock_file
*lock,
static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
- static char term = '\n';
struct object *o;
+ const char *sha1_lf;
if (!lock) {
errno = EINVAL;
@@ -3104,8 +3104,9 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full_to_lockfile(lock->lk, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full_to_lockfile(lock->lk, &term, 1) != 1 ||
+
+ sha1_lf = xstrfmt("%s\n", sha1_to_hex(sha1));
+ if (write_in_full_to_lockfile(lock->lk, sha1_lf, 41) != 41 ||
close_ref(lock) < 0) {
int save_errno = errno;
error("Couldn't write %s", lock->lk->filename.buf);
@@ -3113,6 +3114,7 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = save_errno;
return -1;
}
+ free((void*)sha1_lf);
clear_loose_ref_cache(&ref_cache);
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
@@ -4081,13 +4083,13 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
(*cleanup_fn)(cb.policy_cb);
if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
+ const char *sha1_lf = xstrfmt("%s\n",
+ sha1_to_hex(cb.last_kept_sha1));
if (close_lock_file(&reflog_lock)) {
status |= error("couldn't write %s: %s", log_file,
strerror(errno));
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
- (write_in_full_to_lockfile(lock->lk,
- sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_in_full_to_lockfile(lock->lk, "\n", 1) != 1 ||
+ (write_in_full_to_lockfile(lock->lk, sha1_lf, 41) != 41
||
close_ref(lock) < 0)) {
status |= error("couldn't write %s",
lock->lk->filename.buf);
@@ -4098,6 +4100,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
commit_ref(lock)) {
status |= error("couldn't set %s", lock->ref_name);
}
+ free((void*)sha1_lf);
}
free(log_file);
unlock_ref(lock);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] refs.c: replace write_str_in_full by write_in_full
There is another occurrence where we could have used write_str_in_full
(line 3107: write_in_full(lock->lk->fd, &term, 1)), so the current state
is inconsistent. This replaces the only occurrence of write_str_in_full
by write_in_full, so we only need to wrap write_in_full in the next patch.
No functional changes intended.
Signed-off-by: Stefan Beller
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index e905f51..8281bed 100644
--- a/refs.c
+++ b/refs.c
@@ -4080,7 +4080,7 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
(write_in_full(lock->lk->fd,
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock->lk->fd, "\n") != 1 ||
+write_in_full(lock->lk->fd, "\n", 1) != 1 ||
close_ref(lock) < 0)) {
status |= error("couldn't write %s",
lock->lk->filename.buf);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv1 0/6] Fix bug in large transactions
(reported as: git update-ref --stdin : too many open files, 2014-12-20) First a test case is introduced to demonstrate the failure, the patches 2-6 are little refactoring and the last patch fixes the bug and also marks the bugs as resolved in the test suite. Unfortunately this applies on top of origin/next. Any feedback would be welcome! Thanks, Stefan Stefan Beller (6): update-ref: Test handling large transactions properly refs.c: remove lock_fd from struct ref_lock refs.c: replace write_str_in_full by write_in_full refs.c: Have a write_in_full_to_lock_file wrapping write_in_full refs.c: write to a lock file only once refs.c: enable large transactions refs.c| 41 + t/t1400-update-ref.sh | 28 2 files changed, 57 insertions(+), 12 deletions(-) -- 2.2.1.62.g3f15098 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: add git apply whitespace expansion tests
"Kyle J. McKay" writes: > So since I've not been able to get test 2 or 3 to core dump (even > before 250b3c6c) I tend to believe you are correct in that the code > thinks (incorrectly) that the result should fit within the buffer. Thanks; let me steal your tests when I reroll. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen writes: > Hm, being one day offline and there are lots of ideas and > new patches, I like that. > I run these test under msys and cygwin on latest pu (a3dc223ff234481356c): > ... > (msys passes or skips all) > > Without digging further, these fail on my cygwin: > ... > I'm not sure what is the best way forward, it seems as if CYGIN is "half > POSIX" now. Are you reporting differences between the state before these patches and after, or just the fact that with these patches the named tests break (which may or may not be broken before the patches)? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] .clang-format: introduce the use of clang-format
On Wed, Jan 21, 2015 at 04:28:00PM -0500, Ramkumar Ramachandra wrote:
> > So overall I think it has some promise, but I do not think it is quite
> > flexible enough yet for us to use day-to-day.
>
> The big negative is that it will probably never be. I'll try to look
> at the larger issues later this week, if you can compromise on the
> fine details that are probably too hard to fix.
The key to me is that we do not have necessarily take every suggestion
the tool makes. So it does not have to be perfect, just "pretty good".
But...I think it is not quite so simple. The clang-format-diff script
(and git-clang-format) _do_ seem to operate on more than just the lines
I've changed. I'm not clear on whether they're examining the whole file
(just with the patches applied), or there's something in between
happening.
So rejecting the tool's suggestion one day may mean it suggests the same
change to you any other time you touch nearby parts of the file, which
could be annoying.
> > 2. It can operate on patches (and generates patches for you to apply!
> > You could add a git-add--interactive mode to selectively take its
> > suggestions).
>
> There's a git-clang-format in the $CLANG_ROOT/tools/clang-format/. I do:
>
>$ g cf @~
>
> ... with the appropriate aliases.
Neat. Debian's package does not ship with that. I hacked-in very
rudimentary interactive-add support for clang-format-diff (below) before
getting your response. It would be better built around "git-clang-format
--diff" (though that script would need to be taught to do the right
thing with the --color argument).
However, because of the "suggest the same change" thing I mentioned
above, I am not sure whether interactively selecting is a good idea or
not. You might end up having to say "no" to the same suggestions a lot.
Anyway, here it is, for reference. You can use it like:
git add--interactive --patch=format --
and you could probably even stick an "exec" line into an interactive
rebase to go through and fixup individual patches in a whole series.
---
diff --git a/.gitignore b/.gitignore
index a052419..6f5b815 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,7 @@
/git-checkout-index
/git-cherry
/git-cherry-pick
+/git-clang-format-diff
/git-clean
/git-clone
/git-column
diff --git a/Makefile b/Makefile
index c44eb3a..113534e 100644
--- a/Makefile
+++ b/Makefile
@@ -455,6 +455,7 @@ unexport CDPATH
SCRIPT_SH += git-am.sh
SCRIPT_SH += git-bisect.sh
+SCRIPT_SH += git-clang-format-diff.sh
SCRIPT_SH += git-difftool--helper.sh
SCRIPT_SH += git-filter-branch.sh
SCRIPT_SH += git-merge-octopus.sh
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index c725674..fd83adf 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -167,6 +167,16 @@ my %patch_modes = (
FILTER => undef,
IS_REVERSE => 0,
},
+ 'format' => {
+ DIFF => 'clang-format-diff',
+ APPLY => sub { apply_patch_for_checkout_commit '', @_ },
+ APPLY_CHECK => 'apply',
+ VERB => 'Apply',
+ TARGET => 'to index and worktree',
+ PARTICIPLE => 'applying',
+ FILTER => undef,
+ IS_REVERSE => 0
+ },
);
my %patch_mode_flavour = %{$patch_modes{stage}};
@@ -1591,6 +1601,15 @@ sub process_args {
'checkout_head' :
'checkout_nothead');
$arg = shift @ARGV or die "missing --";
}
+ } elsif ($1 eq 'format') {
+ $patch_mode = $1;
+ $arg = shift @ARGV or die "missing --";
+ if ($arg eq '--') {
+ $patch_mode_revision = 'HEAD^';
+ } else {
+ $patch_mode_revision = $arg;
+ $arg = shift @ARGV or die "missing --";
+ }
} elsif ($1 eq 'stage' or $1 eq 'stash') {
$patch_mode = $1;
$arg = shift @ARGV or die "missing --";
diff --git a/git-clang-format-diff.sh b/git-clang-format-diff.sh
new file mode 100755
index 000..9351883
--- /dev/null
+++ b/git-clang-format-diff.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+# This is what it's called in the Debian package, but it seems
+# like there ought to be a symlink without the version...
+CFD=clang-format-diff-3.6
+
+# Strip out --color, as clang's patch reader cannot handle it.
+# Robustly handling arrays in bourne shell is insane.
+eval "set -- $(
+ for i in "$@"; do
+ test "--color" = "$i" && continue
+ printf " '"
+ printf '%s' "$i" | sed "s/'/'''/g"
+ printf "'"
+ done
+)"
+
+git diff-index
Re: [PATCH v2 05/18] fsck: Allow demoting errors to warnings via receive.fsck.warn =
Johannes Schindelin writes:
>>> @@ -1488,8 +1501,13 @@ static const char *unpack(int err_fd, struct
>>> shallow_info *si)
>>>
>>> argv_array_pushl(&child.args, "index-pack",
>>> "--stdin", hdr_arg, keep_arg, NULL);
>>> - if (fsck_objects)
>>> - argv_array_push(&child.args, "--strict");
>>> + if (fsck_objects) {
>>> + if (fsck_severity.len)
>>> + argv_array_pushf(&child.args, "--strict=%s",
>>> + fsck_severity.buf);
>>> + else
>>> + argv_array_push(&child.args, "--strict");
>>> + }
>>
>> Hmm. The above two hunks look suspiciously similar. Would it be
>> worth to give them a single helper function?
>
> Hmm. Not sure. I see what you mean, but for now I found
>
> + argv_array_pushf(&child.args, "--strict%s%s",
> + fsck_severity.len ? "=" : "",
> + fsck_severity.buf);
>
> to be more elegant than to add a fully-fledged new function. But if
> you feel strongly, I will gladly implement a separate function; I
> would appreciate suggestions as to the function name...
Peff first introduced that trick elsewhere in our codebase, I think,
but I find it a bit too ugly.
As you accumulate fsck_severity strbuf like this anyway:
strbuf_addf(&fsck_severity, "%s%s=%s",
fsck_severity.len ? "," : "", var, value);
to flip what to prefix each element on the list with, I wonder if it
is simpler to change that empty string to "=", which will allow you
to say this:
argv_array_pushf(&child.args, "--strict%s", fsck_severity.buf);
Or even this:
strbuf_addf(&fsck_strict_arg, "%s%s=%s",
fsck_strict_arg.len ? "," : "--strict=", var, value);
and then the child.args stuff can become
if (fsck_strict_arg.len)
argv_array_push(&child.args, fsck_strict_arg.buf);
In any case, I tend to agree with you that it is overkill to add a
helper function for just to add a single element to the argument
list.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] .clang-format: introduce the use of clang-format
Jeff King wrote:
> On Wed, Jan 21, 2015 at 12:01:27PM -0500, Ramkumar Ramachandra wrote:
>> +BreakBeforeBraces: Linux
>> [...]
>> +BreakBeforeBraces: Stroustrup
Oh, oops.
> - It really wants to break function declarations that go over the
>column limit, even though we often do not do so. I think we're pretty
>inconsistent here, and I'd be fine going either way with it.
>
> - It really wanted to left-align some of my asterisks, like:
>
> struct foo_list {
>...
> } * foo, **foo_tail;
>
>The odd thing is that it gets the second one right, but not the first
>one (which should be "*foo" with no space). Setting:
>
> DerivePointerAlignment: false
> PointerAlignment: Right
>
>cleared it up, but I'm curious why the auto-deriver didn't work.
Sounds like a bug.
> - It really doesn't like list-alignment, like:
>
> #define FOO1
> #define LONGER 2
>
>and would prefer only a single space between "FOO" and "1". I think
>I'm OK with that, but we have a lot of aligned bits in the existing
>code.
>
> - It really wants to put function __attribute__ macros on the same line
>as the function. We often have it on a line above (especially it can
>be so long). I couldn't find a way to specify this.
You have to compromise a bit if you want to use an auto-formatting
tool, without losing your head patching every little detail :)
> - I had a long ternary operator broken across three lines, like:
>
> foo = bar ?
>some_long_thing(...) :
>some_other_long_thing(...);
>
>It put it all on one long line, which was much less readable. I set
>BreakBeforeTernaryOperators to "true", but it did nothing. I set it
>to "false", and then it broke. Which seems like a bug. It also
>insisted on indenting it like:
>
> foo = bar ?
>some_long_thing(...) :
>some_other_long_thing(...);
>
> which I found less readable.
To be honest, the LLVM community doesn't fix bugs just because they
can be fixed: it's quite heavily driven by commercial interest. And I
really don't find long ternary operators in a modern C++ codebase.
> I'm slightly dubious that
> any automated formatter can ever be _perfect_ (sometimes
> human-subjective readability trumps a hard-and-fast rule), but this
> seems like it might have some promise.
It works almost perfectly for the LLVM umbrella of projects. When they
want to change a coding convention (like leading Uppercase for
variable names), they write a clang-tidy thing to do it automatically.
> So overall I think it has some promise, but I do not think it is quite
> flexible enough yet for us to use day-to-day.
The big negative is that it will probably never be. I'll try to look
at the larger issues later this week, if you can compromise on the
fine details that are probably too hard to fix.
> 2. It can operate on patches (and generates patches for you to apply!
> You could add a git-add--interactive mode to selectively take its
> suggestions).
There's a git-clang-format in the $CLANG_ROOT/tools/clang-format/. I do:
$ g cf @~
... with the appropriate aliases.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-add--interactive: print message if there are no untracked files
Junio C Hamano writes:
>> sub add_untracked_cmd {
>> -my @add = list_and_choose({ PROMPT => 'Add untracked' },
>> - list_untracked());
>> -if (@add) {
>> -system(qw(git update-index --add --), @add);
>> -say_n_paths('added', @add);
>> +if (system(qw(git ls-files --others --exclude-standard --))) {
>
> But this ls-files invocation that knows too much about how
> list_untracked() computes things does not.
>
> Why not
> ...
> or something instead?
Actually, is there any case where list_and_choose() should give a
prompt to choose from zero candidates?
In other words, I am wondering if this affects other callers of
list_and_choose in any negative way.
git-add--interactive.perl | 4
1 file changed, 4 insertions(+)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 94b988c..46ed9a7 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -519,6 +519,10 @@ sub error_msg {
sub list_and_choose {
my ($opts, @stuff) = @_;
my (@chosen, @return);
+
+ if (!@stuff) {
+ return @return;
+ }
my $i;
my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i18n: bundle: mark git-bundle usage for translation
Thanks, but please hold this off until the 2.3 final is tagged. It is way too late for anything that is not a regression fix at this point in this cycle, and changes that affect i18n are the last thing we want to take late in the cycle because l10n people need time to update the translations. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-add--interactive: print message if there are no untracked files
Alexander Kuleshov writes:
> If user selects 'add untracked' and there are no untracked files,
> "Add untracked>>" opens. But it does not make sense in this case,
> because there are no untracked files. So let's print message and
> exit from "add untracked" mode.
That reasoning makes perfect sense.
> Signed-off-by: Alexander Kuleshov
> ---
> git-add--interactive.perl | 14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 94b988c..1a6dcf3 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -724,11 +724,15 @@ sub revert_cmd {
> }
>
> sub add_untracked_cmd {
> - my @add = list_and_choose({ PROMPT => 'Add untracked' },
> - list_untracked());
> - if (@add) {
> - system(qw(git update-index --add --), @add);
> - say_n_paths('added', @add);
> + if (system(qw(git ls-files --others --exclude-standard --))) {
But this ls-files invocation that knows too much about how
list_untracked() computes things does not.
Why not
my @add = list_untracked();
if (@add) {
@add = list_and_choose({...}, @add);
}
if (!@add) {
Nothing to do;
} else {
Run update-index
}
or something instead?
> + my @add = list_and_choose({ PROMPT => 'Add untracked' },
> + list_untracked());
> + if (@add) {
> + system(qw(git update-index --add --), @add);
> + say_n_paths('added', @add);
> + }
> + } else {
> + print "No untracked files.\n";
> }
> print "\n";
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] .clang-format: introduce the use of clang-format
On Wed, Jan 21, 2015 at 12:01:27PM -0500, Ramkumar Ramachandra wrote:
> Instead of manually eyeballing style in reviews, just ask all
> contributors to run their patches through [git-]clang-format.
Thanks for mentioning this; I hadn't seen the tool before.
I didn't see it mentioned here, but for those who are also new to the
tool, it has modes both for checking the content itself as well as diffs
(so you are not stuck wading through its reformats of code you didn't
touch).
> +BreakBeforeBraces: Linux
> [...]
> +BreakBeforeBraces: Stroustrup
These seem conflicting. It looks like you added "Stroustrup" to keep the
brace on the line with the "struct" keyword. But this does the wrong
thing for "cuddled else"s like:
if (...) {
...
} else {
...
}
I don't think clang-format has a mode that expresses our style.
I ran some of my recent patches through clang-format-diff, and it
generated quite a bit of output. Here are a few notes on what I saw.
Feel free to ignore. They are not your problem, but others evaluating
the tool might find it useful (and a few of them might suggest some
settings for .clang-format).
- It really wants to break function declarations that go over the
column limit, even though we often do not do so. I think we're pretty
inconsistent here, and I'd be fine going either way with it.
- It really wanted to left-align some of my asterisks, like:
struct foo_list {
...
} * foo, **foo_tail;
The odd thing is that it gets the second one right, but not the first
one (which should be "*foo" with no space). Setting:
DerivePointerAlignment: false
PointerAlignment: Right
cleared it up, but I'm curious why the auto-deriver didn't work.
- It really doesn't like list-alignment, like:
#define FOO1
#define LONGER 2
and would prefer only a single space between "FOO" and "1". I think
I'm OK with that, but we have a lot of aligned bits in the existing
code.
- It really wants to put function __attribute__ macros on the same line
as the function. We often have it on a line above (especially it can
be so long). I couldn't find a way to specify this.
- I had a long ternary operator broken across three lines, like:
foo = bar ?
some_long_thing(...) :
some_other_long_thing(...);
It put it all on one long line, which was much less readable. I set
BreakBeforeTernaryOperators to "true", but it did nothing. I set it
to "false", and then it broke. Which seems like a bug. It also
insisted on indenting it like:
foo = bar ?
some_long_thing(...) :
some_other_long_thing(...);
which I found less readable.
So overall I think it has some promise, but I do not think it is quite
flexible enough yet for us to use day-to-day. I'm slightly dubious that
any automated formatter can ever be _perfect_ (sometimes
human-subjective readability trumps a hard-and-fast rule), but this
seems like it might have some promise. And over other indenters I have
seen:
1. It's built on clang, so we know the parsing is solid.
2. It can operate on patches (and generates patches for you to apply!
You could add a git-add--interactive mode to selectively take its
suggestions).
Again, thanks for sharing.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git index containing tree extension for unknown path
Christian Halstrick writes: > Is it allowed that the git index contains a tree extension mentioning > patch 'x/y/z' while the only entry in the index is a '.gitattributes' > files in the root? Depends on the definition of "mention", but it is not unexpected that you see "x", "y", and "z" in the cache-tree extension as invalidated nodes after you do something like this: rm -fr test && git init test && cd test mkdir -p x/y/z && >x/y/z/1 && git add x && git write-tree && # cache-tree is fully valid mv x/y/z x/y/a && git add x # cache-tree invalidated "z", if appears, should still know that "y" is its parent and "y", if appears, should still know that "x" is its parent. All of the three should say they have been invalidated by showing a negative entry-count and show the "correct" subtree count that appear in the extension (i.e. if "z" is there as an invalidated leaf, it should say "-1 0" to indicate an invalidated entry by a negative entry count, with zero subtrees, and "y" would show "-1 1" to indicate an invalidated entry with one subtree, namely "z", etc.). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: .gitattributes on branch behaves unexpected. Should it be more stateless?
Please, no top posting. It breaks the discussion flow. From: "Max W" Hi philip, thanks for your reply. You don't say which parts you believe should be identical, nor why. I expected my representation to be identical, nevertheless what path I have taken to come there. e.g. git clone -b git clone; git checkout should result in a binary-indentical representation of the files tracked by git. But they don't. Don't forget that some of the EOL setting are also in the git config file(s) [global, system, local] so there may be differences from them. Use 'git cat-file' to get the plumbing level view of the cannonical object content. I am prtesuming that the ONLY differences you are seeing are end of line conversion changes for checkout. Is that correct. Are there other whitespace changes happening? Is that on the way into the repo (git add). There are some white space settings available. The other aspect maybe language coding settings (utf8, iso, cp-..., etc). Or is it even more than that? (Your first email suggested it was just line endings) The discussion may be better helped on the googlegroups git-users forum git-us...@googlegroups.com if this is more of a 'getting started with Git' problem. Why did I expect this? Good question. Feels more intuitive for me. Like "don't worry what you have done before. When your HEAD is on a certain commit, git will make sure all your files in your filesystem are they way you (and the other committers) want them to be". [..] Does that help? Yes, it helped me to distinguish better between data and representation. New understanding: .gitattributes determines how to represent data. As .gitattributes is under git control there can be 2 versions of .gitattributes in 2 branches. So I can tell git - on branch foo with gitattributes * eol=LF show me all files with LF - on branch bar with gitattributes * eol=CRLF show me all files with CRLF But this doesn't work. The representation of the files is determined/depending on how I cloned or fetched the repo. A "git checkout bar" does not change the representation. Does this help to show where my concerns / my issue is? Best Regards, Max 2015-01-17 14:16 GMT+01:00 Philip Oakley : Hi, I am asking myself if git and .gitattributes should be more stateless? i.e. whatever you have done before is irrelevant, when you reach status XYZ with your git repo, it is EXACTLY and BINARY the same all the time and everywhere. It took some time for me to figure out, that depending on HOW you clone, the resulting local repo may differ. I did not expect this. I assumed that when I clone, it is a clone (meaning: 100% identical). And that the things I have done in my local repo before, don't have any relevance at all. You don't say which parts you believe should be identical, nor why. Internally Git can represent its object store in many ways based on some objects being 'loose' and some objects being 'packed'. However both styles of representation are of the same base objects and their contents. Then we have external OS representation, in particular the end of line representations between the three main OS types Win/Mac/'nix. Git gives _you_ the ability the use any of these representations for the same base objects. Thus the object file with text "Hello World/EOL/Goodbye World" will have three different binary representations once you export them to the selected file system type (according to you .gitattributes settings). If you always select LF endings for text files (both on the way in and on the way out of the repo), then you will get identical files on the different clones. Git has many settings for personalisation. Does that help? ** How to reproduce ** 1) create a repo, add a file with LF ending, add a .gitattributes telling git to do a CRLF conversion 2) clone the repo 3) on brach development, change .gitattributes to LF 4) clone again 5) clone again, directly onto the branch development (git clone -b) ** Expected result, (I) ** clone 2) and original repo 1) are bytewise identical ** Actual result (I) ** clone 2) and original repo 1) differ, 1) has LF, 2) has CRLF as I have been warned before, I am (more or less) fine/OK with this ** Expected result, (II) ** - clone without -b (4) and clone with -b (5) are bytewise identical - I would have expected, that whatever I do, as soon as I have a clone and I am on branch "development", my file should be LF - I would have expected, that HOW you clone is irrelevant ** Actual result (II) ** without -b (4) I have a CRLF file on my disk. with -b (5) I have a LF file on my disk. The clones are not bytewise indentical. It appears as if the .gitattributes in branch development does not have any reliable effect. A potential solution might be be that - checkout - commit (a modified .gitattribues) - do change the files in the local repo. As of now my understanding is that this is not how .gitattributes (or .gitign
[PATCH v3 19/19] fsck: support ignoring objects in `git fsck` via fsck.skiplist
Identical to support in `git receive-pack for the config option
`receive.fsck.skiplist`, we now support ignoring given objects in
`git fsck` altogether.
This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).
Signed-off-by: Johannes Schindelin
---
Documentation/config.txt | 7 +++
builtin/fsck.c | 10 ++
2 files changed, 17 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 636adff..644411a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1223,6 +1223,13 @@ that setting `fsck.ignore = missing-email` will hide
that issue.
This feature is intended to support working with legacy repositories
which cannot be repaired without disruptive changes.
+fsck.skipList::
+ The path to a sorted list of object names (i.e. one SHA-1 per
+ line) that are known to be broken in a non-fatal way and should
+ be ignored. This feature is useful when an established project
+ should be accepted despite early commits containing errors that
+ can be safely ignored such as invalid committer email addresses.
+
gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'. This defaults
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 7ae4715..760b4bd 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -49,6 +49,16 @@ static int show_dangling = 1;
static int fsck_config(const char *var, const char *value, void *cb)
{
+ if (strcmp(var, "receive.fsck.skiplist") == 0) {
+ const char *path = is_absolute_path(value) ?
+ value : git_path("%s", value);
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "skiplist=%s", path);
+ fsck_set_severity(&fsck_obj_options, sb.buf);
+ strbuf_release(&sb);
+ return 0;
+ }
+
if (skip_prefix(var, "fsck.", &var)) {
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s=%s", var, value);
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 16/19] fsck: Support demoting errors to warnings
We already have support in `git receive-pack` to deal with some legacy
repositories which have non-fatal issues.
Let's make `git fsck` itself useful with such repositories, too, by
allowing users to ignore known issues, or at least demote those issues
to mere warnings.
Example: `git -c fsck.ignore=missing-email fsck` would hide problems with
missing emails in author, committer and tagger lines.
In the same spirit that `git receive-pack`'s usage of the fsck machinery
differs from `git fsck`'s – some of the non-fatal warnings in `git fsck`
are fatal with `git receive-pack` when receive.fsckObjects = true, for
example – we strictly separate the fsck.* from the receive.fsck.*
settings.
Signed-off-by: Johannes Schindelin
---
Documentation/config.txt | 15 +++
builtin/fsck.c | 15 +++
t/t1450-fsck.sh | 11 +++
3 files changed, 41 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index cc4cd91..115811c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1208,6 +1208,21 @@ filter..smudge::
object to a worktree file upon checkout. See
linkgit:gitattributes[5] for details.
+fsck.error::
+fsck.warn::
+fsck.ignore::
+ The `fsck.error`, `fsck.warn` and `fsck.ignore` settings specify
+ comma-separated lists of fsck message IDs which should trigger
+ fsck to error out, to print the message and continue, or to ignore
+ said messages, respectively.
++
+For convenience, fsck prefixes the error/warning with the name of the option,
+e.g. "missing-email: invalid author/committer line - missing email" means
+that setting `fsck.ignore = missing-email` will hide that issue.
++
+This feature is intended to support working with legacy repositories
+which cannot be repaired without disruptive changes.
+
gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'. This defaults
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 99d4538..6f5e671 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -46,6 +46,19 @@ static int show_dangling = 1;
#define DIRENT_SORT_HINT(de) ((de)->d_ino)
#endif
+static int fsck_config(const char *var, const char *value, void *cb)
+{
+ if (skip_prefix(var, "fsck.", &var)) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%s=%s", var, value);
+ fsck_set_severity(&fsck_obj_options, sb.buf);
+ strbuf_release(&sb);
+ return 0;
+ }
+
+ return git_default_config(var, value, cb);
+}
+
static void objreport(struct object *obj, const char *severity,
const char *err)
{
@@ -638,6 +651,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
include_reflogs = 0;
}
+ git_config(fsck_config, NULL);
+
fsck_head_link();
fsck_object_dir(get_object_directory());
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index ea0f216..a79ff9f 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -287,6 +287,17 @@ test_expect_success 'rev-list --verify-objects with bad
sha1' '
grep -q "error: sha1 mismatch 63ff"
out
'
+test_expect_success 'force fsck to ignore double author' '
+ git cat-file commit HEAD >basis &&
+ sed "s/^author .*/&,&/" multiple-authors &&
+ new=$(git hash-object -t commit -w --stdin http://vger.kernel.org/majordomo-info.html
[PATCH v3 17/19] fsck: Introduce `git fsck --quick`
This option avoids unpacking each and all objects, and just verifies the
connectivity. In particular with large repositories, this speeds up the
operation, at the expense of missing corrupt blobs and ignoring
unreachable objects, if any.
Signed-off-by: Johannes Schindelin
---
Documentation/git-fsck.txt | 7 ++-
builtin/fsck.c | 7 ++-
t/t1450-fsck.sh| 22 ++
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 25c431d..b98fb43 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
-[--[no-]full] [--strict] [--verbose] [--lost-found]
+[--[no-]full] [--quick] [--strict] [--verbose] [--lost-found]
[--[no-]dangling] [--[no-]progress] [*]
DESCRIPTION
@@ -60,6 +60,11 @@ index file, all SHA-1 references in `refs` namespace, and
all reflogs
object pools. This is now default; you can turn it off
with --no-full.
+--quick::
+ Check only the connectivity of tags, commits and tree objects. By
+ avoiding to unpack blobs, this speeds up the operation, at the
+ expense of missing corrupt objects.
+
--strict::
Enable more strict checking, namely to catch a file mode
recorded with g+w bit set, which was created by older
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6f5e671..7ae4715 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -23,6 +23,7 @@ static int show_tags;
static int show_unreachable;
static int include_reflogs = 1;
static int check_full = 1;
+static int quick;
static int check_strict;
static int keep_cache_objects;
static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
@@ -184,6 +185,8 @@ static void check_reachable_object(struct object *obj)
if (!(obj->flags & HAS_OBJ)) {
if (has_sha1_pack(obj->sha1))
return; /* it is in pack - forget about it */
+ if (quick && has_sha1_file(obj->sha1))
+ return;
printf("missing %s %s\n", typename(obj->type),
sha1_to_hex(obj->sha1));
errors_found |= ERROR_REACHABLE;
return;
@@ -618,6 +621,7 @@ static struct option fsck_opts[] = {
OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head
nodes")),
OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes
(default)")),
OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate
objects")),
+ OPT_BOOL(0, "quick", &quick, N_("check only connectivity")),
OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")),
OPT_BOOL(0, "lost-found", &write_lost_and_found,
N_("write dangling objects in
.git/lost-found")),
@@ -654,7 +658,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
git_config(fsck_config, NULL);
fsck_head_link();
- fsck_object_dir(get_object_directory());
+ if (!quick)
+ fsck_object_dir(get_object_directory());
prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt->next) {
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index a79ff9f..1c624a3 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -431,4 +431,26 @@ test_expect_success 'fsck notices ref pointing to missing
tag' '
test_must_fail git -C missing fsck
'
+test_expect_success 'fsck --quick' '
+ rm -rf quick &&
+ git init quick &&
+ (
+ cd quick &&
+ touch empty &&
+ git add empty &&
+ test_commit empty &&
+ empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
+ rm -f $empty &&
+ echo invalid >$empty &&
+ test_must_fail git fsck --strict &&
+ git fsck --strict --quick &&
+ tree=$(git rev-parse HEAD:) &&
+ suffix=${tree#??} &&
+ tree=.git/objects/${tree%$suffix}/$suffix &&
+ rm -f $tree &&
+ echo invalid >$tree &&
+ test_must_fail git fsck --strict --quick
+ )
+'
+
test_done
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 18/19] fsck: git receive-pack: support excluding objects from fsck'ing
The optional new config option `receive.fsck.skiplist` specifies the path
to a file listing the names, i.e. SHA-1s, one per line, of objects that
are to be ignored by `git receive-pack` when `receive.fsckObjects = true`.
This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).
The intended use case is for server administrators to inspect objects
that are reported by `git push` as being too problematic to enter the
repository, and to add the objects' SHA-1 to a (preferably sorted) file
when the objects are legitimate, i.e. when it is determined that those
problematic objects should be allowed to enter the server.
Signed-off-by: Johannes Schindelin
---
Documentation/config.txt| 7 ++
builtin/receive-pack.c | 9 +++
fsck.c | 53 +
fsck.h | 1 +
t/t5504-fetch-receive-strict.sh | 12 ++
5 files changed, 82 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 115811c..636adff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2173,6 +2173,13 @@ which would not pass pushing when `receive.fsckObjects =
true`, allowing
the host to accept repositories with certain known issues but still catch
other issues.
+receive.fsck.skipList::
+ The path to a sorted list of object names (i.e. one SHA-1 per
+ line) that are known to be broken in a non-fatal way and should
+ be ignored. This feature is useful when an established project
+ should be accepted despite early commits containing errors that
+ can be safely ignored such as invalid committer email addresses.
+
receive.unpackLimit::
If the number of objects received in a push is below this
limit then the objects will be unpacked into loose object
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 18d5012..8e6d1a1 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -116,6 +116,15 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (strcmp(var, "receive.fsck.skiplist") == 0) {
+ const char *path = is_absolute_path(value) ?
+ value : git_path("%s", value);
+ if (fsck_severity.len)
+ strbuf_addch(&fsck_severity, ',');
+ strbuf_addf(&fsck_severity, "skiplist=%s", path);
+ return 0;
+ }
+
if (skip_prefix(var, "receive.fsck.", &var)) {
strbuf_addf(&fsck_severity, "%s%s=%s",
fsck_severity.len ? "," : "", var, value);
diff --git a/fsck.c b/fsck.c
index 1334941..15cb8bd 100644
--- a/fsck.c
+++ b/fsck.c
@@ -8,6 +8,7 @@
#include "fsck.h"
#include "refs.h"
#include "utf8.h"
+#include "sha1-array.h"
#define FSCK_FATAL -1
#define FSCK_INFO -2
@@ -117,6 +118,43 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
return severity;
}
+static void init_skiplist(struct fsck_options *options, const char *path)
+{
+ static struct sha1_array skiplist = SHA1_ARRAY_INIT;
+ int sorted, fd;
+ char buffer[41];
+ unsigned char sha1[20];
+
+ if (options->skiplist)
+ sorted = options->skiplist->sorted;
+ else {
+ sorted = 1;
+ options->skiplist = &skiplist;
+ }
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ die("Could not open skip list: %s", path);
+ for (;;) {
+ int result = read_in_full(fd, buffer, sizeof(buffer));
+ if (result < 0)
+ die_errno("Could not read '%s'", path);
+ if (!result)
+ break;
+ if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
+ die("Invalid SHA-1: %s", buffer);
+ sha1_array_append(&skiplist, sha1);
+ if (sorted && skiplist.nr > 1 &&
+ hashcmp(skiplist.sha1[skiplist.nr - 2],
+ sha1) > 0)
+ sorted = 0;
+ }
+ close(fd);
+
+ if (sorted)
+ skiplist.sorted = 1;
+}
+
static inline int substrcmp(const char *string, int len, const char *match)
{
int match_len = strlen(match);
@@ -156,6 +194,17 @@ void fsck_set_severity(struct fsck_options *options, const
char *mode)
severity = FSCK_WARN;
else if (!substrcmp(mode, equal, "ignore"))
severity = FSCK_IGNORE;
+ else if (!substrcmp(mode, equal, "skiplist")) {
+ char *path = xstrndup(mode + equal + 1,
+ len - equal - 1);
+
[PATCH v3 15/19] fsck: Document the new receive.fsck.* options.
Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 28 1 file changed, 28 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index ae6791d..cc4cd91 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2130,6 +2130,34 @@ receive.fsckObjects:: Defaults to false. If not set, the value of `transfer.fsckObjects` is used instead. +receive.fsck.error:: +receive.fsck.warn:: +receive.fsck.ignore:: + When `receive.fsckObjects` is set to true, errors can be switched + to warnings and vice versa by configuring the `receive.fsck.*` + settings. These settings contain comma-separated lists of fsck + message IDs. For convenience, fsck prefixes the error/warning with + the message ID, e.g. "missing-email: invalid + author/committer line - missing email" means that setting + `receive.fsck.ignore = missing-email` will hide that issue. ++ +-- +error;; + a comma-separated list of fsck message IDs that should be + trigger fsck to error out. +warn;; + a comma-separated list of fsck message IDs that should be + displayed, but fsck should continue to error out. +ignore;; + a comma-separated list of fsck message IDs that should be + ignored completely. +-- ++ +This feature is intended to support working with legacy repositories +which would not pass pushing when `receive.fsckObjects = true`, allowing +the host to accept repositories with certain known issues but still catch +other issues. + receive.unpackLimit:: If the number of objects received in a push is below this limit then the objects will be unpacked into loose object -- 2.2.0.33.gc18b867 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 14/19] fsck: Allow upgrading fsck warnings to errors
The 'invalid tag name' and 'missing tagger entry' warnings can now be
upgraded to errors by specifying `invalid-tag-name` and
`missing-tagger-entry` to the receive.fsck.error config setting.
Incidentally, the missing tagger warning is now really shown as a warning
(as opposed to being reported with the "error:" prefix, as it used to be
the case before this commit).
Signed-off-by: Johannes Schindelin
---
fsck.c| 24 +---
t/t5302-pack-index.sh | 2 +-
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/fsck.c b/fsck.c
index 028a7ca..1334941 100644
--- a/fsck.c
+++ b/fsck.c
@@ -10,6 +10,7 @@
#include "utf8.h"
#define FSCK_FATAL -1
+#define FSCK_INFO -2
#define FOREACH_MSG_ID(FUNC) \
/* fatal errors */ \
@@ -55,10 +56,11 @@
FUNC(HAS_DOT, WARN) \
FUNC(HAS_DOTDOT, WARN) \
FUNC(HAS_DOTGIT, WARN) \
- FUNC(INVALID_TAG_NAME, WARN) \
- FUNC(MISSING_TAGGER_ENTRY, WARN) \
FUNC(NULL_SHA1, WARN) \
- FUNC(ZERO_PADDED_FILEMODE, WARN)
+ FUNC(ZERO_PADDED_FILEMODE, WARN) \
+ /* infos (reported as warnings, but ignored by default) */ \
+ FUNC(INVALID_TAG_NAME, INFO) \
+ FUNC(MISSING_TAGGER_ENTRY, INFO)
#define MSG_ID(id, severity) FSCK_MSG_##id,
enum fsck_msg_id {
@@ -200,6 +202,8 @@ static int report(struct fsck_options *options, struct
object *object,
if (msg_severity == FSCK_FATAL)
msg_severity = FSCK_ERROR;
+ else if (msg_severity == FSCK_INFO)
+ msg_severity = FSCK_WARN;
append_msg_id(&sb, msg_id_info[id].id_string);
@@ -658,15 +662,21 @@ static int fsck_tag_buffer(struct tag *tag, const char
*data,
goto done;
}
strbuf_addf(&sb, "refs/tags/%.*s", (int)(eol - buffer), buffer);
- if (check_refname_format(sb.buf, 0))
- report(options, &tag->object, FSCK_MSG_INVALID_TAG_NAME,
+ if (check_refname_format(sb.buf, 0)) {
+ ret = report(options, &tag->object, FSCK_MSG_INVALID_TAG_NAME,
"invalid 'tag' name: %.*s",
(int)(eol - buffer), buffer);
+ if (ret)
+ goto done;
+ }
buffer = eol + 1;
- if (!skip_prefix(buffer, "tagger ", &buffer))
+ if (!skip_prefix(buffer, "tagger ", &buffer)) {
/* early tags do not contain 'tagger' lines; warn only */
- report(options, &tag->object, FSCK_MSG_MISSING_TAGGER_ENTRY,
"invalid format - expected 'tagger' line");
+ ret = report(options, &tag->object,
FSCK_MSG_MISSING_TAGGER_ENTRY, "invalid format - expected 'tagger' line");
+ if (ret)
+ goto done;
+ }
else
ret = fsck_ident(&buffer, &tag->object, options);
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 61bc8da..3dc5ec4 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -259,7 +259,7 @@ EOF
thirtyeight=${tag#??} &&
rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight &&
git index-pack --strict tag-test-${pack1}.pack 2>err &&
-grep "^error:.* expected .tagger. line" err
+grep "^warning:.* expected .tagger. line" err
'
test_done
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 12/19] fsck: Disallow demoting grave fsck errors to warnings
Some kinds of errors are intrinsically unrecoverable (e.g. errors while
uncompressing objects). It does not make sense to allow demoting them to
mere warnings.
Signed-off-by: Johannes Schindelin
---
fsck.c | 13 +++--
t/t5504-fetch-receive-strict.sh | 9 +
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index 4adf9ce..27a381b 100644
--- a/fsck.c
+++ b/fsck.c
@@ -9,7 +9,12 @@
#include "refs.h"
#include "utf8.h"
+#define FSCK_FATAL -1
+
#define FOREACH_MSG_ID(FUNC) \
+ /* fatal errors */ \
+ FUNC(NUL_IN_HEADER, FATAL) \
+ FUNC(UNTERMINATED_HEADER, FATAL) \
/* errors */ \
FUNC(BAD_DATE, ERROR) \
FUNC(BAD_EMAIL, ERROR) \
@@ -40,10 +45,8 @@
FUNC(MISSING_TYPE_ENTRY, ERROR) \
FUNC(MULTIPLE_AUTHORS, ERROR) \
FUNC(NOT_SORTED, ERROR) \
- FUNC(NUL_IN_HEADER, ERROR) \
FUNC(TAG_OBJECT_NOT_TAG, ERROR) \
FUNC(UNKNOWN_TYPE, ERROR) \
- FUNC(UNTERMINATED_HEADER, ERROR) \
FUNC(ZERO_PADDED_DATE, ERROR) \
/* warnings */ \
FUNC(BAD_FILEMODE, WARN) \
@@ -157,6 +160,9 @@ void fsck_set_severity(struct fsck_options *options, const
char *mode)
}
msg_id = parse_msg_id(mode, len);
+ if (severity != FSCK_ERROR &&
+ msg_id_info[msg_id].severity == FSCK_FATAL)
+ die("Cannot demote %.*s", len, mode);
options->msg_severity[msg_id] = severity;
mode += len;
}
@@ -187,6 +193,9 @@ static int report(struct fsck_options *options, struct
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_severity = fsck_msg_severity(id, options), result;
+ if (msg_severity == FSCK_FATAL)
+ msg_severity = FSCK_ERROR;
+
append_msg_id(&sb, msg_id_info[id].id_string);
va_start(ap, fmt);
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 40c7557..75d718f 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -135,4 +135,13 @@ test_expect_success 'push with receive.fsck.warn =
missing-email' '
grep "missing-email" act
'
+test_expect_success 'receive.fsck.warn = unterminated-header triggers error' '
+ rm -rf dst &&
+ git init dst &&
+ git --git-dir=dst/.git config receive.fsckobjects true &&
+ git --git-dir=dst/.git config receive.fsck.warn unterminated-header &&
+ test_must_fail git push --porcelain dst HEAD >act 2>&1 &&
+ grep "Cannot demote unterminated-header" act
+'
+
test_done
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 13/19] fsck: Optionally ignore specific fsck issues completely
An fsck issue in a legacy repository might be so common that one would
like not to bother the user with mentioning it at all. With this change,
that is possible by setting the respective error to "ignore".
This change "abuses" the warn=missing-email test to verify that "ignore"
is also accepted and works correctly. And while at it, it makes sure
that multiple options work, too (they are passed to unpack-objects or
index-pack as a comma-separated list via the --strict=... command-line
option).
Signed-off-by: Johannes Schindelin
---
fsck.c | 5 +
fsck.h | 1 +
t/t5504-fetch-receive-strict.sh | 7 ++-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 27a381b..028a7ca 100644
--- a/fsck.c
+++ b/fsck.c
@@ -152,6 +152,8 @@ void fsck_set_severity(struct fsck_options *options, const
char *mode)
severity = FSCK_ERROR;
else if (!substrcmp(mode, equal, "warn"))
severity = FSCK_WARN;
+ else if (!substrcmp(mode, equal, "ignore"))
+ severity = FSCK_IGNORE;
else
die("Unknown fsck message severity: '%.*s'",
equal, mode);
@@ -193,6 +195,9 @@ static int report(struct fsck_options *options, struct
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_severity = fsck_msg_severity(id, options), result;
+ if (msg_severity == FSCK_IGNORE)
+ return 0;
+
if (msg_severity == FSCK_FATAL)
msg_severity = FSCK_ERROR;
diff --git a/fsck.h b/fsck.h
index 4349860..7be6c50 100644
--- a/fsck.h
+++ b/fsck.h
@@ -3,6 +3,7 @@
#define FSCK_ERROR 1
#define FSCK_WARN 2
+#define FSCK_IGNORE 3
struct fsck_options;
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 75d718f..5e54a13 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -132,7 +132,12 @@ test_expect_success 'push with receive.fsck.warn =
missing-email' '
test_must_fail git push --porcelain dst bogus &&
git --git-dir=dst/.git config receive.fsck.warn missing-email &&
git push --porcelain dst bogus >act 2>&1 &&
- grep "missing-email" act
+ grep "missing-email" act &&
+ git --git-dir=dst/.git branch -D bogus &&
+ git --git-dir=dst/.git config receive.fsck.ignore missing-email &&
+ git --git-dir=dst/.git config receive.fsck.warn bad-date &&
+ git push --porcelain dst bogus >act 2>&1 &&
+ test_must_fail grep "missing-email" act
'
test_expect_success 'receive.fsck.warn = unterminated-header triggers error' '
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/19] fsck: Add a simple test for receive.fsck.*
Signed-off-by: Johannes Schindelin --- t/t5504-fetch-receive-strict.sh | 20 1 file changed, 20 insertions(+) diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh index 69ee13c..40c7557 100755 --- a/t/t5504-fetch-receive-strict.sh +++ b/t/t5504-fetch-receive-strict.sh @@ -115,4 +115,24 @@ test_expect_success 'push with transfer.fsckobjects' ' test_cmp exp act ' +cat >bogus-commit <<\EOF +tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 +author Bugs Bunny 1234567890 + +committer Bugs Bunny 1234567890 + + +This commit object intentionally broken +EOF + +test_expect_success 'push with receive.fsck.warn = missing-email' ' + commit="$(git hash-object -t commit -w --stdin < bogus-commit)" && + git push . $commit:refs/heads/bogus && + rm -rf dst && + git init dst && + git --git-dir=dst/.git config receive.fsckobjects true && + test_must_fail git push --porcelain dst bogus && + git --git-dir=dst/.git config receive.fsck.warn missing-email && + git push --porcelain dst bogus >act 2>&1 && + grep "missing-email" act +' + test_done -- 2.2.0.33.gc18b867 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 10/19] fsck: Make fsck_tag() warn-friendly
When fsck_tag() identifies a problem with the commit, it should try
to make it possible to continue checking the commit object, in case the
user wants to demote the detected errors to mere warnings.
Just like fsck_commit(), there are certain problems that could hide other
issues with the same tag object. For example, if the 'type' line is not
encountered in the correct position, the 'tag' line – if there is any –
would not be handled at all.
Signed-off-by: Johannes Schindelin
---
fsck.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 3118db1..4adf9ce 100644
--- a/fsck.c
+++ b/fsck.c
@@ -614,7 +614,8 @@ static int fsck_tag_buffer(struct tag *tag, const char
*data,
}
if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') {
ret = report(options, &tag->object,
FSCK_MSG_INVALID_OBJECT_SHA1, "invalid 'object' line format - bad sha1");
- goto done;
+ if (ret)
+ goto done;
}
buffer += 41;
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 09/19] fsck: Handle multiple authors in commits specially
This problem has been detected in the wild, and is the primary reason
to introduce an option to demote certain fsck errors to warnings. Let's
offer to ignore this particular problem specifically.
Technically, we could handle such repositories by setting
receive.fsck.warn = missing-committer, but that could hide missing tree
objects in the same commit because we cannot continue verifying any
commit object after encountering a missing committer line, while we can
continue in the case of multiple author lines.
Signed-off-by: Johannes Schindelin
---
fsck.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/fsck.c b/fsck.c
index 8979357..3118db1 100644
--- a/fsck.c
+++ b/fsck.c
@@ -38,6 +38,7 @@
FUNC(MISSING_TREE, ERROR) \
FUNC(MISSING_TYPE, ERROR) \
FUNC(MISSING_TYPE_ENTRY, ERROR) \
+ FUNC(MULTIPLE_AUTHORS, ERROR) \
FUNC(NOT_SORTED, ERROR) \
FUNC(NUL_IN_HEADER, ERROR) \
FUNC(TAG_OBJECT_NOT_TAG, ERROR) \
@@ -545,6 +546,14 @@ static int fsck_commit_buffer(struct commit *commit, const
char *buffer,
err = fsck_ident(&buffer, &commit->object, options);
if (err)
return err;
+ while (skip_prefix(buffer, "author ", &buffer)) {
+ err = report(options, &commit->object,
FSCK_MSG_MULTIPLE_AUTHORS, "invalid format - multiple 'author' lines");
+ if (err)
+ return err;
+ err = fsck_ident(&buffer, &commit->object, options);
+ if (err)
+ return err;
+ }
if (!skip_prefix(buffer, "committer ", &buffer))
return report(options, &commit->object,
FSCK_MSG_MISSING_COMMITTER, "invalid format - expected 'committer' line");
err = fsck_ident(&buffer, &commit->object, options);
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/19] fsck: Allow demoting errors to warnings via receive.fsck.warn =
For example, missing emails in commit and tag objects can be demoted to
mere warnings with
git config receive.fsck.warn = missing-email
The value is actually a comma-separated list, and there is a
corresponding receive.fsck.error setting.
In case that the same key is listed in multiple receive.fsck.* lines in
the config, the latter configuration wins.
As git receive-pack does not actually perform the checks, it hands off
the setting to index-pack or unpack-objects in the form of an optional
argument to the --strict option.
Signed-off-by: Johannes Schindelin
---
builtin/index-pack.c | 4
builtin/receive-pack.c | 15 +--
builtin/unpack-objects.c | 5 +
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 925f7b5..b82b4dd 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1565,6 +1565,10 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
} else if (!strcmp(arg, "--strict")) {
strict = 1;
do_fsck_object = 1;
+ } else if (skip_prefix(arg, "--strict=", &arg)) {
+ strict = 1;
+ do_fsck_object = 1;
+ fsck_set_severity(&fsck_options, arg);
} else if (!strcmp(arg,
"--check-self-contained-and-connected")) {
strict = 1;
check_self_contained_and_connected = 1;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e0ce78e..18d5012 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -36,6 +36,7 @@ static enum deny_action deny_current_branch =
DENY_UNCONFIGURED;
static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
static int receive_fsck_objects = -1;
static int transfer_fsck_objects = -1;
+static struct strbuf fsck_severity = STRBUF_INIT;
static int receive_unpack_limit = -1;
static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
@@ -115,6 +116,12 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (skip_prefix(var, "receive.fsck.", &var)) {
+ strbuf_addf(&fsck_severity, "%s%s=%s",
+ fsck_severity.len ? "," : "", var, value);
+ return 0;
+ }
+
if (strcmp(var, "receive.fsckobjects") == 0) {
receive_fsck_objects = git_config_bool(var, value);
return 0;
@@ -1471,7 +1478,9 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
if (quiet)
argv_array_push(&child.args, "-q");
if (fsck_objects)
- argv_array_push(&child.args, "--strict");
+ argv_array_pushf(&child.args, "--strict%s%s",
+ fsck_severity.len ? "=" : "",
+ fsck_severity.buf);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -1489,7 +1498,9 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
argv_array_pushl(&child.args, "index-pack",
"--stdin", hdr_arg, keep_arg, NULL);
if (fsck_objects)
- argv_array_push(&child.args, "--strict");
+ argv_array_pushf(&child.args, "--strict%s%s",
+ fsck_severity.len ? "=" : "",
+ fsck_severity.buf);
if (fix_thin)
argv_array_push(&child.args, "--fix-thin");
child.out = -1;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 6d17040..fe9117c 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -530,6 +530,11 @@ int cmd_unpack_objects(int argc, const char **argv, const
char *prefix)
strict = 1;
continue;
}
+ if (skip_prefix(arg, "--strict=", &arg)) {
+ strict = 1;
+ fsck_set_severity(&fsck_options, arg);
+ continue;
+ }
if (starts_with(arg, "--pack_header=")) {
struct pack_header *hdr;
char *c;
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 08/19] fsck: Make fsck_commit() warn-friendly
When fsck_commit() identifies a problem with the commit, it should try
to make it possible to continue checking the commit object, in case the
user wants to demote the detected errors to mere warnings.
Note that some problems are too problematic to simply ignore. For
example, when the header lines are mixed up, we punt after encountering
an incorrect line. Therefore, demoting certain warnings to errors can
hide other problems. Example: demoting the missing-author error to
a warning would hide a problematic committer line.
Signed-off-by: Johannes Schindelin
---
fsck.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/fsck.c b/fsck.c
index 16500e3..8979357 100644
--- a/fsck.c
+++ b/fsck.c
@@ -508,12 +508,18 @@ static int fsck_commit_buffer(struct commit *commit,
const char *buffer,
if (!skip_prefix(buffer, "tree ", &buffer))
return report(options, &commit->object, FSCK_MSG_MISSING_TREE,
"invalid format - expected 'tree' line");
- if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')
- return report(options, &commit->object, FSCK_MSG_BAD_TREE_SHA1,
"invalid 'tree' line format - bad sha1");
+ if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n') {
+ err = report(options, &commit->object, FSCK_MSG_BAD_TREE_SHA1,
"invalid 'tree' line format - bad sha1");
+ if (err)
+ return err;
+ }
buffer += 41;
while (skip_prefix(buffer, "parent ", &buffer)) {
- if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
- return report(options, &commit->object,
FSCK_MSG_BAD_PARENT_SHA1, "invalid 'parent' line format - bad sha1");
+ if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') {
+ err = report(options, &commit->object,
FSCK_MSG_BAD_PARENT_SHA1, "invalid 'parent' line format - bad sha1");
+ if (err)
+ return err;
+ }
buffer += 41;
parent_line_count++;
}
@@ -522,11 +528,17 @@ static int fsck_commit_buffer(struct commit *commit,
const char *buffer,
if (graft) {
if (graft->nr_parent == -1 && !parent_count)
; /* shallow commit */
- else if (graft->nr_parent != parent_count)
- return report(options, &commit->object,
FSCK_MSG_MISSING_GRAFT, "graft objects missing");
+ else if (graft->nr_parent != parent_count) {
+ err = report(options, &commit->object,
FSCK_MSG_MISSING_GRAFT, "graft objects missing");
+ if (err)
+ return err;
+ }
} else {
- if (parent_count != parent_line_count)
- return report(options, &commit->object,
FSCK_MSG_MISSING_PARENT, "parent objects missing");
+ if (parent_count != parent_line_count) {
+ err = report(options, &commit->object,
FSCK_MSG_MISSING_PARENT, "parent objects missing");
+ if (err)
+ return err;
+ }
}
if (!skip_prefix(buffer, "author ", &buffer))
return report(options, &commit->object,
FSCK_MSG_MISSING_AUTHOR, "invalid format - expected 'author' line");
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/19] fsck: Make fsck_ident() warn-friendly
When fsck_ident() identifies a problem with the ident, it should still
advance the pointer to the next line so that fsck can continue in the
case of a mere warning.
Signed-off-by: Johannes Schindelin
---
fsck.c | 49 +++--
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/fsck.c b/fsck.c
index 09f69fe..16500e3 100644
--- a/fsck.c
+++ b/fsck.c
@@ -453,40 +453,45 @@ static int require_end_of_header(const void *data,
unsigned long size,
static int fsck_ident(const char **ident, struct object *obj, struct
fsck_options *options)
{
+ const char *p = *ident;
char *end;
- if (**ident == '<')
+ *ident = strchrnul(*ident, '\n');
+ if (**ident == '\n')
+ (*ident)++;
+
+ if (*p == '<')
return report(options, obj, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL,
"invalid author/committer line - missing space before email");
- *ident += strcspn(*ident, "<>\n");
- if (**ident == '>')
+ p += strcspn(p, "<>\n");
+ if (*p == '>')
return report(options, obj, FSCK_MSG_BAD_NAME, "invalid
author/committer line - bad name");
- if (**ident != '<')
+ if (*p != '<')
return report(options, obj, FSCK_MSG_MISSING_EMAIL, "invalid
author/committer line - missing email");
- if ((*ident)[-1] != ' ')
+ if (p[-1] != ' ')
return report(options, obj,
FSCK_MSG_MISSING_SPACE_BEFORE_EMAIL, "invalid author/committer line - missing
space before email");
- (*ident)++;
- *ident += strcspn(*ident, "<>\n");
- if (**ident != '>')
+ p++;
+ p += strcspn(p, "<>\n");
+ if (*p != '>')
return report(options, obj, FSCK_MSG_BAD_EMAIL, "invalid
author/committer line - bad email");
- (*ident)++;
- if (**ident != ' ')
+ p++;
+ if (*p != ' ')
return report(options, obj, FSCK_MSG_MISSING_SPACE_BEFORE_DATE,
"invalid author/committer line - missing space before date");
- (*ident)++;
- if (**ident == '0' && (*ident)[1] != ' ')
+ p++;
+ if (*p == '0' && p[1] != ' ')
return report(options, obj, FSCK_MSG_ZERO_PADDED_DATE, "invalid
author/committer line - zero-padded date");
- if (date_overflows(strtoul(*ident, &end, 10)))
+ if (date_overflows(strtoul(p, &end, 10)))
return report(options, obj, FSCK_MSG_DATE_OVERFLOW, "invalid
author/committer line - date causes integer overflow");
- if (end == *ident || *end != ' ')
+ if ((end == p || *end != ' '))
return report(options, obj, FSCK_MSG_BAD_DATE, "invalid
author/committer line - bad date");
- *ident = end + 1;
- if ((**ident != '+' && **ident != '-') ||
- !isdigit((*ident)[1]) ||
- !isdigit((*ident)[2]) ||
- !isdigit((*ident)[3]) ||
- !isdigit((*ident)[4]) ||
- ((*ident)[5] != '\n'))
+ p = end + 1;
+ if ((*p != '+' && *p != '-') ||
+ !isdigit(p[1]) ||
+ !isdigit(p[2]) ||
+ !isdigit(p[3]) ||
+ !isdigit(p[4]) ||
+ (p[5] != '\n'))
return report(options, obj, FSCK_MSG_BAD_TIMEZONE, "invalid
author/committer line - bad time zone");
- (*ident) += 6;
+ p += 6;
return 0;
}
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/19] fsck: Report the ID of the error/warning
Some legacy code has objects with non-fatal fsck issues; To enable the
user to ignore those issues, let's print out the ID (e.g. when
encountering "missing-email", the user might want to call `git config
receive.fsck.warn missing-email`).
Signed-off-by: Johannes Schindelin
---
fsck.c | 19 +++
t/t1450-fsck.sh | 4 ++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index 02715ee..09f69fe 100644
--- a/fsck.c
+++ b/fsck.c
@@ -161,6 +161,23 @@ void fsck_set_severity(struct fsck_options *options, const
char *mode)
}
}
+static void append_msg_id(struct strbuf *sb, const char *msg_id)
+{
+ for (;;) {
+ char c = *(msg_id)++;
+
+ if (!c)
+ break;
+ if (c == '_')
+ c = '-';
+ else
+ c = tolower(c);
+ strbuf_addch(sb, c);
+ }
+
+ strbuf_addstr(sb, ": ");
+}
+
__attribute__((format (printf, 4, 5)))
static int report(struct fsck_options *options, struct object *object,
enum fsck_msg_id id, const char *fmt, ...)
@@ -169,6 +186,8 @@ static int report(struct fsck_options *options, struct
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_severity = fsck_msg_severity(id, options), result;
+ append_msg_id(&sb, msg_id_info[id].id_string);
+
va_start(ap, fmt);
strbuf_vaddf(&sb, fmt, ap);
result = options->error_func(object, msg_severity, sb.buf);
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index cfb32b6..ea0f216 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -231,8 +231,8 @@ test_expect_success 'tag with incorrect tag name & missing
tagger' '
git fsck --tags 2>out &&
cat >expect <<-EOF &&
- warning in tag $tag: invalid '\''tag'\'' name: wrong name format
- warning in tag $tag: invalid format - expected '\''tagger'\'' line
+ warning in tag $tag: invalid-tag-name: invalid '\''tag'\'' name: wrong
name format
+ warning in tag $tag: missing-tagger-entry: invalid format - expected
'\''tagger'\'' line
EOF
test_cmp expect out
'
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 03/19] fsck: Provide a function to parse fsck message IDs
This function will be used in the next commits to allow the user to
ask fsck to handle specific problems differently, e.g. demoting certain
errors to warnings. It has to handle partial strings because we would
like to be able to parse, say, 'missing-email,missing-tagger-entry'
command lines.
To make the parsing robust, we generate strings from the enum keys, and
using these keys, we will map lower-case, dash-separated strings values
to the corresponding enum values.
Signed-off-by: Johannes Schindelin
---
fsck.c | 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index 30f7a48..2d91e28 100644
--- a/fsck.c
+++ b/fsck.c
@@ -63,15 +63,38 @@ enum fsck_msg_id {
};
#undef MSG_ID
-#define MSG_ID(id, severity) { FSCK_##severity },
+#define STR(x) #x
+#define MSG_ID(id, severity) { STR(id), FSCK_##severity },
static struct {
+ const char *id_string;
int severity;
} msg_id_info[FSCK_MSG_MAX + 1] = {
FOREACH_MSG_ID(MSG_ID)
- { -1 }
+ { NULL, -1 }
};
#undef MSG_ID
+static int parse_msg_id(const char *text, int len)
+{
+ int i, j;
+
+ for (i = 0; i < FSCK_MSG_MAX; i++) {
+ const char *key = msg_id_info[i].id_string;
+ /* id_string is upper-case, with underscores */
+ for (j = 0; j < len; j++) {
+ char c = *(key++);
+ if (c == '_')
+ c = '-';
+ if (text[j] != tolower(c))
+ break;
+ }
+ if (j == len && !*key)
+ return i;
+ }
+
+ die("Unhandled message id: %.*s", len, text);
+}
+
static int fsck_msg_severity(enum fsck_msg_id msg_id,
struct fsck_options *options)
{
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 04/19] fsck: Offer a function to demote fsck errors to warnings
There are legacy repositories out there whose older commits and tags
have issues that prevent pushing them when 'receive.fsckObjects' is set.
One real-life example is a commit object that has been hand-crafted to
list two authors.
Often, it is not possible to fix those issues without disrupting the
work with said repositories, yet it is still desirable to perform checks
by setting `receive.fsckObjects = true`. This commit is the first step
to allow demoting specific fsck issues to mere warnings.
The function added by this commit parses a list of settings in the form:
missing-email=warn,bad-name=warn,...
Unfortunately, the FSCK_WARN/FSCK_ERROR flag is only really heeded by
git fsck so far, but other call paths (e.g. git index-pack --strict)
error out *always* no matter what type was specified. Therefore, we
need to take extra care to default to all FSCK_ERROR in those cases.
Signed-off-by: Johannes Schindelin
---
fsck.c | 64 +---
fsck.h | 7 +--
2 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/fsck.c b/fsck.c
index 2d91e28..02715ee 100644
--- a/fsck.c
+++ b/fsck.c
@@ -100,13 +100,67 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- severity = msg_id_info[msg_id].severity;
- if (options->strict && severity == FSCK_WARN)
- severity = FSCK_ERROR;
+ if (options->msg_severity && msg_id >= 0 && msg_id < FSCK_MSG_MAX)
+ severity = options->msg_severity[msg_id];
+ else {
+ severity = msg_id_info[msg_id].severity;
+ if (options->strict && severity == FSCK_WARN)
+ severity = FSCK_ERROR;
+ }
return severity;
}
+static inline int substrcmp(const char *string, int len, const char *match)
+{
+ int match_len = strlen(match);
+ if (match_len != len)
+ return -1;
+ return memcmp(string, match, len);
+}
+
+void fsck_set_severity(struct fsck_options *options, const char *mode)
+{
+ int severity = FSCK_ERROR;
+
+ if (!options->msg_severity) {
+ int i;
+ int *msg_severity = xmalloc(sizeof(int) * FSCK_MSG_MAX);
+ for (i = 0; i < FSCK_MSG_MAX; i++)
+ msg_severity[i] = fsck_msg_severity(i, options);
+ options->msg_severity = msg_severity;
+ }
+
+ while (*mode) {
+ int len = strcspn(mode, " ,|"), equal, msg_id;
+
+ if (!len) {
+ mode++;
+ continue;
+ }
+
+ for (equal = 0; equal < len; equal++)
+ if (mode[equal] == '=')
+ break;
+
+ if (equal < len) {
+ if (!substrcmp(mode, equal, "error"))
+ severity = FSCK_ERROR;
+ else if (!substrcmp(mode, equal, "warn"))
+ severity = FSCK_WARN;
+ else
+ die("Unknown fsck message severity: '%.*s'",
+ equal, mode);
+ mode += equal + 1;
+ len -= equal + 1;
+ }
+
+ msg_id = parse_msg_id(mode, len);
+ options->msg_severity[msg_id] = severity;
+ mode += len;
+ }
+}
+
__attribute__((format (printf, 4, 5)))
static int report(struct fsck_options *options, struct object *object,
enum fsck_msg_id id, const char *fmt, ...)
@@ -596,6 +650,10 @@ int fsck_object(struct object *obj, void *data, unsigned
long size,
int fsck_error_function(struct object *obj, int severity, const char *message)
{
+ if (severity == FSCK_WARN) {
+ warning("object %s: %s", sha1_to_hex(obj->sha1), message);
+ return 0;
+ }
error("object %s: %s", sha1_to_hex(obj->sha1), message);
return 1;
}
diff --git a/fsck.h b/fsck.h
index f6f268a..4349860 100644
--- a/fsck.h
+++ b/fsck.h
@@ -6,6 +6,8 @@
struct fsck_options;
+void fsck_set_severity(struct fsck_options *options, const char *mode);
+
/*
* callback function for fsck_walk
* type is the expected type of the object or OBJ_ANY
@@ -25,10 +27,11 @@ struct fsck_options {
fsck_walk_func walk;
fsck_error error_func;
unsigned strict:1;
+ int *msg_severity;
};
-#define FSCK_OPTIONS_DEFAULT { NULL, fsck_error_function, 0 }
-#define FSCK_OPTIONS_STRICT { NULL, fsck_error_function, 1 }
+#define FSCK_OPTIONS_DEFAULT { NULL, fsck_error_function, 0, NULL }
+#define FSCK_OPTIONS_STRICT { NULL, fsck_error_function, 1, NULL }
/* descend in all linked child objects
* the return value is:
--
2.2.0.33.gc18b867
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
[PATCH v3 02/19] fsck: Introduce identifiers for fsck messages
Instead of specifying whether a message by the fsck machinery constitutes
an error or a warning, let's specify an identifier relating to the
concrete problem that was encountered. This is necessary for upcoming
support to be able to demote certain errors to warnings.
In the process, simplify the requirements on the calling code: instead of
having to handle full-blown varargs in every callback, we now send a
string buffer ready to be used by the callback.
We could use a simple enum for the message IDs here, but we want to
guarantee that the enum values are associated with the appropriate
severity levels. Besides, we want to introduce a parser in the next commit
that maps the string representation to the enum value, hence we use the
slightly ugly preprocessor construct that is extensible for use with said
parser.
Signed-off-by: Johannes Schindelin
---
builtin/fsck.c | 24 ++-
fsck.c | 201 +
fsck.h | 5 +-
3 files changed, 153 insertions(+), 77 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 2241e29..99d4538 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -47,32 +47,22 @@ static int show_dangling = 1;
#endif
static void objreport(struct object *obj, const char *severity,
- const char *err, va_list params)
+ const char *err)
{
- fprintf(stderr, "%s in %s %s: ",
- severity, typename(obj->type), sha1_to_hex(obj->sha1));
- vfprintf(stderr, err, params);
- fputs("\n", stderr);
+ fprintf(stderr, "%s in %s %s: %s\n",
+ severity, typename(obj->type), sha1_to_hex(obj->sha1), err);
}
-__attribute__((format (printf, 2, 3)))
-static int objerror(struct object *obj, const char *err, ...)
+static int objerror(struct object *obj, const char *err)
{
- va_list params;
- va_start(params, err);
errors_found |= ERROR_OBJECT;
- objreport(obj, "error", err, params);
- va_end(params);
+ objreport(obj, "error", err);
return -1;
}
-__attribute__((format (printf, 3, 4)))
-static int fsck_error_func(struct object *obj, int type, const char *err, ...)
+static int fsck_error_func(struct object *obj, int type, const char *message)
{
- va_list params;
- va_start(params, err);
- objreport(obj, (type == FSCK_WARN) ? "warning" : "error", err, params);
- va_end(params);
+ objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
return (type == FSCK_WARN) ? 0 : 1;
}
diff --git a/fsck.c b/fsck.c
index d83b811..30f7a48 100644
--- a/fsck.c
+++ b/fsck.c
@@ -9,6 +9,98 @@
#include "refs.h"
#include "utf8.h"
+#define FOREACH_MSG_ID(FUNC) \
+ /* errors */ \
+ FUNC(BAD_DATE, ERROR) \
+ FUNC(BAD_EMAIL, ERROR) \
+ FUNC(BAD_NAME, ERROR) \
+ FUNC(BAD_PARENT_SHA1, ERROR) \
+ FUNC(BAD_TIMEZONE, ERROR) \
+ FUNC(BAD_TREE_SHA1, ERROR) \
+ FUNC(DATE_OVERFLOW, ERROR) \
+ FUNC(DUPLICATE_ENTRIES, ERROR) \
+ FUNC(INVALID_OBJECT_SHA1, ERROR) \
+ FUNC(INVALID_TAG_OBJECT, ERROR) \
+ FUNC(INVALID_TREE, ERROR) \
+ FUNC(INVALID_TYPE, ERROR) \
+ FUNC(MISSING_AUTHOR, ERROR) \
+ FUNC(MISSING_COMMITTER, ERROR) \
+ FUNC(MISSING_EMAIL, ERROR) \
+ FUNC(MISSING_GRAFT, ERROR) \
+ FUNC(MISSING_NAME_BEFORE_EMAIL, ERROR) \
+ FUNC(MISSING_OBJECT, ERROR) \
+ FUNC(MISSING_PARENT, ERROR) \
+ FUNC(MISSING_SPACE_BEFORE_DATE, ERROR) \
+ FUNC(MISSING_SPACE_BEFORE_EMAIL, ERROR) \
+ FUNC(MISSING_TAG, ERROR) \
+ FUNC(MISSING_TAG_ENTRY, ERROR) \
+ FUNC(MISSING_TAG_OBJECT, ERROR) \
+ FUNC(MISSING_TREE, ERROR) \
+ FUNC(MISSING_TYPE, ERROR) \
+ FUNC(MISSING_TYPE_ENTRY, ERROR) \
+ FUNC(NOT_SORTED, ERROR) \
+ FUNC(NUL_IN_HEADER, ERROR) \
+ FUNC(TAG_OBJECT_NOT_TAG, ERROR) \
+ FUNC(UNKNOWN_TYPE, ERROR) \
+ FUNC(UNTERMINATED_HEADER, ERROR) \
+ FUNC(ZERO_PADDED_DATE, ERROR) \
+ /* warnings */ \
+ FUNC(BAD_FILEMODE, WARN) \
+ FUNC(EMPTY_NAME, WARN) \
+ FUNC(FULL_PATHNAME, WARN) \
+ FUNC(HAS_DOT, WARN) \
+ FUNC(HAS_DOTDOT, WARN) \
+ FUNC(HAS_DOTGIT, WARN) \
+ FUNC(INVALID_TAG_NAME, WARN) \
+ FUNC(MISSING_TAGGER_ENTRY, WARN) \
+ FUNC(NULL_SHA1, WARN) \
+ FUNC(ZERO_PADDED_FILEMODE, WARN)
+
+#define MSG_ID(id, severity) FSCK_MSG_##id,
+enum fsck_msg_id {
+ FOREACH_MSG_ID(MSG_ID)
+ FSCK_MSG_MAX
+};
+#undef MSG_ID
+
+#define MSG_ID(id, severity) { FSCK_##severity },
+static struct {
+ int severity;
+} msg_id_info[FSCK_MSG_MAX + 1] = {
+ FOREACH_MSG_ID(MSG_ID)
+ { -1 }
+};
+#undef MSG_ID
+
+static int fsck_msg_severity(enum fsck_msg_id msg_id,
+ struct fsck_options *options)
+{
+ int severity;
+
+ severity = msg_id_info[msg_id].severity;
+ if (options->strict && severity == FSCK_WARN)
+
[PATCH v3 01/19] fsck: Introduce fsck options
Just like the diff machinery, we are about to introduce more settings,
therefore it makes sense to carry them around as a (pointer to a) struct
containing all of them.
Signed-off-by: Johannes Schindelin
---
builtin/fsck.c | 20 +--
builtin/index-pack.c | 9 +--
builtin/unpack-objects.c | 11 ++--
fsck.c | 150 +++
fsck.h | 17 +-
5 files changed, 114 insertions(+), 93 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index a27515a..2241e29 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -25,6 +25,8 @@ static int include_reflogs = 1;
static int check_full = 1;
static int check_strict;
static int keep_cache_objects;
+static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
+static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
static unsigned char head_sha1[20];
static const char *head_points_at;
static int errors_found;
@@ -76,7 +78,7 @@ static int fsck_error_func(struct object *obj, int type,
const char *err, ...)
static struct object_array pending;
-static int mark_object(struct object *obj, int type, void *data)
+static int mark_object(struct object *obj, int type, void *data, struct
fsck_options *options)
{
struct object *parent = data;
@@ -119,7 +121,7 @@ static int mark_object(struct object *obj, int type, void
*data)
static void mark_object_reachable(struct object *obj)
{
- mark_object(obj, OBJ_ANY, NULL);
+ mark_object(obj, OBJ_ANY, NULL, NULL);
}
static int traverse_one_object(struct object *obj)
@@ -132,7 +134,7 @@ static int traverse_one_object(struct object *obj)
if (parse_tree(tree) < 0)
return 1; /* error already displayed */
}
- result = fsck_walk(obj, mark_object, obj);
+ result = fsck_walk(obj, obj, &fsck_walk_options);
if (tree)
free_tree_buffer(tree);
return result;
@@ -158,7 +160,7 @@ static int traverse_reachable(void)
return !!result;
}
-static int mark_used(struct object *obj, int type, void *data)
+static int mark_used(struct object *obj, int type, void *data, struct
fsck_options *options)
{
if (!obj)
return 1;
@@ -296,9 +298,9 @@ static int fsck_obj(struct object *obj)
fprintf(stderr, "Checking %s %s\n",
typename(obj->type), sha1_to_hex(obj->sha1));
- if (fsck_walk(obj, mark_used, NULL))
+ if (fsck_walk(obj, NULL, &fsck_obj_options))
objerror(obj, "broken links");
- if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func))
+ if (fsck_object(obj, NULL, 0, &fsck_obj_options))
return -1;
if (obj->type == OBJ_TREE) {
@@ -630,6 +632,12 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
+ fsck_walk_options.walk = mark_object;
+ fsck_obj_options.walk = mark_used;
+ fsck_obj_options.error_func = fsck_error_func;
+ if (check_strict)
+ fsck_obj_options.strict = 1;
+
if (show_progress == -1)
show_progress = isatty(2);
if (verbose)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4632117..925f7b5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -74,6 +74,7 @@ static int nr_threads;
static int from_stdin;
static int strict;
static int do_fsck_object;
+static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
static int verbose;
static int show_stat;
static int check_self_contained_and_connected;
@@ -191,7 +192,7 @@ static void cleanup_thread(void)
#endif
-static int mark_link(struct object *obj, int type, void *data)
+static int mark_link(struct object *obj, int type, void *data, struct
fsck_options *options)
{
if (!obj)
return -1;
@@ -782,10 +783,10 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
if (!obj)
die(_("invalid %s"), typename(type));
if (do_fsck_object &&
- fsck_object(obj, buf, size, 1,
- fsck_error_function))
+ fsck_object(obj, buf, size, &fsck_options))
die(_("Error in object"));
- if (fsck_walk(obj, mark_link, NULL))
+ fsck_options.walk = mark_link;
+ if (fsck_walk(obj, NULL, &fsck_options))
die(_("Not all child objects of %s are
reachable"), sha1_to_hex(obj->sha1));
if (obj->type == OBJ_TREE) {
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index ac66672..6d17040 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -20,6 +20,7 @@ static un
[PATCH v3 00/19] Introduce an internal API to interact with the fsck machinery
At the moment, the git-fsck's integrity checks are targeted toward the end user, i.e. the error messages are really just messages, intended for human consumption. Under certain circumstances, some of those errors should be allowed to be turned into mere warnings, though, because the cost of fixing the issues might well be larger than the cost of carrying those flawed objects. For example, when an already-public repository contains a commit object with two authors for years, it does not make sense to force the maintainer to rewrite the history, affecting all contributors negatively by forcing them to update. This branch introduces an internal fsck API to be able to turn some of the errors into warnings, and to make it easier to call the fsck machinery from elsewhere in general. I am proud to report that this work has been sponsored by GitHub. Interdiff vs v2 below the diffstat. Johannes Schindelin (19): fsck: Introduce fsck options fsck: Introduce identifiers for fsck messages fsck: Provide a function to parse fsck message IDs fsck: Offer a function to demote fsck errors to warnings fsck: Allow demoting errors to warnings via receive.fsck.warn = fsck: Report the ID of the error/warning fsck: Make fsck_ident() warn-friendly fsck: Make fsck_commit() warn-friendly fsck: Handle multiple authors in commits specially fsck: Make fsck_tag() warn-friendly fsck: Add a simple test for receive.fsck.* fsck: Disallow demoting grave fsck errors to warnings fsck: Optionally ignore specific fsck issues completely fsck: Allow upgrading fsck warnings to errors fsck: Document the new receive.fsck.* options. fsck: Support demoting errors to warnings fsck: Introduce `git fsck --quick` fsck: git receive-pack: support excluding objects from fsck'ing fsck: support ignoring objects in `git fsck` via fsck.skiplist Documentation/config.txt| 57 + Documentation/git-fsck.txt | 7 +- builtin/fsck.c | 76 -- builtin/index-pack.c| 13 +- builtin/receive-pack.c | 24 +- builtin/unpack-objects.c| 16 +- fsck.c | 525 +++- fsck.h | 27 ++- t/t1450-fsck.sh | 37 ++- t/t5302-pack-index.sh | 2 +- t/t5504-fetch-receive-strict.sh | 46 11 files changed, 668 insertions(+), 162 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 0daba8a..644411a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1208,7 +1208,9 @@ filter..smudge:: object to a worktree file upon checkout. See linkgit:gitattributes[5] for details. -fsck.*:: +fsck.error:: +fsck.warn:: +fsck.ignore:: The `fsck.error`, `fsck.warn` and `fsck.ignore` settings specify comma-separated lists of fsck message IDs which should trigger fsck to error out, to print the message and continue, or to ignore @@ -1221,6 +1223,13 @@ that setting `fsck.ignore = missing-email` will hide that issue. This feature is intended to support working with legacy repositories which cannot be repaired without disruptive changes. +fsck.skipList:: + The path to a sorted list of object names (i.e. one SHA-1 per + line) that are known to be broken in a non-fatal way and should + be ignored. This feature is useful when an established project + should be accepted despite early commits containing errors that + can be safely ignored such as invalid committer email addresses. + gc.aggressiveDepth:: The depth parameter used in the delta compression algorithm used by 'git gc --aggressive'. This defaults @@ -2143,31 +2152,41 @@ receive.fsckObjects:: Defaults to false. If not set, the value of `transfer.fsckObjects` is used instead. -receive.fsck.*:: +receive.fsck.error:: +receive.fsck.warn:: +receive.fsck.ignore:: When `receive.fsckObjects` is set to true, errors can be switched to warnings and vice versa by configuring the `receive.fsck.*` settings. These settings contain comma-separated lists of fsck message IDs. For convenience, fsck prefixes the error/warning with - the message ID, e.g. "missing-email: invalid author/committer line - - missing email" means that setting `receive.fsck.ignore = - missing-email` will hide that issue. + the message ID, e.g. "missing-email: invalid + author/committer line - missing email" means that setting + `receive.fsck.ignore = missing-email` will hide that issue. + -- - error:: - a comma-separated list of fsck message IDs that should be - trigger fsck to error out. - warn:: - a comma-separated list of fsck message IDs that should be - displayed, but fsck should continue to error out. - ignore:: - a comma-separated list of fsck message
[PATCH] git-add--interactive: print message if there are no untracked files
If user selects 'add untracked' and there are no untracked files,
"Add untracked>>" opens. But it does not make sense in this case, because there
are no untracked files. So let's print message and exit from "add untracked"
mode.
Signed-off-by: Alexander Kuleshov
---
git-add--interactive.perl | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 94b988c..1a6dcf3 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -724,11 +724,15 @@ sub revert_cmd {
}
sub add_untracked_cmd {
- my @add = list_and_choose({ PROMPT => 'Add untracked' },
- list_untracked());
- if (@add) {
- system(qw(git update-index --add --), @add);
- say_n_paths('added', @add);
+ if (system(qw(git ls-files --others --exclude-standard --))) {
+ my @add = list_and_choose({ PROMPT => 'Add untracked' },
+ list_untracked());
+ if (@add) {
+ system(qw(git update-index --add --), @add);
+ say_n_paths('added', @add);
+ }
+ } else {
+ print "No untracked files.\n";
}
print "\n";
}
--
2.3.0.rc1.247.gb53aa6f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] setting ulimit in test suite broken for me
On Wed, Jan 21, 2015 at 10:59:06AM -0800, Stefan Beller wrote:
> so I wanted to create a new test for large transactions, which should look
> like:
>
> run_with_limited_open_files () {
> (ulimit -n 64 && "$@")
> }
>
> test_lazy_prereq ULIMIT 'run_with_limited_open_files true'
>
> test_expect_success ULIMIT 'large transaction creating branches does
> not burst open file limit' '
> (
> echo $(ulimit -n)
> for i in $(seq 65)
> do
> echo "create refs/heads/$i HEAD"
> done >large_input &&
> git update-ref --stdingit rev-parse --verify -q refs/heads/65
> )
> '
>
> Mind the "echo $(ulimit -n)" in there as a debugging output.
> So if I run the test with "-d -v" to actually see the debugging output,
> I see ulimit -n set to 32768 instead of the desired 64.
You define run_with_limited_open_files, which starts a subshell, drops
the limit inside the subshell, and then spawns the child. But you never
call it in the test. Were you expecting that tests with the ULIMIT
prereq to automatically respect the limit? The prereq is just about
checking whether we can set the ulimit at all.
Try:
run_with_limited_open_files ulimit -n
in your debugging statement (and of course use run_with... for the
actual git command you want to limit).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i18n: bundle: mark git-bundle usage for translation
Signed-off-by: Alexander Kuleshov
---
builtin/bundle.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/bundle.c b/builtin/bundle.c
index 92a8a60..ca4a6a4 100644
--- a/builtin/bundle.c
+++ b/builtin/bundle.c
@@ -10,10 +10,10 @@
*/
static const char builtin_bundle_usage[] =
- "git bundle create \n"
- " or: git bundle verify \n"
- " or: git bundle list-heads [...]\n"
- " or: git bundle unbundle [...]";
+ N_("git bundle create \n"
+ " or: git bundle verify \n"
+ " or: git bundle list-heads [...]\n"
+ " or: git bundle unbundle [...]");
int cmd_bundle(int argc, const char **argv, const char *prefix)
{
--
2.3.0.rc1.246.g8d68d51.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG?] setting ulimit in test suite broken for me
Hi,
so I wanted to create a new test for large transactions, which should look like:
run_with_limited_open_files () {
(ulimit -n 64 && "$@")
}
test_lazy_prereq ULIMIT 'run_with_limited_open_files true'
test_expect_success ULIMIT 'large transaction creating branches does
not burst open file limit' '
(
echo $(ulimit -n)
for i in $(seq 65)
do
echo "create refs/heads/$i HEAD"
done >large_input &&
git update-ref --stdin
---
t/t7004-tag.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 35c805a..4f09cb4 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1460,20 +1460,21 @@ test_expect_success 'invalid sort parameter in
configuratoin' '
'
run_with_limited_stack () {
(ulimit -s 128 && "$@")
}
test_lazy_prereq ULIMIT 'run_with_limited_stack true'
# we require ulimit, this excludes Windows
test_expect_success ULIMIT '--contains works in a deep repo' '
+ echo $(ulimit -s) > ../ulimit_recorded
>expect &&
i=1 &&
while test $i -lt 8000
do
echo "commit refs/heads/master
committer A U Thor $((10 + $i * 100)) +0200
data
Re: [PATCH 20/24] update-index: test the system before enabling untracked cache
Duy Nguyen writes: > On Wed, Jan 21, 2015 at 3:32 PM, Junio C Hamano wrote: >> Nguyễn Thái Ngọc Duy writes: >> >>> Helped-by: Eric Sunshine >>> Signed-off-by: Nguyễn Thái Ngọc Duy >>> Signed-off-by: Junio C Hamano >>> --- >> >> It appears that this hijacks a fixed name dir-mtime-test at the root >> level of every project managed by Git. Is that intended? > > I did think about filename clash, but I chose a fixed name anyway for > simplicity, otherwise we would need to reconstruct paths > "dir-mtime-test/..." in many places. If you stuff the name of test directory (default "dir-mtime-test") in a strbuf and formulate test paths by chomping to the original and then appending "/..." at the end, like your remove_test_directory() already does, wouldn't that be sufficient? >> Shouldn't --use-untracked-cache option require the working tree >> (i.e. die in a bare repository)? > > setup_work_tree() takes care of that OK, thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/18] Introduce an internal API to interact with the fsck machinery
Hi Junio, On 2015-01-21 10:17, Junio C Hamano wrote: > The documentation did not format well. Tentatively I added the > attached fix-up on top of the series before merging to 'pu'. > > [...] Sorry for that! I have to admit that I did not even build the documentation :-( I incorporated your fixes into the respective patches. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 18/18] fsck: git receive-pack: support excluding objects from fsck'ing
Hi Junio, On 2015-01-21 10:02, Junio C Hamano wrote: > Johannes Schindelin writes: > >> The optional new config option `receive.fsck.skiplist` specifies the path >> to a file listing the names, i.e. SHA-1s, one per line, of objects that >> are to be ignored by `git receive-pack` when `receive.fsckObjects = true`. > > Makes sense, but I wonder if it makes sense to have a similar "ok to > be broken" list for "git fsck" (or perhaps they could even use the > same list) for exactly the same reason why this option makes sense. Sure! The most pressing use case for the skiplist feature is a Git server, hence this patch. I will implement the corresponding `git fsck` patch before re-submitting. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/18] fsck: Add a simple test for receive.fsck.*
Hi Junio, On 2015-01-21 09:59, Junio C Hamano wrote: > Johannes Schindelin writes: > >> Signed-off-by: Johannes Schindelin >> --- >> t/t5504-fetch-receive-strict.sh | 20 >> 1 file changed, 20 insertions(+) >> >> diff --git a/t/t5504-fetch-receive-strict.sh >> b/t/t5504-fetch-receive-strict.sh >> index 69ee13c..d491172 100755 >> --- a/t/t5504-fetch-receive-strict.sh >> +++ b/t/t5504-fetch-receive-strict.sh >> @@ -115,4 +115,24 @@ test_expect_success 'push with transfer.fsckobjects' ' >> test_cmp exp act >> ' >> >> +cat >bogus-commit << EOF > > "cat >bogus-commit <<\EOF", to reduce the mental burden of readers. Certainly! Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/18] fsck: Allow demoting errors to warnings via receive.fsck.warn =
Hi Junio,
On 2015-01-21 09:54, Junio C Hamano wrote:
> Johannes Schindelin writes:
>
>>
>> +if (starts_with(var, "receive.fsck.")) {
>> +if (fsck_severity.len)
>> +strbuf_addch(&fsck_severity, ',');
>> +strbuf_addf(&fsck_severity, "%s=%s", var + 13, value);
>
> Wouldn't it be safer to use skip_prefix() that lets you avoid the
> hardcoded "var + 13" here?
Yep, and much more elegant, too. I also fixed three more instances of the same
pattern.
>> @@ -1470,8 +1478,13 @@ static const char *unpack(int err_fd, struct
>> shallow_info *si)
>> argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
>> if (quiet)
>> argv_array_push(&child.args, "-q");
>> -if (fsck_objects)
>> -argv_array_push(&child.args, "--strict");
>> +if (fsck_objects) {
>> +if (fsck_severity.len)
>> +argv_array_pushf(&child.args, "--strict=%s",
>> +fsck_severity.buf);
>> +else
>> +argv_array_push(&child.args, "--strict");
>> +}
>> child.no_stdout = 1;
>> child.err = err_fd;
>> child.git_cmd = 1;
>> @@ -1488,8 +1501,13 @@ static const char *unpack(int err_fd, struct
>> shallow_info *si)
>>
>> argv_array_pushl(&child.args, "index-pack",
>> "--stdin", hdr_arg, keep_arg, NULL);
>> -if (fsck_objects)
>> -argv_array_push(&child.args, "--strict");
>> +if (fsck_objects) {
>> +if (fsck_severity.len)
>> +argv_array_pushf(&child.args, "--strict=%s",
>> +fsck_severity.buf);
>> +else
>> +argv_array_push(&child.args, "--strict");
>> +}
>
> Hmm. The above two hunks look suspiciously similar. Would it be
> worth to give them a single helper function?
Hmm. Not sure. I see what you mean, but for now I found
+ argv_array_pushf(&child.args, "--strict%s%s",
+ fsck_severity.len ? "=" : "",
+ fsck_severity.buf);
to be more elegant than to add a fully-fledged new function. But if you feel
strongly, I will gladly implement a separate function; I would appreciate
suggestions as to the function name...
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] git-svn updates from Ramkumar
Eric Wong wrote: > Adding it to GITPERLLIB should work... I tried everything including replicating the commands in the brew formula that does the Right thing: https://github.com/Homebrew/homebrew/blob/master/Library/Formula/git.rb I'm clearly missing something. Can someone on Darwin tell us the right flags/ env variables? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/18] fsck: Offer a function to demote fsck errors to warnings
Hi Junio,
On 2015-01-21 09:49, Junio C Hamano wrote:
> Johannes Schindelin writes:
>
>> +static inline int substrcmp(const char *string, int len, const char *match)
>> +{
>> +int match_len = strlen(match);
>> +if (match_len != len)
>> +return -1;
>> +return memcmp(string, match, len);
>> +}
>
> Is this what we call "starts_with()" these days?
Unfortunately not quite: It really requires the substring specified by `string`
and `len` to be identical to the full `match`. For example, `substrcmp("Hello
world!", 5, "Hell")` would report a failure (because the substring "Hello" is
*not* matching "Hell"), while `starts_with("Hello world!", "Hell")` would
obviously succeed.
>> +void fsck_set_severity(struct fsck_options *options, const char *mode)
>> +{
>> +int severity = FSCK_ERROR;
>> +
>> +if (!options->msg_severity) {
>> +int i;
>> +int *msg_severity = malloc(sizeof(int) * FSCK_MSG_MAX);
>
> xmalloc()?
Absolutely! Fixed.
Thanks,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] contacts: introduce --since and --min-percent
Ramkumar Ramachandra wrote: > Signed-off-by: Ramkumar Ramachandra Junio: this isn't in the latest rebuild of pu; I wonder if you've forgotten to pick it up. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] .clang-format: introduce the use of clang-format
René Scharfe wrote: > However, even then struct declarations that are combined with variable > declaration and initialization get mangled: I'm pretty sure this is a bug in clang-format. It might be semi-trivial to fix too. Thanks for your inputs. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] .clang-format: introduce the use of clang-format
Instead of manually eyeballing style in reviews, just ask all
contributors to run their patches through [git-]clang-format.
However, struct declarations that are combined with variable
declaration and initialization get mangled:
struct a {
int n;
const char *s;
} arr[] = {
{ 1, "one" },
{ 2, "two" }
};
becomes:
struct a {
int n;
const char *s;
} arr[] = { { 1, "one" }, { 2, "two" } };
It gets formatted better if arr is declared separately.
Helped-by: René Scharfe
Signed-off-by: Ramkumar Ramachandra
---
.clang-format | 11 +++
1 file changed, 11 insertions(+)
create mode 100644 .clang-format
diff --git a/.clang-format b/.clang-format
new file mode 100644
index 000..a336438
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,11 @@
+BasedOnStyle: LLVM
+IndentWidth: 8
+UseTab: Always
+BreakBeforeBraces: Linux
+AllowShortBlocksOnASingleLine: false
+AllowShortIfStatementsOnASingleLine: false
+IndentCaseLabels: false
+AllowShortFunctionsOnASingleLine: None
+ContinuationIndentWidth: 8
+Cpp11BracedListStyle: false
+BreakBeforeBraces: Stroustrup
--
2.2.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git index containing tree extension for unknown path
Is it allowed that the git index contains a tree extension mentioning patch 'x/y/z' while the only entry in the index is a '.gitattributes' files in the root? I have such a repo in a bug report against JGit [1]. Native git has no problems with this repo but JGit can't read such an index. I am wondering whether such a index is valid. [1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=457152 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/24] nd/untracked-cache update
On Wed, Jan 21, 2015 at 2:28 AM, Torsten Bögershausen wrote: > Do you have a commit on a public repo ? Just pushed to https://github.com/pclouds/git/commits/untracked-cache > pu + your serious, or master + V3 + this delta ? it's a replacement of nd/untracked-cache on 'pu'. You may need to merge it to master because the base is probably old. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/24] update-index: test the system before enabling untracked cache
On Wed, Jan 21, 2015 at 3:32 PM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Helped-by: Eric Sunshine >> Signed-off-by: Nguyễn Thái Ngọc Duy >> Signed-off-by: Junio C Hamano >> --- > > It appears that this hijacks a fixed name dir-mtime-test at the root > level of every project managed by Git. Is that intended? I did think about filename clash, but I chose a fixed name anyway for simplicity, otherwise we would need to reconstruct paths "dir-mtime-test/..." in many places. > Shouldn't --use-untracked-cache option require the working tree > (i.e. die in a bare repository)? setup_work_tree() takes care of that > ~/w/git $ git init --bare foo Khởi tạo trống rỗng kho Git trong /home/pclouds/w/git/foo/ > ~/w/git $ cd foo/ > ~/w/git/foo $ ../git update-index --untracked-cache fatal: This operation must be run in a work tree -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Makefile: collect some Makefile variables instead of directly assignment
Hello Junio, 2015-01-21 15:23 GMT+06:00 Junio C Hamano : > Alexander Kuleshov writes: > >> Some of Makefile variables as TEST_PROGRAMS_NEED_X and BUILTIN_OBJS filled >> directly by hand, let's collect it with the standard functions of 'make' >> util. > > I am not sure if we want to do this. > > $(wildcard) is a double-edged sword. It will grab any file that > matches on the filesystem, not just the ones we want to include in > the Git source set. I often have a file called test-something and > I'd prefer not to see such a random thing included in the build, > only because the filename matches some pattern. Yes, grabbing files by test-*.c is unreliable in this case. But what about builtin/*.c? Is there any plans that builtin will contain something another than builtin object files? > > While "we consider anything with a name that match the pattern we > say matter (e.g. test-*.c or builtin/*.c) as part of the source set" > is sometimes handy (it allows us to be lazy), it risks surprising > unsuspecting users. > > So I dunno. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Makefile: collect some Makefile variables instead of directly assignment
Alexander Kuleshov writes: > Some of Makefile variables as TEST_PROGRAMS_NEED_X and BUILTIN_OBJS filled > directly by hand, let's collect it with the standard functions of 'make' util. I am not sure if we want to do this. $(wildcard) is a double-edged sword. It will grab any file that matches on the filesystem, not just the ones we want to include in the Git source set. I often have a file called test-something and I'd prefer not to see such a random thing included in the build, only because the filename matches some pattern. While "we consider anything with a name that match the pattern we say matter (e.g. test-*.c or builtin/*.c) as part of the source set" is sometimes handy (it allows us to be lazy), it risks surprising unsuspecting users. So I dunno. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/18] Introduce an internal API to interact with the fsck machinery
The documentation did not format well. Tentatively I added the attached fix-up on top of the series before merging to 'pu'. * The wildcard in "fsck.*" and "receive.fsck.*" may have made sense back in v1 when the variables are unbounded set, but v2 fixes it and we now have a known fixed set of variables, so lets list them explicitly (this is not a "fix to unbreak formatting"). * The "--" that is not closed was giving me this: asciidoc: ERROR: git-config.txt: line 413: section title not permitted in delimited block asciidoc: ERROR: config.txt: line 2414: [blockdef-open] missing closing delimiter make: *** [git-config.xml] Error 1 (this is "workaround to unbreak formatting"; I didn't check the actual output closely). * the line that begins with "- missing email" after indent was taken as an bulletted item or something, so I rewrapped the paragraph somewhat to avoid having the dash at the beginning. Documentation/config.txt | 35 +++ 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6718578..aae66bb 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1208,7 +1208,9 @@ filter..smudge:: object to a worktree file upon checkout. See linkgit:gitattributes[5] for details. -fsck.*:: +fsck.error:: +fsck.warn:: +fsck.ignore:: The `fsck.error`, `fsck.warn` and `fsck.ignore` settings specify comma-separated lists of fsck message IDs which should trigger fsck to error out, to print the message and continue, or to ignore @@ -2138,25 +2140,26 @@ receive.fsckObjects:: Defaults to false. If not set, the value of `transfer.fsckObjects` is used instead. -receive.fsck.*:: +receive.fsck.error:: +receive.fsck.warn:: +receive.fsck.ignore:: When `receive.fsckObjects` is set to true, errors can be switched to warnings and vice versa by configuring the `receive.fsck.*` settings. These settings contain comma-separated lists of fsck message IDs. For convenience, fsck prefixes the error/warning with - the message ID, e.g. "missing-email: invalid author/committer line - - missing email" means that setting `receive.fsck.ignore = - missing-email` will hide that issue. -+ --- - error:: - a comma-separated list of fsck message IDs that should be - trigger fsck to error out. - warn:: - a comma-separated list of fsck message IDs that should be - displayed, but fsck should continue to error out. - ignore:: - a comma-separated list of fsck message IDs that should be - ignored completely. + the message ID, e.g. "missing-email: invalid + author/committer line - missing email" means that setting + `receive.fsck.ignore = missing-email` will hide that issue. ++ +error;; + a comma-separated list of fsck message IDs that should be + trigger fsck to error out. +warn;; + a comma-separated list of fsck message IDs that should be + displayed, but fsck should continue to error out. +ignore;; + a comma-separated list of fsck message IDs that should be + ignored completely. + This feature is intended to support working with legacy repositories which would not pass pushing when `receive.fsckObjects = true`, allowing -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Fix typos in various places
These patches provides two minor typo fixes in pack-bitmap.c and t/lib-terminal.sh Alexander Kuleshov (2): pack-bitmap: fix typo t/lib-terminal.sh: fix typo pack-bitmap.c | 2 +- t/lib-terminal.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.3.0-rc1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] t/lib-terminal.sh: fix typo
Signed-off-by: Alexander Kuleshov
---
t/lib-terminal.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 5184549..cd220e3 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,7 +1,7 @@
# Helpers for terminal output tests.
# Catch tests which should depend on TTY but forgot to. There's no need
-# to aditionally check that the TTY prereq is set here. If the test declared
+# to additionally check that the TTY prereq is set here. If the test declared
# it and we are running the test, then it must have been set.
test_terminal () {
if ! test_declared_prereq TTY
--
2.3.0-rc1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] pack-bitmap: fix typo
Signed-off-by: Alexander Kuleshov
---
pack-bitmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 0cd85f6..365f9d9 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -60,7 +60,7 @@ static struct bitmap_index {
struct ewah_bitmap *blobs;
struct ewah_bitmap *tags;
- /* Map from SHA1 -> `stored_bitmap` for all the bitmapped comits */
+ /* Map from SHA1 -> `stored_bitmap` for all the bitmapped commits */
khash_sha1 *bitmaps;
/* Number of bitmapped commits */
--
2.3.0-rc1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Fix typos in various places
These patches provides two minor typo fixes in pack-bitmap.c and t/lib-terminal.sh Alexander Kuleshov (2): pack-bitmap: fix typo t/lib-terminal.sh: fix typo pack-bitmap.c | 2 +- t/lib-terminal.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.3.0-rc1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 18/18] fsck: git receive-pack: support excluding objects from fsck'ing
Johannes Schindelin writes: > The optional new config option `receive.fsck.skiplist` specifies the path > to a file listing the names, i.e. SHA-1s, one per line, of objects that > are to be ignored by `git receive-pack` when `receive.fsckObjects = true`. Makes sense, but I wonder if it makes sense to have a similar "ok to be broken" list for "git fsck" (or perhaps they could even use the same list) for exactly the same reason why this option makes sense. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/18] fsck: Add a simple test for receive.fsck.*
Johannes Schindelin writes: > Signed-off-by: Johannes Schindelin > --- > t/t5504-fetch-receive-strict.sh | 20 > 1 file changed, 20 insertions(+) > > diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh > index 69ee13c..d491172 100755 > --- a/t/t5504-fetch-receive-strict.sh > +++ b/t/t5504-fetch-receive-strict.sh > @@ -115,4 +115,24 @@ test_expect_success 'push with transfer.fsckobjects' ' > test_cmp exp act > ' > > +cat >bogus-commit << EOF "cat >bogus-commit <<\EOF", to reduce the mental burden of readers. > +tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 > +author Bugs Bunny 1234567890 + > +committer Bugs Bunny 1234567890 + > + > +This commit object intentionally broken > +EOF > + > +test_expect_success 'push with receive.fsck.warn = missing-email' ' > + commit="$(git hash-object -t commit -w --stdin < bogus-commit)" && > + git push . $commit:refs/heads/bogus && > + rm -rf dst && > + git init dst && > + git --git-dir=dst/.git config receive.fsckobjects true && > + test_must_fail git push --porcelain dst bogus && > + git --git-dir=dst/.git config receive.fsck.warn missing-email && > + git push --porcelain dst bogus >act 2>&1 && > + grep "missing-email" act > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/18] fsck: Make fsck_ident() warn-friendly
Johannes Schindelin writes:
> When fsck_ident() identifies a problem with the ident, it should still
> advance the pointer to the next line so that fsck can continue in the
> case of a mere warning.
Quite sensible.
>
> Signed-off-by: Johannes Schindelin
> ---
> fsck.c | 49 +++--
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index 78944f0..233385b 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -453,40 +453,45 @@ static int require_end_of_header(const void *data,
> unsigned long size,
>
> static int fsck_ident(const char **ident, struct object *obj, struct
> fsck_options *options)
> {
> + const char *p = *ident;
> char *end;
>
> - if (**ident == '<')
> + *ident = strchrnul(*ident, '\n');
> + if (**ident == '\n')
> + (*ident)++;
> +
> + if (*p == '<')
> return report(options, obj, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL,
> "invalid author/committer line - missing space before email");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/18] fsck: Allow demoting errors to warnings via receive.fsck.warn =
Johannes Schindelin writes:
>
> + if (starts_with(var, "receive.fsck.")) {
> + if (fsck_severity.len)
> + strbuf_addch(&fsck_severity, ',');
> + strbuf_addf(&fsck_severity, "%s=%s", var + 13, value);
Wouldn't it be safer to use skip_prefix() that lets you avoid the
hardcoded "var + 13" here?
> @@ -1470,8 +1478,13 @@ static const char *unpack(int err_fd, struct
> shallow_info *si)
> argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
> if (quiet)
> argv_array_push(&child.args, "-q");
> - if (fsck_objects)
> - argv_array_push(&child.args, "--strict");
> + if (fsck_objects) {
> + if (fsck_severity.len)
> + argv_array_pushf(&child.args, "--strict=%s",
> + fsck_severity.buf);
> + else
> + argv_array_push(&child.args, "--strict");
> + }
> child.no_stdout = 1;
> child.err = err_fd;
> child.git_cmd = 1;
> @@ -1488,8 +1501,13 @@ static const char *unpack(int err_fd, struct
> shallow_info *si)
>
> argv_array_pushl(&child.args, "index-pack",
>"--stdin", hdr_arg, keep_arg, NULL);
> - if (fsck_objects)
> - argv_array_push(&child.args, "--strict");
> + if (fsck_objects) {
> + if (fsck_severity.len)
> + argv_array_pushf(&child.args, "--strict=%s",
> + fsck_severity.buf);
> + else
> + argv_array_push(&child.args, "--strict");
> + }
Hmm. The above two hunks look suspiciously similar. Would it be
worth to give them a single helper function?
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 6d17040..82f2d62 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -530,6 +530,11 @@ int cmd_unpack_objects(int argc, const char **argv,
> const char *prefix)
> strict = 1;
> continue;
> }
> + if (starts_with(arg, "--strict=")) {
> + strict = 1;
> + fsck_set_severity(&fsck_options, arg + 9);
> + continue;
> + }
> if (starts_with(arg, "--pack_header=")) {
> struct pack_header *hdr;
> char *c;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Makefile: collect some Makefile variables instead of directly assignment
Some of Makefile variables as TEST_PROGRAMS_NEED_X and BUILTIN_OBJS filled directly by hand, let's collect it with the standard functions of 'make' util. Signed-off-by: Alexander Kuleshov --- Makefile | 134 +-- 1 file changed, 2 insertions(+), 132 deletions(-) diff --git a/Makefile b/Makefile index b5b4cee..055db4b 100644 --- a/Makefile +++ b/Makefile @@ -563,38 +563,7 @@ X = PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS)) -TEST_PROGRAMS_NEED_X += test-chmtime -TEST_PROGRAMS_NEED_X += test-ctype -TEST_PROGRAMS_NEED_X += test-config -TEST_PROGRAMS_NEED_X += test-date -TEST_PROGRAMS_NEED_X += test-delta -TEST_PROGRAMS_NEED_X += test-dump-cache-tree -TEST_PROGRAMS_NEED_X += test-dump-split-index -TEST_PROGRAMS_NEED_X += test-dump-untracked-cache -TEST_PROGRAMS_NEED_X += test-genrandom -TEST_PROGRAMS_NEED_X += test-hashmap -TEST_PROGRAMS_NEED_X += test-index-version -TEST_PROGRAMS_NEED_X += test-line-buffer -TEST_PROGRAMS_NEED_X += test-match-trees -TEST_PROGRAMS_NEED_X += test-mergesort -TEST_PROGRAMS_NEED_X += test-mktemp -TEST_PROGRAMS_NEED_X += test-parse-options -TEST_PROGRAMS_NEED_X += test-path-utils -TEST_PROGRAMS_NEED_X += test-prio-queue -TEST_PROGRAMS_NEED_X += test-read-cache -TEST_PROGRAMS_NEED_X += test-regex -TEST_PROGRAMS_NEED_X += test-revision-walking -TEST_PROGRAMS_NEED_X += test-run-command -TEST_PROGRAMS_NEED_X += test-scrap-cache-tree -TEST_PROGRAMS_NEED_X += test-sha1 -TEST_PROGRAMS_NEED_X += test-sha1-array -TEST_PROGRAMS_NEED_X += test-sigchain -TEST_PROGRAMS_NEED_X += test-string-list -TEST_PROGRAMS_NEED_X += test-submodule-config -TEST_PROGRAMS_NEED_X += test-subprocess -TEST_PROGRAMS_NEED_X += test-svn-fe -TEST_PROGRAMS_NEED_X += test-urlmatch-normalization -TEST_PROGRAMS_NEED_X += test-wildmatch +TEST_PROGRAMS_NEED_X = $(patsubst %.c,%, $(wildcard test-*.c)) TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X)) @@ -811,105 +780,7 @@ LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o LIB_OBJS += zlib.o -BUILTIN_OBJS += builtin/add.o -BUILTIN_OBJS += builtin/annotate.o -BUILTIN_OBJS += builtin/apply.o -BUILTIN_OBJS += builtin/archive.o -BUILTIN_OBJS += builtin/bisect--helper.o -BUILTIN_OBJS += builtin/blame.o -BUILTIN_OBJS += builtin/branch.o -BUILTIN_OBJS += builtin/bundle.o -BUILTIN_OBJS += builtin/cat-file.o -BUILTIN_OBJS += builtin/check-attr.o -BUILTIN_OBJS += builtin/check-ignore.o -BUILTIN_OBJS += builtin/check-mailmap.o -BUILTIN_OBJS += builtin/check-ref-format.o -BUILTIN_OBJS += builtin/checkout-index.o -BUILTIN_OBJS += builtin/checkout.o -BUILTIN_OBJS += builtin/clean.o -BUILTIN_OBJS += builtin/clone.o -BUILTIN_OBJS += builtin/column.o -BUILTIN_OBJS += builtin/commit-tree.o -BUILTIN_OBJS += builtin/commit.o -BUILTIN_OBJS += builtin/config.o -BUILTIN_OBJS += builtin/count-objects.o -BUILTIN_OBJS += builtin/credential.o -BUILTIN_OBJS += builtin/describe.o -BUILTIN_OBJS += builtin/diff-files.o -BUILTIN_OBJS += builtin/diff-index.o -BUILTIN_OBJS += builtin/diff-tree.o -BUILTIN_OBJS += builtin/diff.o -BUILTIN_OBJS += builtin/fast-export.o -BUILTIN_OBJS += builtin/fetch-pack.o -BUILTIN_OBJS += builtin/fetch.o -BUILTIN_OBJS += builtin/fmt-merge-msg.o -BUILTIN_OBJS += builtin/for-each-ref.o -BUILTIN_OBJS += builtin/fsck.o -BUILTIN_OBJS += builtin/gc.o -BUILTIN_OBJS += builtin/get-tar-commit-id.o -BUILTIN_OBJS += builtin/grep.o -BUILTIN_OBJS += builtin/hash-object.o -BUILTIN_OBJS += builtin/help.o -BUILTIN_OBJS += builtin/index-pack.o -BUILTIN_OBJS += builtin/init-db.o -BUILTIN_OBJS += builtin/interpret-trailers.o -BUILTIN_OBJS += builtin/log.o -BUILTIN_OBJS += builtin/ls-files.o -BUILTIN_OBJS += builtin/ls-remote.o -BUILTIN_OBJS += builtin/ls-tree.o -BUILTIN_OBJS += builtin/mailinfo.o -BUILTIN_OBJS += builtin/mailsplit.o -BUILTIN_OBJS += builtin/merge.o -BUILTIN_OBJS += builtin/merge-base.o -BUILTIN_OBJS += builtin/merge-file.o -BUILTIN_OBJS += builtin/merge-index.o -BUILTIN_OBJS += builtin/merge-ours.o -BUILTIN_OBJS += builtin/merge-recursive.o -BUILTIN_OBJS += builtin/merge-tree.o -BUILTIN_OBJS += builtin/mktag.o -BUILTIN_OBJS += builtin/mktree.o -BUILTIN_OBJS += builtin/mv.o -BUILTIN_OBJS += builtin/name-rev.o -BUILTIN_OBJS += builtin/notes.o -BUILTIN_OBJS += builtin/pack-objects.o -BUILTIN_OBJS += builtin/pack-redundant.o -BUILTIN_OBJS += builtin/pack-refs.o -BUILTIN_OBJS += builtin/patch-id.o -BUILTIN_OBJS += builtin/prune-packed.o -BUILTIN_OBJS += builtin/prune.o -BUILTIN_OBJS += builtin/push.o -BUILTIN_OBJS += builtin/read-tree.o -BUILTIN_OBJS += builtin/receive-pack.o -BUILTIN_OBJS += builtin/reflog.o -BUILTIN_OBJS += builtin/remote.o -BUILTIN_OBJS += builtin/remote-ext.o -BUILTIN_OBJS += builtin/remote-fd.o -BUILTIN_OBJS += builtin/repack.o -BUILTIN_OBJS += builtin/replace.o -BUILTIN_OBJS += builtin/rerere.o -BUILTIN_OBJS += builtin/reset.o -BUILTIN_OBJS += builtin/rev-list.o -BUILTIN_OBJS += builtin/rev-parse.o -BUILTIN_OBJS += builtin/revert.o -BUILTIN_OBJS += builtin/rm.o -BUILTIN_
Re: [PATCH v2 04/18] fsck: Offer a function to demote fsck errors to warnings
Johannes Schindelin writes:
> +static inline int substrcmp(const char *string, int len, const char *match)
> +{
> + int match_len = strlen(match);
> + if (match_len != len)
> + return -1;
> + return memcmp(string, match, len);
> +}
Is this what we call "starts_with()" these days?
> +void fsck_set_severity(struct fsck_options *options, const char *mode)
> +{
> + int severity = FSCK_ERROR;
> +
> + if (!options->msg_severity) {
> + int i;
> + int *msg_severity = malloc(sizeof(int) * FSCK_MSG_MAX);
xmalloc()?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/24] update-index: test the system before enabling untracked cache
Nguyễn Thái Ngọc Duy writes: > Helped-by: Eric Sunshine > Signed-off-by: Nguyễn Thái Ngọc Duy > Signed-off-by: Junio C Hamano > --- It appears that this hijacks a fixed name dir-mtime-test at the root level of every project managed by Git. Is that intended? Shouldn't --use-untracked-cache option require the working tree (i.e. die in a bare repository)? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
