Re: [PATCH V2 1/2] t9801: check git-p4's branch detection and client view together
On April 20, 2015 6:43:49 AM GMT+01:00, Junio C Hamano gits...@pobox.com wrote: Vitor Antunes vitor@gmail.com writes: Add failing scenario where branch detection is enabled together with use client view. In this specific scenario git-p4 will break when the perforce client view removes part of the depot path. I somehow cannot parse together with use client view, especially the word use. Is it user client view or something (I am not familiar with p4 lingo), or perhaps use of client view? I meant spec instead of view. As in - -use-client-spec. In perforce you need to configure your workspace using a client specification. One of the configured values is the client view, which maps files/folders in the server to locations in your local workspace. What I'm trying to fix with these patches is the ability of git-p4 to process the client view definition through the use of p4 where to determine the correct location of the local files, such that it is able to apply the necessary patches for submission to the perforce server. While thinking about client views I completely forgot that the git-p4 argument that enables thos feature uses spec and not view. -- 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 0/3] Another approach to large transactions
Stefan Beller sbel...@google.com writes: The problem comes from guessing the number of fds we're allowed to use. At first I thought it was a fundamental issue with the code being broken, but it turns out we just need a larger offset as we apparently have 9 files open already, before the transaction even starts. I did not expect the number to be that high, which is why I came up with the arbitrary number of 8 (3 for stdin/out/err, maybe packed refs and reflog so I guessed, 8 would do fine). I am not sure if the 9 is a constant or if it scales to some unknown property yet. So to make the series work, all we need is: - int remaining_fds = get_max_fd_limit() - 8; + int remaining_fds = get_max_fd_limit() - 9; I am going to try to understand where the 9 comes from and resend the patches. I have a suspicion that the above is an indication that the approach is fundamentally not sound. 9 may be OK in your test repository, but that may fail in a repository with different resource usage patterns. On the core management side, xmalloc() and friends retry upon failure, after attempting to free the resource. I wonder if your codepath can do something similar to that, perhaps? On the other hand, it may be that this let's keep it open as long as possible, as creat-close-open-write-close is more expensive may not be worth the complexity. I wonder if it might not be a bad idea to start with a simpler rule, e.g. use creat-write-close for ref updates outside transactions, and creat-close-open-write-close for inside transactions, as that is likely to be multi-ref updates or something stupid and simple like that? Michael? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Apr 2015, #03; Mon, 20)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. 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] * jc/push-cert (2015-04-02) 1 commit (merged to 'next' on 2015-04-08 at aecdd43) + push --signed: tighten what the receiving end can ask to sign The git push --signed protocol extension did not limit what the nonce that is a server-chosen string can contain or how long it can be, which was unnecessarily lax. Limit both the length and the alphabet to a reasonably small space that can still have enough entropy. * ma/bash-completion-leaking-x (2015-04-12) 1 commit (merged to 'next' on 2015-04-14 at 3a52a6d) + completion: fix global bash variable leak on __gitcompappend The completion script (in contrib/) contaminated global namespace and clobbered on a shell variable $x. * ps/grep-help-all-callback-arg (2015-04-12) 1 commit (merged to 'next' on 2015-04-14 at e0a8092) + grep: correctly initialize help-all option Code clean-up. * tb/connect-ipv6-parse-fix (2015-04-08) 1 commit (merged to 'next' on 2015-04-14 at e720918) + connect.c: ignore extra colon after hostname An earlier update to the parser that disects a URL broke an address, followed by a colon, followed by an empty string (instead of the port number), e.g. ssh://example.com:/path/to/repo. * va/fix-git-p4-tests (2015-04-12) 3 commits (merged to 'next' on 2015-04-14 at 261bf90) + t9814: guarantee only one source exists in git-p4 copy tests + git-p4: fix copy detection test + t9814: fix broken shell syntax in git-p4 rename test Test fixes for git-p4. -- [New Topics] * jc/epochtime-wo-tz (2015-04-15) 2 commits - parse_date_basic(): let the system handle DST conversion - parse_date_basic(): return early when given a bogus timestamp git commit --date=now or anything that relies on approxidate lost the daylight-saving-time offset. Will merge to 'next'. * jc/plug-fmt-merge-msg-leak (2015-04-20) 1 commit - fmt-merge-msg: plug small leak of commit buffer Will merge to 'next'. * cn/bom-in-gitignore (2015-04-16) 5 commits - attr: skip UTF8 BOM at the beginning of the input file - config: use utf8_bom[] from utf.[ch] in git_parse_source() - utf8-bom: introduce skip_utf8_bom() helper - add_excludes_from_file: clarify the bom skipping logic - dir: allow a BOM at the beginning of exclude files Teach the codepaths that read .gitignore and .gitattributes files that these files encoded in UTF-8 may have UTF-8 BOM marker at the beginning; this makes it in line with what we do for configuration files already. Will merge to 'next'. * ee/clean-remove-dirs (2015-04-18) 4 commits - clean: improve performance when removing lots of directories - p7300: add performance tests for clean - t7300: add tests to document behavior of clean and nested git - setup: add gentle version of read_gitfile Still WIP. * ep/fix-test-lib-functions-report (2015-04-16) 1 commit - test-lib-functions.sh: fix the second argument to some helper functions Will merge to 'next'. * jk/still-interesting (2015-04-17) 1 commit - limit_list: avoid quadratic behavior from still_interesting git rev-list --objects $old --not --all to see if everything that is reachable from $old is already connected to the existing refs was very inefficient. Will merge to 'next'. * jk/type-from-string-gently (2015-04-17) 1 commit (merged to 'next' on 2015-04-20 at a97611f) + type_from_string_gently: make sure length matches git cat-file bl $blob failed to barf even though there is no object type that is bl. * ls/p4-changes-block-size (2015-04-20) 1 commit - git-p4: use -m when running p4 changes git p4 learned --changes-block-size n to read the changes in chunks from Perforce, instead of making one call to p4 changes that may trigger too many rows scanned error from Perforce. Will merge to 'next'. * mg/show-notes-doc (2015-04-17) 1 commit (merged to 'next' on 2015-04-20 at 2e93969) + rev-list-options.txt: complete sentence about notes matching Documentation fix. Will merge to 'master' in the first batch of post v2.4 cycle. * mm/add-p-split-error (2015-04-16) 5 commits - stash -p: demonstrate failure of split with mixed y/n - t3904-stash-patch: factor PERL prereq at the top of the file - t3904-stash-patch: fix test description - add -p: demonstrate failure when running 'edit' after a split - t3701-add-interactive: simplify code * mm/usage-log-l-can-take-regex (2015-04-20) 2 commits - log -L: improve error message on malformed argument - Documentation: change -L:regex to -L:funcname Will merge to 'next'. * nd/pathspec-strip-fix (2015-04-18) 1 commit - pathspec:
Re: [PATCH 0/3] Another approach to large transactions
On Mon, Apr 20, 2015 at 3:51 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: The problem comes from guessing the number of fds we're allowed to use. At first I thought it was a fundamental issue with the code being broken, but it turns out we just need a larger offset as we apparently have 9 files open already, before the transaction even starts. I did not expect the number to be that high, which is why I came up with the arbitrary number of 8 (3 for stdin/out/err, maybe packed refs and reflog so I guessed, 8 would do fine). I am not sure if the 9 is a constant or if it scales to some unknown property yet. So to make the series work, all we need is: - int remaining_fds = get_max_fd_limit() - 8; + int remaining_fds = get_max_fd_limit() - 9; I am going to try to understand where the 9 comes from and resend the patches. I have a suspicion that the above is an indication that the approach is fundamentally not sound. 9 may be OK in your test repository, but that may fail in a repository with different resource usage patterns. You put my concerns in a better wording. On the core management side, xmalloc() and friends retry upon failure, after attempting to free the resource. I wonder if your codepath can do something similar to that, perhaps? But then we'd need to think about which fds can be 'garbage collected'. The lock files certainly can be closed and reopened. The first 3 fd not so. So we'd need to maintain a data structure of file descriptors good/bad for this reclaiming. On the other hand, it may be that this let's keep it open as long as possible, as creat-close-open-write-close is more expensive may not be worth the complexity. I wonder if it might not be a bad idea to start with a simpler rule, e.g. use creat-write-close for ref updates outside transactions, and creat-close-open-write-close for inside transactions, as that is likely to be multi-ref updates or something stupid and simple like that? I thought about any ref about goes through transaction nowadays. Having the current patches the first n locks are creat-write-close, while the remaining locks have the creat-close-open-write-close pattern, so it slows only the large transactions. My plan is to strace all open calls and check if the aforementioned 9 open files are just a constant. Michael? -- 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] refs.c: enable large transactions
This is another attempt on enabling large transactions
(large in terms of open file descriptors). We keep track of how many
lock files are opened by the ref_transaction_commit function.
When more than a reasonable amount of files is open, we close
the file descriptors to make sure the transaction can continue.
Another idea I had during implementing this was to move this file
closing into the lock file API, such that only a certain amount of
lock files can be open at any given point in time and we'd be 'garbage
collecting' open fds when necessary in any relevant call to the lock
file API. This would have brought the advantage of having such
functionality available in other users of the lock file API as well.
The downside however is the over complication, you really need to always
check for (lock-fd != -1) all the time, which may slow down other parts
of the code, which did not ask for such a feature.
Signed-off-by: Stefan Beller sbel...@google.com
---
This replaces the latest patch on origin/sb/remove-fd-from-ref-lock
The test suite passes now
refs.c| 13 +
t/t1400-update-ref.sh | 4 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 4f495bd..1e8cb16 100644
--- a/refs.c
+++ b/refs.c
@@ -3041,6 +3041,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
+ if (lock-lk-fd == -1)
+ reopen_lock_file(lock-lk);
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) {
@@ -3719,6 +3721,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
{
int ret = 0, i;
int n = transaction-nr;
+ /*
+* We may want to open many files in a large transaction, so come up
with
+* a reasonable maximum, keep some spares for stdin/out and other open
+* files.
+*/
+ int remaining_fds = get_max_fd_limit() - 32;
struct ref_update **updates = transaction-updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
struct string_list_item *ref_to_delete;
@@ -3762,6 +3770,11 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ if (remaining_fds 0) {
+ remaining_fds--;
+ } else {
+ 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 7a69f1a..636d3a1 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_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
@@ -1082,7 +1082,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.4.0.rc2.5.g4c2045b.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
Re: [PATCH] rev-list-options.txt: complete sentence about notes matching
Michael J Gruber g...@drmicha.warpmail.net writes: Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Documentation/rev-list-options.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index f620ee4..77ac439 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -59,8 +59,8 @@ endif::git-rev-list[] matches any of the given patterns are chosen (but see `--all-match`). + -When `--show-notes` is in effect, the message from the notes as -if it is part of the log message. +When `--show-notes` is in effect, the message from the notes is +matched as if it were part of the log message. --all-match:: Limit the commits output to ones that match all given `--grep`, Makes sense, 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 0/3] Another approach to large transactions
On Mon, Apr 20, 2015 at 4:07 PM, Stefan Beller sbel...@google.com wrote: On Mon, Apr 20, 2015 at 3:51 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: The problem comes from guessing the number of fds we're allowed to use. At first I thought it was a fundamental issue with the code being broken, but it turns out we just need a larger offset as we apparently have 9 files open already, before the transaction even starts. I did not expect the number to be that high, which is why I came up with the arbitrary number of 8 (3 for stdin/out/err, maybe packed refs and reflog so I guessed, 8 would do fine). I am not sure if the 9 is a constant or if it scales to some unknown property yet. So to make the series work, all we need is: - int remaining_fds = get_max_fd_limit() - 8; + int remaining_fds = get_max_fd_limit() - 9; I am going to try to understand where the 9 comes from and resend the patches. I have a suspicion that the above is an indication that the approach is fundamentally not sound. 9 may be OK in your test repository, but that may fail in a repository with different resource usage patterns. You put my concerns in a better wording. On the core management side, xmalloc() and friends retry upon failure, after attempting to free the resource. I wonder if your codepath can do something similar to that, perhaps? But then we'd need to think about which fds can be 'garbage collected'. The lock files certainly can be closed and reopened. The first 3 fd not so. So we'd need to maintain a data structure of file descriptors good/bad for this reclaiming. On the other hand, it may be that this let's keep it open as long as possible, as creat-close-open-write-close is more expensive may not be worth the complexity. I wonder if it might not be a bad idea to start with a simpler rule, e.g. use creat-write-close for ref updates outside transactions, and creat-close-open-write-close for inside transactions, as that is likely to be multi-ref updates or something stupid and simple like that? I thought about any ref about goes through transaction nowadays. Having the current patches the first n locks are creat-write-close, while the remaining locks have the creat-close-open-write-close pattern, so it slows only the large transactions. My plan is to strace all open calls and check if the aforementioned 9 open files are just a constant. When running the test locally, i.e. not in the test suite, but typing the commands myself into the shell, Git is fine with having just 5 file descriptors left. The additional 4 required fds come from beign run inside the test suite. When strace-ing git, I cannot see any possible other fds which would require having some left over space required. So I'd propose we'd just take a reasonable number not too small for various test setups like 32 and then go with the proposed patches. I'll just resend the patches to have a new basis for discussion. Michael? -- 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] fmt-merge-msg: plug small leak of commit buffer
Jeff King p...@peff.net writes:
I note that record_person does not seem to care about the commit at all,
so an alternative fix would be:
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 1d962dc..9f0e608 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -223,16 +223,14 @@ static void add_branch_desc(struct strbuf *out, const
char *name)
#define util_as_integral(elem) ((intptr_t)((elem)-util))
-static void record_person(int which, struct string_list *people,
- struct commit *commit)
+static void record_person_from_buf(int which, struct string_list *people,
+const char *buffer)
{
- const char *buffer;
char *name_buf, *name, *name_end;
struct string_list_item *elem;
const char *field;
field = (which == 'a') ? \nauthor : \ncommitter ;
- buffer = get_commit_buffer(commit, NULL);
name = strstr(buffer, field);
if (!name)
return;
@@ -245,7 +243,6 @@ static void record_person(int which, struct string_list
*people,
if (name_end name)
return;
name_buf = xmemdupz(name, name_end - name + 1);
- unuse_commit_buffer(commit, buffer);
elem = string_list_lookup(people, name_buf);
if (!elem) {
@@ -256,6 +253,14 @@ static void record_person(int which, struct string_list
*people,
free(name_buf);
}
+static void record_person(int which, struct string_list *people,
+ struct commit *commit)
+{
+ const char *buf = get_commit_buffer(commit, NULL);
+ record_person_from_buf(which, people, buf);
+ unuse_commit_buffer(commit, buf);
+}
+
static int cmp_string_list_util_as_integral(const void *a_, const void *b_)
{
const struct string_list_item *a = a_, *b = b_;
This has the slight advantage that it adapts naturally if record_person
grows more exits, but I don't think it is a big deal either way (it only
matters if the new exit fails to copy the surrounding code and use goto
leave).
Yeah, let me steal that from you.
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/RFC v3 0/4] Improving performance of git clean
On 04/18, Erik Elfström wrote: * Still have issues in the performance tests, see comments from Thomas Gummerer on v2 I've looked at the modern style tests again, and I don't the code churn is worth it just for using them for the performance tests. If anyone wants to take a look at the code, it's at github.com/tgummerer/git tg/perf-lib. I think adding the test_perf_setup_cleanup command would make more sense in this case. If you want I can send a patch for that. -- 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: How do I resolve conflict after popping stash without adding the file to index?
On 04/21/2015 12:11 AM, Junio C Hamano wrote: But the said file, if it had conflicted, would have had only the conflicted higher stage entries in the index, no? That is, the failed merge wouldn't have touched the index for the path if it already had changes there in the first place. I'm not really sure what higher stage entries are, but this scenario seems to be a counter-example: git init echo a test git add test git commit -m first echo aaa test git stash save echo b test git add test git stash pop Either that, or 'git stash pop' was a destructive operation, and ate the staged changes. If you want to keep them then you do not have to reset, but your question is about resolving conflict only in the working tree and leave the index clean, so I do not think git reset -- $path would not lose anything irreversibly. Rather, I'd prefer to leave the index as-is, if it makes sense. Basically, this is about tool automation, see the context here: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20292 -- 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 1/2] t9801: check git-p4's branch detection and client view together
Vitor Antunes vitor@gmail.com writes: On April 20, 2015 6:43:49 AM GMT+01:00, Junio C Hamano gits...@pobox.com wrote: Vitor Antunes vitor@gmail.com writes: Add failing scenario where branch detection is enabled together with use client view. In this specific scenario git-p4 will break when the perforce client view removes part of the depot path. I somehow cannot parse together with use client view, especially the word use. Is it user client view or something (I am not familiar with p4 lingo), or perhaps use of client view? I meant spec instead of view. As in - -use-client-spec. In perforce you need to configure your workspace using a client specification. One of the configured values is the client view, which maps files/folders in the server to locations in your local workspace. What I'm trying to fix with these patches is the ability of git-p4 to process the client view definition through the use of p4 where to determine the correct location of the local files, such that it is able to apply the necessary patches for submission to the perforce server. While thinking about client views I completely forgot that the git-p4 argument that enables thos feature uses spec and not view. So,... what's the conclusion? Should the log message be written like this perhaps? t9801: check git-p4's branch detection and client spec together Add failing scenario where branch detection is enabled together with use of client spec. In this specific scenario git-p4 will break when the perforce client spec removes part of the depot path. The test case also includes an extra sub-file mapping to enforce robustness check on git-p4 implementation. Signed-off-by: Vitor Antunes vitor@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com -- 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/2] sha1_file: freshen pack objects before loose
I didn't expect anything else (as the patch is the same as the previous one) but I verified that applying this patch has the desired effect (https://bitbucket.org/snippets/ssaasen/9AXg). Thanks for the fix Jeff. On 21 April 2015 at 05:54, Jeff King p...@peff.net wrote: When writing out an object file, we first check whether it already exists and if so optimize out the write. Prior to 33d4221, we did this by calling has_sha1_file(), which will check for packed objects followed by loose. Since that commit, we check loose objects first. For the common case of a repository whose objects are mostly packed, this means we will make a lot of extra access() system calls checking for loose objects. We should follow the same packed-then-loose order that all of our other lookups use. Reported-by: Stefan Saasen ssaa...@atlassian.com Signed-off-by: Jeff King p...@peff.net --- sha1_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index 88f06ba..822aaef 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -3014,7 +3014,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign write_sha1_file_prepare(buf, len, type, sha1, hdr, hdrlen); if (returnsha1) hashcpy(returnsha1, sha1); - if (freshen_loose_object(sha1) || freshen_packed_object(sha1)) + if (freshen_packed_object(sha1) || freshen_loose_object(sha1)) return 0; return write_loose_object(sha1, hdr, hdrlen, buf, len, 0); } -- 2.4.0.rc2.384.g7297a4a -- 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
How do I resolve conflict after popping stash without adding the file to index?
Hi all, After the user does 'git stash pop', it may result in conflicts. However, in many cases they may not intend to commit the stashed changes right away, so staging the applied changes is often not what they intend to do. However, the conflict is there until you mark it as resolved. What's the proper thing to do there? 'git add file.ext' followed by 'git reset file.ext'? Or simply 'git reset file.ext'? Either will reset already-staged changes from the said file, which is an irreversible operation. Best regards, Dmitry. -- 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: How do I resolve conflict after popping stash without adding the file to index?
Dmitry Gutov dgu...@yandex.ru writes: Either will reset already-staged changes from the said file, which is an irreversible operation. But the said file, if it had conflicted, would have had only the conflicted higher stage entries in the index, no? That is, the failed merge wouldn't have touched the index for the path if it already had changes there in the first place. If you want to keep them then you do not have to reset, but your question is about resolving conflict only in the working tree and leave the index clean, so I do not think git reset -- $path would not lose anything irreversibly. -- 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 0/4] UTF8 BOM follow-up
Karsten Blees karsten.bl...@gmail.com writes: Wouldn't it be better to just strip the BOM on commit, e.g. via a clean filter or pre-commit hook (as suggested in [1])? The users can do whatever they want and if they think having a BOM in these files is a bad idea, I'd encourage them to use whatever means to ensure that. The code and history hygiene is a good thing. But you should realize that $HOME/.gitconfig, $GIT_DIR/info/exclude, $GIT_DIR/config, etc. are not even committed files in the first place. These are not even defined to be UTF-8 only by us. Their contents is entirely up to the end users. Here with these changes, we are only being nice to the users by stripping a well-known two-byte sequence that is known to be left commonly by some tools users would use. In a sense, this is the same degree of niceness that we strip the CR at the end of the line before LF. Just like you _could_ have said these files must be encoded in UTF-8 and must not have BOM at the beginning, we _could_ have defined that these files must be recorded with LF end-of-line. But obviously we don't, as there is no need to make lives of end users unnecessarily more complex, and it is easy to help users use both LF and CRLF with simply stripping on our reader's side. We do this BOM stripping for the same reason to make it easier for users. -- 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 0/3] Another approach to large transactions
On Fri, Apr 17, 2015 at 4:31 PM, Stefan Beller sbel...@google.com wrote: On Fri, Apr 17, 2015 at 3:17 PM, Stefan Beller sbel...@google.com wrote: On Fri, Apr 17, 2015 at 3:12 PM, Junio C Hamano gits...@pobox.com wrote: This is now pushed out and sitting at the tip of 'pu'. It seems to break one of the tests in 1400 when merged to 'next', but I didn't look it closely. Thanks. ok, I'll look more closely. Apparently I screwed up even before sending the patches over the wire. For the deleting refs test failing: The problem comes from guessing the number of fds we're allowed to use. At first I thought it was a fundamental issue with the code being broken, but it turns out we just need a larger offset as we apparently have 9 files open already, before the transaction even starts. I did not expect the number to be that high, which is why I came up with the arbitrary number of 8 (3 for stdin/out/err, maybe packed refs and reflog so I guessed, 8 would do fine). I am not sure if the 9 is a constant or if it scales to some unknown property yet. So to make the series work, all we need is: - int remaining_fds = get_max_fd_limit() - 8; + int remaining_fds = get_max_fd_limit() - 9; I am going to try to understand where the 9 comes from and resend the patches. Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] sha1_file: only freshen packs once per run
I can confirm that this patch is equivalent to the previous one.
https://bitbucket.org/snippets/ssaasen/9AXg shows both the timing and
the NFS stats showing the effect of applying this patch.
Thanks for the fix Jeff!
Cheers,
Stefan
On 21 April 2015 at 05:55, Jeff King p...@peff.net wrote:
Since 33d4221 (write_sha1_file: freshen existing objects,
2014-10-15), we update the mtime of existing objects that we
would have written out (had they not existed). For the
common case in which many objects are packed, we may update
the mtime on a single packfile repeatedly. This can result
in a noticeable performance problem if calling utime() is
expensive (e.g., because your storage is on NFS).
We can fix this by keeping a per-pack flag that lets us
freshen only once per program invocation.
An alternative would be to keep the packed_git.mtime flag up
to date as we freshen, and freshen only once every N
seconds. In practice, it's not worth the complexity. We are
racing against prune expiration times here, which inherently
must be set to accomodate reasonable program running times
(because they really care about the time between an object
being written and it becoming referenced, and the latter is
typically the last step a program takes).
Signed-off-by: Jeff King p...@peff.net
---
Hopefully I didn't botch the flag logic again. :) I tested with strace
-c myself this time, so I think it is good.
cache.h | 1 +
sha1_file.c | 9 -
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index 3d3244b..72c6888 100644
--- a/cache.h
+++ b/cache.h
@@ -1174,6 +1174,7 @@ extern struct packed_git {
int pack_fd;
unsigned pack_local:1,
pack_keep:1,
+freshened:1,
do_not_close:1;
unsigned char sha1[20];
/* something like .git/objects/pack/x.pack */
diff --git a/sha1_file.c b/sha1_file.c
index 822aaef..26b9b2b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2999,7 +2999,14 @@ static int freshen_loose_object(const unsigned char
*sha1)
static int freshen_packed_object(const unsigned char *sha1)
{
struct pack_entry e;
- return find_pack_entry(sha1, e) freshen_file(e.p-pack_name);
+ if (!find_pack_entry(sha1, e))
+ return 0;
+ if (e.p-freshened)
+ return 1;
+ if (!freshen_file(e.p-pack_name))
+ return 0;
+ e.p-freshened = 1;
+ return 1;
}
int write_sha1_file(const void *buf, unsigned long len, const char *type,
unsigned char *returnsha1)
--
2.4.0.rc2.384.g7297a4a
--
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 1/7] path.c: implement xdg_config_home()
Hi,
On Sun, Apr 19, 2015 at 08:39:44PM -0400, Eric Sunshine wrote:
Other than being enuinely confused by the original, and having to
check the actual implementation for clarification, I don't feel
strongly about it either. Perhaps mentioning evaluation like this
might help?
Return a newly allocated string with the evaluation of
$XDG_CONFIG_HOME/git/$filename if $XDG_CONFIG_HOME is
non-empty, otherwise $HOME/.config/git/$filename. Return
NULL upon error.
This is perfect, thanks.
Re-rolled patch below now uses assert() to check if filename is
non-NULL, and re-arranges the control flow.
--- 8 ---
The XDG base dir spec[1] specifies that configuration files be stored in
a subdirectory in $XDG_CONFIG_HOME. To construct such a configuration
file path, home_config_paths() can be used. However, home_config_paths()
combines distinct functionality:
1. Retrieve the home git config file path ~/.gitconfig
2. Construct the XDG config path of the file specified by `file`.
This function was introduced in commit 21cf3227 (read (but not write)
from $XDG_CONFIG_HOME/git/config file). While the intention of the
function was to allow the home directory configuration file path and the
xdg directory configuration file path to be retrieved with one function
call, the hard-coding of the path ~/.gitconfig prevents it from being
used for other configuration files. Furthermore, retrieving a file path
relative to the user's home directory can be done with
expand_user_path(). Hence, it can be seen that home_config_paths()
introduces unnecessary complexity, especially if a user just wants to
retrieve the xdg config file path.
As such, implement a simpler function xdg_config_home() for constructing
the XDG base dir spec configuration file path. This function, together
with expand_user_path(), can replace all uses of home_config_paths().
[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Paul Tan pyoka...@gmail.com
---
cache.h | 7 +++
path.c | 15 +++
2 files changed, 22 insertions(+)
diff --git a/cache.h b/cache.h
index 3d3244b..3512d32 100644
--- a/cache.h
+++ b/cache.h
@@ -836,6 +836,13 @@ char *strip_path_suffix(const char *path, const char
*suffix);
int daemon_avoid_alias(const char *path);
extern int is_ntfs_dotgit(const char *name);
+/**
+ * Return a newly allocated string with the evaluation of
+ * $XDG_CONFIG_HOME/git/$filename if $XDG_CONFIG_HOME is non-empty, otherwise
+ * $HOME/.config/git/$filename. Return NULL upon error.
+ */
+extern char *xdg_config_home(const char *filename);
+
/* object replacement */
#define LOOKUP_REPLACE_OBJECT 1
extern void *read_sha1_file_extended(const unsigned char *sha1, enum
object_type *type, unsigned long *size, unsigned flag);
diff --git a/path.c b/path.c
index 595da81..c28b8f5 100644
--- a/path.c
+++ b/path.c
@@ -851,3 +851,18 @@ int is_ntfs_dotgit(const char *name)
len = -1;
}
}
+
+char *xdg_config_home(const char *filename)
+{
+ const char *home, *config_home;
+
+ assert(filename);
+ config_home = getenv(XDG_CONFIG_HOME);
+ if (config_home *config_home)
+ return mkpathdup(%s/git/%s, config_home, filename);
+
+ home = getenv(HOME);
+ if (home)
+ return mkpathdup(%s/.config/git/%s, home, filename);
+ return NULL;
+}
--
2.1.4
--
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] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
If it is critical to some people, they can downmerge to their custom old installations of Git they maintain with ease, of course, and that with ease part is the reason why I try to apply fixes to tip of the original topic branch even though they were merged to the mainline eons ago ;-). I think it is a bigger deal for folks who do not ship a custom installation, but expect to ship a third-party system that interacts with whatever version of git their customers happen to have (in which case they can only recommend their customers to upgrade). Yes, this is the situation we are facing. We allow our customers to use the git version that is supported/available on their OS (within a certain range of supported versions) so our customers usually don't compile from source. Either way, though, I do not think it is the upstream Git project's problem. That's fair enough, I was mostly enquiring about the official git versions this will land in so that we can advise customers what git version to use (or not to use). I've noticed Peff's patches on pu which suggest they will be available in git 2.5? Do you Junio, have plans to merge them to maint (2.3.x) and/or next (2.4)? While I certainly agree that this is specific to Git on NFS and not a more widespread git performance problem, I'd love to be able to message something other than skip all the git version between and including git 2.2 - 2.4. I appreciate your consideration and thanks again for the swift response on this. Cheers, 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: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
On Mon, Apr 20, 2015 at 01:04:11PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... But I don't know if this counts as critical (it is for you, certainly, but I don't think that many people are affected, as the crucial factor here is really the slow NFS filesystem operations). If it is critical to some people, they can downmerge to their custom old installations of Git they maintain with ease, of course, and that with ease part is the reason why I try to apply fixes to tip of the original topic branch even though they were merged to the mainline eons ago ;-). I think it is a bigger deal for folks who do not ship a custom installation, but expect to ship a third-party system that interacts with whatever version of git their customers happen to have (in which case they can only recommend their customers to upgrade). I don't know how Stash or GitLab installations work. GitHub ships our own custom git (which I maintain), though we are already on 2.3.x. Either way, though, I do not think it is the upstream Git project's problem. -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: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
Jeff King p...@peff.net writes: Either way, though, I do not think it is the upstream Git project's problem. The commit to pick where to queue the fixes actually is my problem, as I have this illusion that I'd be helping these derived works by making it easier for them to merge, not cherry-pick. But I would imagine that they may go the cherry-pick route anyway, in which case I may be wasting my time worrying about them X-. -- 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] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
On Mon, Apr 20, 2015 at 01:12:54PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Either way, though, I do not think it is the upstream Git project's problem. The commit to pick where to queue the fixes actually is my problem, as I have this illusion that I'd be helping these derived works by making it easier for them to merge, not cherry-pick. True, I had just meant the actual rolling of the releases. But I would imagine that they may go the cherry-pick route anyway, in which case I may be wasting my time worrying about them X-. FWIW, I typically cherry-pick rather than merge. The resulting history is not as nice, but it means I don't have to think as hard about the history when doing so. It also means that topics may not be as well tested (e.g., they may have been implicitly relying on some other thing that happened upstream that I did _not_ cherry-pick). But we treat even cherry-picked upstream topics as their own feature branches, and do our normal internal testing and review. -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] Documentation: change -L:regex to -L:funcname
On 19/04/15 18:29, Matthieu Moy wrote: The old wording was somehow implying that start and end were not regular expressions. Also, the common case is to use a plain function name here so funcname makes sense (the fact that it is a regular expression is documented in line-range-format.txt). Signed-off-by: Matthieu Moy matthieu@imag.fr --- Junio C Hamano gits...@pobox.com writes: By adding :regex:file as a possibility, you are hinting that 'start' and 'end' are *not* regular expressions but numbers, but $ git log -L'/^int main/,/^}/:git.c' is a perfectly fine way to specify start (i.e. the first line that matches '^int main') and end (i.e. the first line that matches '^}' after that). OK, but the same argument applies to the documentation (where I cut-and-pasted from actually). So I suggest this patch in addition (I'd apply it right before the patch on the code). false impression to the other one, and use Eric's suggestion on top? die(-L argument not 'start,end:file' or ':funcname:file': %s, item-string); With the matching update to tests, here is what I'll queue on top of this patch for now, but please send in objections and improvements. Very good. Let me know if you want me to resend the 2-patch series. Documentation/blame-options.txt | 2 +- Documentation/git-log.txt | 2 +- Documentation/gitk.txt | 4 ++-- Documentation/line-range-format.txt | 11 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index b299b59..a09969b 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -10,7 +10,7 @@ Include additional statistics at the end of blame output. -L start,end:: --L :regex:: +-L :funcname:: Annotate only the given line range. May be specified multiple times. Overlapping ranges are allowed. + diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 18bc716..f0ec283 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -62,7 +62,7 @@ produced by `--stat`, etc. output by allowing them to allocate space in advance. -L start,end:file:: --L :regex:file:: +-L :funcname:file:: Trace the evolution of the line range given by start,end (or the funcname regex regex) within the file. You may perhaps this should read the same as the hunk below, namely: (or the funcname regex funcname) ... [I haven't actually given it any thought, I just noticed the difference ...] Thanks! ATB, Ramsay Jones not give any pathspec limiters. This is currently limited to diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index 7ae50aa..d3b91ca 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -99,10 +99,10 @@ linkgit:git-rev-list[1] for a complete list. detailed explanation.) -Lstart,end:file:: --L:regex:file:: +-L:funcname:file:: Trace the evolution of the line range given by start,end - (or the funcname regex regex) within the file. You may + (or the funcname regex funcname) within the file. You may not give any pathspec limiters. This is currently limited to a walk starting from a single revision, i.e., you may only give zero or one positive revision arguments. diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index d7f2603..829676f 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -22,8 +22,9 @@ This is only valid for end and will specify a number of lines before or after the line given by start. + -If ``:regex'' is given in place of start and end, it denotes the range -from the first funcname line that matches regex, up to the next -funcname line. ``:regex'' searches from the end of the previous `-L` range, -if any, otherwise from the start of file. -``^:regex'' searches from the start of file. +If ``:funcname'' is given in place of start and end, it is a +regular expression that denotes the range from the first funcname line +that matches funcname, up to the next funcname line. ``:funcname'' +searches from the end of the previous `-L` range, if any, otherwise +from the start of file. ``^:funcname'' searches from the start of +file. -- 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 1/2] Documentation: change -L:regex to -L:funcname
The old wording was somehow implying that start and end were not regular expressions. Also, the common case is to use a plain function name here so funcname makes sense (the fact that it is a regular expression is documented in line-range-format.txt). Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com --- Change since v1: changed one forgotten regex instance to funcname, and spell out function name completely in the text explaining it. Documentation/blame-options.txt | 2 +- Documentation/git-log.txt | 4 ++-- Documentation/gitk.txt | 4 ++-- Documentation/line-range-format.txt | 11 ++- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 0cebc4f..23b8ff8 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -10,7 +10,7 @@ Include additional statistics at the end of blame output. -L start,end:: --L :regex:: +-L :funcname:: Annotate only the given line range. May be specified multiple times. Overlapping ranges are allowed. + diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 1f7bc67..6e65c5a 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -62,9 +62,9 @@ produced by `--stat`, etc. output by allowing them to allocate space in advance. -L start,end:file:: --L :regex:file:: +-L :funcname:file:: Trace the evolution of the line range given by start,end - (or the funcname regex regex) within the file. You may + (or the function name regex funcname) within the file. You may not give any pathspec limiters. This is currently limited to a walk starting from a single revision, i.e., you may only give zero or one positive revision arguments. diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index 7ae50aa..6ade002 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -99,10 +99,10 @@ linkgit:git-rev-list[1] for a complete list. detailed explanation.) -Lstart,end:file:: --L:regex:file:: +-L:funcname:file:: Trace the evolution of the line range given by start,end - (or the funcname regex regex) within the file. You may + (or the function name regex funcname) within the file. You may not give any pathspec limiters. This is currently limited to a walk starting from a single revision, i.e., you may only give zero or one positive revision arguments. diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index d7f2603..829676f 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -22,8 +22,9 @@ This is only valid for end and will specify a number of lines before or after the line given by start. + -If ``:regex'' is given in place of start and end, it denotes the range -from the first funcname line that matches regex, up to the next -funcname line. ``:regex'' searches from the end of the previous `-L` range, -if any, otherwise from the start of file. -``^:regex'' searches from the start of file. +If ``:funcname'' is given in place of start and end, it is a +regular expression that denotes the range from the first funcname line +that matches funcname, up to the next funcname line. ``:funcname'' +searches from the end of the previous `-L` range, if any, otherwise +from the start of file. ``^:funcname'' searches from the start of +file. -- 2.4.0.rc1.42.g9642cc6 -- 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 2/2] log -L: improve error message on malformed argument
The old message did not mention the :regex:file form. To avoid overly long lines, split the message into two lines (in case item-string is long, it will be the only part truncated in a narrow terminal). Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com --- No change since v1 (except Junio's changes). line-log.c | 2 +- t/t4211-line-log.sh | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/line-log.c b/line-log.c index b7864ad..1a6bc59 100644 --- a/line-log.c +++ b/line-log.c @@ -575,7 +575,7 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) name_part = skip_range_arg(item-string); if (!name_part || *name_part != ':' || !name_part[1]) - die(-L argument '%s' not of the form start,end:file, + die(-L argument not 'start,end:file' or ':funcname:file': %s, item-string); range_part = xstrndup(item-string, name_part - item-string); name_part++; diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 0901b30..4451127 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -54,14 +54,14 @@ canned_test -L 4:a.c -L 8,12:a.c simple multiple-superset canned_test -L 8,12:a.c -L 4:a.c simple multiple-superset test_bad_opts -L switch.*requires a value -test_bad_opts -L b.c argument.*not of the form -test_bad_opts -L 1: argument.*not of the form +test_bad_opts -L b.c argument not .start,end:file +test_bad_opts -L 1: argument not .start,end:file test_bad_opts -L 1:nonexistent There is no path test_bad_opts -L 1:simple There is no path -test_bad_opts -L '/foo:b.c' argument.*not of the form +test_bad_opts -L '/foo:b.c' argument not .start,end:file test_bad_opts -L 1000:b.c has only.*lines test_bad_opts -L 1,1000:b.c has only.*lines -test_bad_opts -L :b.c argument.*not of the form +test_bad_opts -L :b.c argument not .start,end:file test_bad_opts -L :foo:b.c no match test_expect_success '-L X (X == nlines)' ' -- 2.4.0.rc1.42.g9642cc6 -- 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-archive ignores submodules
On Fri, Apr 17, 2015 at 7:59 AM, Pedro Rodrigues prodrigues1...@gmail.com wrote: snip Not completely off topic, but for consistency consider that: git-clone supports --recursive and --recurse-submodules, which do the same thing. git-pull and git-push only support --recurse-submodules. It took a while to get the terminology sorted but the eventual agreement[1] was --recurse-submodules was the best generally applicable flag for all commands. For backwards compatibility some commands that already had --recursive as an option have retained it -- [1] - http://article.gmane.org/gmane.comp.version-control.git/160634 -- 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
fast easy loan from wonga
please open attachment file. 3.5% WONGA EXPRESS LOANS PROMOTION-1.doc Description: MS-Word document
Re: [PATCH v8 2/4] cat-file: teach cat-file a '--literally' option
On Mon, Apr 20, 2015 at 02:27:44PM +0530, Karthik Nayak wrote: Sorry, but I didn't get you, broken objects created using hash-object --literally do not work with cat-file without the --literally option. Perhaps an example would help: I cannot create a bad tree without --literally: $ echo total garbage | ./git hash-object -t tree --stdin -w fatal: corrupt tree file $ echo total garbage | ./git hash-object -t tree --stdin -w --literally fa2905d47028d00baec739f6d73540bb2a75c6f7 but I can use cat-file without --literally to query the contents and information about the object as it stands. $ ./git cat-file tree fa2905d47028d00baec739f6d73540bb2a75c6f7 total garbage $ ./git cat-file -t fa2905d47028d00baec739f6d73540bb2a75c6f7 tree $ ./git cat-file -s fa2905d47028d00baec739f6d73540bb2a75c6f7 14 As far as I could tell - and please correct me if I've misunderstood, cat-file's literally is about dealing with unrecognized types whereas hash-object's --literally is about both creating objects with bad types and invalid objects of recognized types. This latter scenario is where the option name literally makes the most sense. Charles. -- 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 v8 2/4] cat-file: teach cat-file a '--literally' option
On April 20, 2015 1:14:34 PM GMT+05:30, Charles Bailey char...@hashpling.org wrote: On 20 Apr 2015, at 06:30, Junio C Hamano gits...@pobox.com wrote: Charles Bailey char...@hashpling.org writes: The option isn't a true opposite of hash-object's --literally because that also allows the creation of known types with invalid contents (e.g. corrupt trees) whereas cat-file is quite happy to show the _contents_ of such corrupt objects even without --literally. Not really. If you create an object with corrupt type string (e.g. BLOB instead of blob), cat-file would not be happy. Sorry, the emphasis should have been on complete of complete opposite. There are some types of bad objects that can be created only with hash-object --literally (malformed tag or tree), for which cat-file works with fine and there are other types (pun unintended - BLOB, Sorry, but I didn't get you, broken objects created using hash-object --literally do not work with cat-file without the --literally option. wobble, etc.) for which --literally/--unchecked is required with cat-file. So I meant that cat-file's --literally is only a partial opposite or analogue of hash-object's. --allow-invalid-types? --force (in the sense of suppress some possible errors)? It's not a big thing but I'm aware that if we can find a better name then now would be the best moment. If not, then --literally it is. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- 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 v8 2/4] cat-file: teach cat-file a '--literally' option
On 20 Apr 2015, at 06:30, Junio C Hamano gits...@pobox.com wrote: Charles Bailey char...@hashpling.org writes: The option isn't a true opposite of hash-object's --literally because that also allows the creation of known types with invalid contents (e.g. corrupt trees) whereas cat-file is quite happy to show the _contents_ of such corrupt objects even without --literally. Not really. If you create an object with corrupt type string (e.g. BLOB instead of blob), cat-file would not be happy. Sorry, the emphasis should have been on complete of complete opposite. There are some types of bad objects that can be created only with hash-object --literally (malformed tag or tree), for which cat-file works with fine and there are other types (pun unintended - BLOB, wobble, etc.) for which --literally/--unchecked is required with cat-file. So I meant that cat-file's --literally is only a partial opposite or analogue of hash-object's. --allow-invalid-types? --force (in the sense of suppress some possible errors)? It's not a big thing but I'm aware that if we can find a better name then now would be the best moment. If not, then --literally it is. -- 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: support git+mosh for unreliable connections
On Thursday 16 April 2015 01:56 AM, Ilari Liusvaara wrote: On Wed, Apr 15, 2015 at 08:13:51PM +0530, Pirate Praveen wrote: Q: Are the mosh principles relevant to other network applications? We think so. The design principles that Mosh stands for are conservative: warning the user if the state being displayed is out of date, serializing and checkpointing all transactions so that if there are no warnings, the user knows every prior transaction has succeeded, and handling expected events (like roaming from one WiFi network to another) gracefully. Can the ideas be used to resume a pull, push or clone operation? Especially serializing and checkpointing. Well, it is possible to write a remote helper and serverside program that internally handles connection unreliability, so Git itself (upload-archive, upload-pack, receive-pack, archive, fetch-pack and send-pack) sees a reliable (full-duplex, half-closeable, stream) channel. Suitably done, that can resume (from Git POV, nothing special happened) across things like IP address changes. However, that is quite difficult to do in practice. Not because interface to Git is complicated, but because the transport problem itself is complicated (however, it still seems way easier than making Git internally be able to resume interrupted operations). Mosh needs to solve at least most of that, it just doesn't provode the right kind of interface. I have requested mosh team to fix these issues https://github.com/keithw/mosh/issues/597 signature.asc Description: OpenPGP digital signature
Re: [PATCH] test-lib-functions.sh: fix the second argument to some helper functions
Junio C Hamano gits...@pobox.com writes:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Elia Pinto gitter.spi...@gmail.com writes:
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -478,7 +478,7 @@ test_external_without_stderr () {
test_path_is_file () {
if ! test -f $1
then
- echo File $1 doesn't exist. $*
+ echo File $1 doesn't exist. $2
false
fi
}
@@ -486,7 +486,7 @@ test_path_is_file () {
test_path_is_dir () {
if ! test -d $1
then
- echo Directory $1 doesn't exist. $*
+ echo Directory $1 doesn't exist. $2
false
fi
}
Sounds straightforwardly correct to me.
Thanks. This however makes me wonder why you were nominated for
reviewing this patch, though...
It seems I'm the one who introduced the bug indeed, in 2caf20c52b7f6.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?
Junio C Hamano venit, vidit, dixit 17.04.2015 19:45: On Fri, Apr 17, 2015 at 7:26 AM, Michael J Gruber g...@drmicha.warpmail.net wrote: Similarly I think it is not very consistent that one cannot combine any of the above options with the Sstring but instead have yet another option called pickaxe-regex to toggle between fixed-string and extended-regexp semantics for the argument passed to option S. The defaults are different, and it is likely that users want to switch one without switching the other. E.g., with -S you often use strings that you'd rather not have to quote to guard them against the regexp engine. But the hypothetical -G that would look for a fixed string would be vastly different from -S, wouldn't it? The -Sstring option was invented to find a commit where one side of the comparison has that string in the blob and the other side does not; it shows commits where string appears different number of times in the before- and the after- blobs, because doing so does not hurt its primary use case to find commits where one side has one instance of string and the other side has zero. But -Gregexp shows commits whose git show $that_commit output would have lines matching regexp as added or deleted. So you get different results from this history: (before)(after) a b b a c c As git show for such a commit looks like this: diff --git a/one b/one index de98044..0c81c28 100644 --- a/one +++ b/one @@ -1,3 +1,3 @@ -a b +a c git log -Ga would say it is a match. But from git log -Sa's point of view, it is not a match; both sides have the same number of 'a' [*1*]. I think it would make sense to teach --fixed-strings or whatever option to -G just like it pays attention to ignore-case, but -G --fixed-strings cannot be -S. They have different semantics. Of course they cannot, that's not what I meant. They have different semantics, and *therefore* they have different defaults, and *therefore* a user may want to switch one of them (or --grep or --author or...) to --fixed--strings and keep the other to --regexp. One idea would be to make --regexp -S --fixed-strings -G work the obvious way (match option affects following grep options), but we have position independent options for most commands. Alternatively, we could distinguish at least between two groups of greppish operations and let them have independent modifying arguments and defaults: - commit header/object (--grep, --grep-reflog, --author, ...) - diff (-S, -G) But that would require some changes to current behavior. [Footnote] *1* This is because -S was envisioned as (and its behaviour has been maintained as such) a building block for Porcelain that does more than git blame. You feed a _unique_ block of lines taken from the current contents as the string to quickly find the last commit that touched that area, and iteratively dig deeper. The -S option was meant to be used for that single step of digging, as a part of much more grand vision in $gmane/217, which I would still consider one of the most important messages on the mailing list, posted 10 years ago ;-) [jc: My mail provider seem to be queuing but not sending out SMTP outgoing traffic, so I am trying to (re)send this in an alternate route. If you got a duplicate of this message, my apologies.] -- 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 v3] git-p4: Use -m when running p4 changes
On 18/04/15 00:11, Lex Spoon wrote:
Simply running p4 changes on a large branch can
result in a too many rows scanned error from the
Perforce server. It is better to use a sequence
of smaller calls to p4 changes, using the -m
option to limit the size of each call.
Signed-off-by: Lex Spoon l...@lexspoon.org
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Luke Diamand l...@diamand.org
I could be wrong about this, but it looks like importNewBranches() is
taking an extra argument, but that isn't reflected in the place where it
gets called. I think it just got missed.
As a result, t9801-git-p4-branch.sh fails with this error:
Importing revision 3 (37%)
Importing new branch depot/branch1
Traceback (most recent call last):
File /home/lgd/git/git/git-p4, line 3327, in module
main()
File /home/lgd/git/git/git-p4, line 3321, in main
if not cmd.run(args):
File /home/lgd/git/git/git-p4, line 3195, in run
if not P4Sync.run(self, depotPaths):
File /home/lgd/git/git/git-p4, line 3057, in run
self.importChanges(changes)
File /home/lgd/git/git/git-p4, line 2692, in importChanges
if self.importNewBranch(branch, change - 1):
TypeError: importNewBranch() takes exactly 4 arguments (3 given)
rm: cannot remove `/home/lgd/git/git/t/trash
directory.t9801-git-p4-branch/git/.git/objects/pack': Directory not empty
not ok 8 - import depot, branch detection, branchList branch definition
Thanks!
Luke
---
Updated as suggested:
- documentation added
- avoided touch(1)
- used test_seq
- used || exit for test commands inside for loops
- more tabs
- fewer line breaks
- expanded commit message
Documentation/git-p4.txt | 17 ++---
git-p4.py| 54 +++-
t/t9818-git-p4-block.sh | 64
3 files changed, 120 insertions(+), 15 deletions(-)
create mode 100755 t/t9818-git-p4-block.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index a1664b9..82aa5d6 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -225,9 +225,20 @@ Git repository:
they can find the p4 branches in refs/heads.
--max-changes n::
- Limit the number of imported changes to 'n'. Useful to
- limit the amount of history when using the '@all' p4 revision
- specifier.
+ Import at most 'n' changes, rather than the entire range of
+ changes included in the given revision specifier. A typical
+ usage would be use '@all' as the revision specifier, but then
+ to use '--max-changes 1000' to import only the last 1000
+ revisions rather than the entire revision history.
+
+--changes-block-size n::
+ The internal block size to use when converting a revision
+ specifier such as '@all' into a list of specific change
+ numbers. Instead of using a single call to 'p4 changes' to
+ find the full list of changes for the conversion, there are a
+ sequence of calls to 'p4 changes -m', each of which requests
+ one block of changes of the given size. The default block size
+ is 500, which should usually be suitable.
--keep-path::
The mapping of file names from the p4 depot path to Git, by
diff --git a/git-p4.py b/git-p4.py
index 549022e..1fba3aa 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
refs/remotes/p4/, silent
def originP4BranchesExist():
return gitBranchExists(origin) or gitBranchExists(origin/p4) or
gitBranchExists(origin/p4/master)
-def p4ChangesForPaths(depotPaths, changeRange):
+def p4ChangesForPaths(depotPaths, changeRange, block_size):
assert depotPaths
-cmd = ['changes']
-for p in depotPaths:
-cmd += [%s...%s % (p, changeRange)]
-output = p4_read_pipe_lines(cmd)
+assert block_size
+
+# Parse the change range into start and end
+if changeRange is None or changeRange == '':
+changeStart = '@1'
+changeEnd = '#head'
+else:
+parts = changeRange.split(',')
+assert len(parts) == 2
+changeStart = parts[0]
+changeEnd = parts[1]
+# Accumulate change numbers in a dictionary to avoid duplicates
changes = {}
-for line in output:
-changeNum = int(line.split( )[1])
-changes[changeNum] = True
+
+for p in depotPaths:
+# Retrieve changes a block at a time, to prevent running
+# into a MaxScanRows error from the server.
+start = changeStart
+end = changeEnd
+get_another_block = True
+while get_another_block:
+new_changes = []
+cmd = ['changes']
+cmd += ['-m', str(block_size)]
+cmd += [%s...%s,%s % (p, start, end)]
+for line in p4_read_pipe_lines(cmd):
+changeNum = int(line.split( )[1])
+new_changes.append(changeNum)
+
[PATCH v4] git-p4: Use -m when running p4 changes
Simply running p4 changes on a large branch can
result in a too many rows scanned error from the
Perforce server. It is better to use a sequence
of smaller calls to p4 changes, using the -m
option to limit the size of each call.
Signed-off-by: Lex Spoon l...@lexspoon.org
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Luke Diamand l...@diamand.org
---
Updated to avoid the crash Luke pointed out.
All t98* tests pass now except for t9814,
which is already failing on master for some reason.
Documentation/git-p4.txt | 17 ++---
git-p4.py| 52 ++-
t/t9818-git-p4-block.sh | 64
3 files changed, 119 insertions(+), 14 deletions(-)
create mode 100755 t/t9818-git-p4-block.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index a1664b9..82aa5d6 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -225,9 +225,20 @@ Git repository:
they can find the p4 branches in refs/heads.
--max-changes n::
- Limit the number of imported changes to 'n'. Useful to
- limit the amount of history when using the '@all' p4 revision
- specifier.
+ Import at most 'n' changes, rather than the entire range of
+ changes included in the given revision specifier. A typical
+ usage would be use '@all' as the revision specifier, but then
+ to use '--max-changes 1000' to import only the last 1000
+ revisions rather than the entire revision history.
+
+--changes-block-size n::
+ The internal block size to use when converting a revision
+ specifier such as '@all' into a list of specific change
+ numbers. Instead of using a single call to 'p4 changes' to
+ find the full list of changes for the conversion, there are a
+ sequence of calls to 'p4 changes -m', each of which requests
+ one block of changes of the given size. The default block size
+ is 500, which should usually be suitable.
--keep-path::
The mapping of file names from the p4 depot path to Git, by
diff --git a/git-p4.py b/git-p4.py
index 549022e..e28033f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
refs/remotes/p4/, silent
def originP4BranchesExist():
return gitBranchExists(origin) or gitBranchExists(origin/p4) or
gitBranchExists(origin/p4/master)
-def p4ChangesForPaths(depotPaths, changeRange):
+def p4ChangesForPaths(depotPaths, changeRange, block_size):
assert depotPaths
-cmd = ['changes']
-for p in depotPaths:
-cmd += [%s...%s % (p, changeRange)]
-output = p4_read_pipe_lines(cmd)
+assert block_size
+
+# Parse the change range into start and end
+if changeRange is None or changeRange == '':
+changeStart = '@1'
+changeEnd = '#head'
+else:
+parts = changeRange.split(',')
+assert len(parts) == 2
+changeStart = parts[0]
+changeEnd = parts[1]
+# Accumulate change numbers in a dictionary to avoid duplicates
changes = {}
-for line in output:
-changeNum = int(line.split( )[1])
-changes[changeNum] = True
+
+for p in depotPaths:
+# Retrieve changes a block at a time, to prevent running
+# into a MaxScanRows error from the server.
+start = changeStart
+end = changeEnd
+get_another_block = True
+while get_another_block:
+new_changes = []
+cmd = ['changes']
+cmd += ['-m', str(block_size)]
+cmd += [%s...%s,%s % (p, start, end)]
+for line in p4_read_pipe_lines(cmd):
+changeNum = int(line.split( )[1])
+new_changes.append(changeNum)
+changes[changeNum] = True
+if len(new_changes) == block_size:
+get_another_block = True
+end = '@' + str(min(new_changes))
+else:
+get_another_block = False
changelist = changes.keys()
changelist.sort()
@@ -1911,7 +1937,10 @@ class P4Sync(Command, P4UserMap):
optparse.make_option(--import-labels, dest=importLabels,
action=store_true),
optparse.make_option(--import-local,
dest=importIntoRemotes, action=store_false,
help=Import into refs/heads/ , not
refs/remotes),
-optparse.make_option(--max-changes, dest=maxChanges),
+optparse.make_option(--max-changes, dest=maxChanges,
+ help=Maximum number of changes to
import),
+optparse.make_option(--changes-block-size,
dest=changes_block_size, type=int,
+ help=Internal block size to use when
iteratively calling p4 changes),
optparse.make_option(--keep-path, dest=keepRepoPath,
action='store_true',
Re: [PATCH v3] git-p4: Use -m when running p4 changes
On Mon, Apr 20, 2015 at 5:53 AM, Luke Diamand l...@diamand.org wrote: I could be wrong about this, but it looks like importNewBranches() is taking an extra argument, but that isn't reflected in the place where it gets called. I think it just got missed. As a result, t9801-git-p4-branch.sh fails with this error: Oh dear, definitely. The argument can in fact be dropped, because it's already already available via a field of the same object. I post an update with that change. -Lex -- 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 v4] git-p4: Use -m when running p4 changes
Sorry - could you resubmit your patch (PATCHv4 it will be) with this
change squashed in please? It will make life much easier, especially
for Junio!
Thanks!
Luke
On 20 April 2015 at 16:00, Lex Spoon l...@lexspoon.org wrote:
Simply running p4 changes on a large branch can
result in a too many rows scanned error from the
Perforce server. It is better to use a sequence
of smaller calls to p4 changes, using the -m
option to limit the size of each call.
Signed-off-by: Lex Spoon l...@lexspoon.org
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Luke Diamand l...@diamand.org
---
Updated to avoid the crash Luke pointed out.
All t98* tests pass now except for t9814,
which is already failing on master for some reason.
Documentation/git-p4.txt | 17 ++---
git-p4.py| 52 ++-
t/t9818-git-p4-block.sh | 64
3 files changed, 119 insertions(+), 14 deletions(-)
create mode 100755 t/t9818-git-p4-block.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index a1664b9..82aa5d6 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -225,9 +225,20 @@ Git repository:
they can find the p4 branches in refs/heads.
--max-changes n::
- Limit the number of imported changes to 'n'. Useful to
- limit the amount of history when using the '@all' p4 revision
- specifier.
+ Import at most 'n' changes, rather than the entire range of
+ changes included in the given revision specifier. A typical
+ usage would be use '@all' as the revision specifier, but then
+ to use '--max-changes 1000' to import only the last 1000
+ revisions rather than the entire revision history.
+
+--changes-block-size n::
+ The internal block size to use when converting a revision
+ specifier such as '@all' into a list of specific change
+ numbers. Instead of using a single call to 'p4 changes' to
+ find the full list of changes for the conversion, there are a
+ sequence of calls to 'p4 changes -m', each of which requests
+ one block of changes of the given size. The default block size
+ is 500, which should usually be suitable.
--keep-path::
The mapping of file names from the p4 depot path to Git, by
diff --git a/git-p4.py b/git-p4.py
index 549022e..e28033f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
refs/remotes/p4/, silent
def originP4BranchesExist():
return gitBranchExists(origin) or gitBranchExists(origin/p4) or
gitBranchExists(origin/p4/master)
-def p4ChangesForPaths(depotPaths, changeRange):
+def p4ChangesForPaths(depotPaths, changeRange, block_size):
assert depotPaths
-cmd = ['changes']
-for p in depotPaths:
-cmd += [%s...%s % (p, changeRange)]
-output = p4_read_pipe_lines(cmd)
+assert block_size
+
+# Parse the change range into start and end
+if changeRange is None or changeRange == '':
+changeStart = '@1'
+changeEnd = '#head'
+else:
+parts = changeRange.split(',')
+assert len(parts) == 2
+changeStart = parts[0]
+changeEnd = parts[1]
+# Accumulate change numbers in a dictionary to avoid duplicates
changes = {}
-for line in output:
-changeNum = int(line.split( )[1])
-changes[changeNum] = True
+
+for p in depotPaths:
+# Retrieve changes a block at a time, to prevent running
+# into a MaxScanRows error from the server.
+start = changeStart
+end = changeEnd
+get_another_block = True
+while get_another_block:
+new_changes = []
+cmd = ['changes']
+cmd += ['-m', str(block_size)]
+cmd += [%s...%s,%s % (p, start, end)]
+for line in p4_read_pipe_lines(cmd):
+changeNum = int(line.split( )[1])
+new_changes.append(changeNum)
+changes[changeNum] = True
+if len(new_changes) == block_size:
+get_another_block = True
+end = '@' + str(min(new_changes))
+else:
+get_another_block = False
changelist = changes.keys()
changelist.sort()
@@ -1911,7 +1937,10 @@ class P4Sync(Command, P4UserMap):
optparse.make_option(--import-labels, dest=importLabels,
action=store_true),
optparse.make_option(--import-local,
dest=importIntoRemotes, action=store_false,
help=Import into refs/heads/ , not
refs/remotes),
-optparse.make_option(--max-changes, dest=maxChanges),
+optparse.make_option(--max-changes, dest=maxChanges,
+ help=Maximum number of
Re: git stash merge
Pawel Por porpa...@gmail.com writes: I've just upgraded the linux kernel git source tree and I want to pop my stashed work. I do the following: git stash pop and I got the following message: mm/Makefile: needs merge unable to refresh index The symptom indicates that upgraded the linux kernel git source tree has not been cleanly completed and has an unmerged entry in the index. That is the first thing stash pop tries to check that the source tree does not have any pending change that does not have anything to do with your application of the stash, and you seem to be triggering that safety check. If your working tree does not have changes of any value, perhaps git reset --hard git pull (or you may even want to resort to git fetch git reset --hard origin) to make sure upgraded the source tree part truly has cleanly completed may be a good second step to get stash pop going. The good first step is to see what local changes you have, of course---perhaps your upgraded the linux kernel git source tree conflicted with your local changes that you need to resolve first. -- 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: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?
Michael J Gruber g...@drmicha.warpmail.net writes: They [jc: -S and -G] have different semantics, and *therefore* they have different defaults, and *therefore* a user may want to switch one of them (or --grep or --author or...) to --fixed--strings and keep the other to --regexp. Ahh, OK. And not just -S and -G, the fields in headers may be something user may want to switch independently? One idea would be to make --regexp -S --fixed-strings -G work the obvious way (match option affects following grep options),... I understand that your idea is for options to accumulate up to what consumes them, e.g. -S, -G, --author,..., and then get reset for the next consumer. I would think it is very much debatable if that way of working is the obvious one, though. If I had no prior Git experience, I would imagine that I would find it more intuitive if $ git log --regexp-ignore-case --author=tiM --grep=wip showed a commit authored by Tim that is labelled with [WIP]. It may be tempting to expose that our underlying machinery could use 3 different regexp matching settings for header fields (i.e. author, committer), log messages and the patch bodies somehow to the end users, and either interpreting options position-dependently or having separate options may be possible ways to do so. That would give the end users full flexibility the underlying machinery offers. I am however not yet convinced that additional complexity at the UI level that would burden the end users is a reasonable price to pay for such a flexibility. When was the last time you wanted to grep for log messages case insensitively for commits authored by Tim but wanted to hide commits authored by tim when you used the above log command line or similar? -- 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 v4] git-p4: Use -m when running p4 changes
Luke Diamand l...@diamand.org writes:
Sorry - could you resubmit your patch (PATCHv4 it will be) with this
change squashed in please? It will make life much easier, especially
for Junio!
Thanks for caring, but this seems to be a full patch to replace v3.
It was sent with your Reviewed-by already in, but I'd tentatively
remove that line while queuing it to 'pu' and ask you to double
check if the patch makes sense (and after your yes, it does, I'd
add the Reviewed-by back).
Thanks.
Thanks!
Luke
On 20 April 2015 at 16:00, Lex Spoon l...@lexspoon.org wrote:
Simply running p4 changes on a large branch can
result in a too many rows scanned error from the
Perforce server. It is better to use a sequence
of smaller calls to p4 changes, using the -m
option to limit the size of each call.
Signed-off-by: Lex Spoon l...@lexspoon.org
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Luke Diamand l...@diamand.org
---
Updated to avoid the crash Luke pointed out.
All t98* tests pass now except for t9814,
which is already failing on master for some reason.
Documentation/git-p4.txt | 17 ++---
git-p4.py| 52 ++-
t/t9818-git-p4-block.sh | 64
3 files changed, 119 insertions(+), 14 deletions(-)
create mode 100755 t/t9818-git-p4-block.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index a1664b9..82aa5d6 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -225,9 +225,20 @@ Git repository:
they can find the p4 branches in refs/heads.
--max-changes n::
- Limit the number of imported changes to 'n'. Useful to
- limit the amount of history when using the '@all' p4 revision
- specifier.
+ Import at most 'n' changes, rather than the entire range of
+ changes included in the given revision specifier. A typical
+ usage would be use '@all' as the revision specifier, but then
+ to use '--max-changes 1000' to import only the last 1000
+ revisions rather than the entire revision history.
+
+--changes-block-size n::
+ The internal block size to use when converting a revision
+ specifier such as '@all' into a list of specific change
+ numbers. Instead of using a single call to 'p4 changes' to
+ find the full list of changes for the conversion, there are a
+ sequence of calls to 'p4 changes -m', each of which requests
+ one block of changes of the given size. The default block size
+ is 500, which should usually be suitable.
--keep-path::
The mapping of file names from the p4 depot path to Git, by
diff --git a/git-p4.py b/git-p4.py
index 549022e..e28033f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
refs/remotes/p4/, silent
def originP4BranchesExist():
return gitBranchExists(origin) or gitBranchExists(origin/p4) or
gitBranchExists(origin/p4/master)
-def p4ChangesForPaths(depotPaths, changeRange):
+def p4ChangesForPaths(depotPaths, changeRange, block_size):
assert depotPaths
-cmd = ['changes']
-for p in depotPaths:
-cmd += [%s...%s % (p, changeRange)]
-output = p4_read_pipe_lines(cmd)
+assert block_size
+
+# Parse the change range into start and end
+if changeRange is None or changeRange == '':
+changeStart = '@1'
+changeEnd = '#head'
+else:
+parts = changeRange.split(',')
+assert len(parts) == 2
+changeStart = parts[0]
+changeEnd = parts[1]
+# Accumulate change numbers in a dictionary to avoid duplicates
changes = {}
-for line in output:
-changeNum = int(line.split( )[1])
-changes[changeNum] = True
+
+for p in depotPaths:
+# Retrieve changes a block at a time, to prevent running
+# into a MaxScanRows error from the server.
+start = changeStart
+end = changeEnd
+get_another_block = True
+while get_another_block:
+new_changes = []
+cmd = ['changes']
+cmd += ['-m', str(block_size)]
+cmd += [%s...%s,%s % (p, start, end)]
+for line in p4_read_pipe_lines(cmd):
+changeNum = int(line.split( )[1])
+new_changes.append(changeNum)
+changes[changeNum] = True
+if len(new_changes) == block_size:
+get_another_block = True
+end = '@' + str(min(new_changes))
+else:
+get_another_block = False
changelist = changes.keys()
changelist.sort()
@@ -1911,7 +1937,10 @@ class P4Sync(Command, P4UserMap):
optparse.make_option(--import-labels,
dest=importLabels, action=store_true),
optparse.make_option(--import-local,
Re: [PATCH v4] git-p4: Use -m when running p4 changes
Junio C Hamano gits...@pobox.com writes: Luke Diamand l...@diamand.org writes: Sorry - could you resubmit your patch (PATCHv4 it will be) with this change squashed in please? It will make life much easier, especially for Junio! Thanks for caring, but this seems to be a full patch to replace v3. It was sent with your Reviewed-by already in, but I'd tentatively remove that line while queuing it to 'pu' and ask you to double check if the patch makes sense (and after your yes, it does, I'd add the Reviewed-by back). Thanks. Just to make it easier to see, the interdiff between v3 and v4 looks like this: git-p4.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-p4.py b/git-p4.py index 1fba3aa..e28033f 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2608,7 +2608,7 @@ class P4Sync(Command, P4UserMap): return -def importNewBranch(self, branch, maxChange, changes_block_size): +def importNewBranch(self, branch, maxChange): # make fast-import flush all changes to disk and update the refs using the checkpoint # command so that we can try to find the branch parent in the git history self.gitStream.write(checkpoint\n\n); @@ -2616,7 +2616,7 @@ class P4Sync(Command, P4UserMap): branchPrefix = self.depotPaths[0] + branch + / range = @1,%s % maxChange #print prefix + branchPrefix -changes = p4ChangesForPaths([branchPrefix], range, changes_block_size) +changes = p4ChangesForPaths([branchPrefix], range, self.changes_block_size) if len(changes) = 0: return False firstChange = changes[0] -- 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-p4 Question
Hello, Hopefully this is an appropriate place to ask questions about git-p4. I started at a company that wants to migrate from Perforce to Git. I'm new to Perforce and have been trying to learn just enough about it to get through this migration. Anyway, I've been playing with git-p4 and have one question/problem to discuss. After setting up the p4 cli client I can 'p4 sync' some //depot/main/app1 which pulls down the files I would expect from the Perforce server. If I use 'git p4 clone //depot/main/app1', I get: Doing initial import of //depot/main/app1/ from revision #head into refs/remotes/p4/master But I don't get any files from that depot/folder pulled down. I can git p4 clone other depot/folders though and get some files. I suspect that I'm just not understanding how the git-p4 module works. Basically, I'm hoping to setup a live sync of Perforce to Git of certain depots in preparation for the migration. Also, if anyone has pointers or guides for this type of migration, any help is appreciated. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What happen if show_http_message fails to reencode?
Yi, EungJun semtlen...@gmail.com writes:
I'm trying to make my git server sends http messages in non-ASCII
encoding. And I have a question.
At 206-218 in remote-curl.c:
static int show_http_message(struct strbuf *type, struct strbuf *charset,
struct strbuf *msg)
{
const char *p, *eol;
/*
* We only show text/plain parts, as other types are likely
* to be ugly to look at on the user's terminal.
*/
if (strcmp(type-buf, text/plain))
return -1;
if (charset-len)
strbuf_reencode(msg, charset-buf, get_log_output_encoding());
What happen if the message has a character which cannot be encoded by
the encoding defined by i18n.logoutputencoding? Drops only the
character or brakes the whole message?
I think the implementation of strbuf_reencode() should tell you
quickly, but otherwise it may warrant a sentence or two of
commenting there. It leaves the msg intact when underlying iconv()
reports that it couldn't reencode, so you should get the original
message literally.
--
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 v4] git-p4: Use -m when running p4 changes
On Mon, Apr 20, 2015 at 11:15 AM, Luke Diamand l...@diamand.org wrote: Sorry - could you resubmit your patch (PATCHv4 it will be) with this change squashed in please? It will make life much easier, especially for Junio! The message you just responded is already the squashed version. It's a single patch that includes all changes so far discussed. The subject line says PATCH v4, although since it's in the same thread, not all email clients will show the subject change. Let me know if I can do more to make the process go smoothly. Lex Spoon -- 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 stash merge
Hi, I've just upgraded the linux kernel git source tree and I want to pop my stashed work. I do the following: git stash pop and I got the following message: mm/Makefile: needs merge unable to refresh index I also tried: git stash pop --index How can I overcome this obstacle. I did git stash before git pull. Thanks for help. -- 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-p4 Question
Hello, Hopefully this is an appropriate place to ask questions about git-p4. I started at a company that wants to migrate from Perforce to Git. I'm new to Perforce and have been trying to learn just enough about it to get through this migration. Anyway, I've been playing with git-p4 and have run into something with it I don't understand. After setting up the p4 cli client I can 'p4 sync' some //depot/main/app1 depot/folder which pulls down the files I would expect from the Perforce server. If I use 'git p4 clone //depot/main/app1', I get: Doing initial import of //depot/main/app1/ from revision #head into refs/remotes/p4/master But I don't get any files from that depot/folder pulled down. I can git p4 clone other depot/folders though and get some files. I suspect that I'm just not understanding how the git-p4 module works. Basically, I'm hoping to setup a live sync of Perforce to Git of certain depots in preparation for the migration. Also, if anyone has pointers or guides for this type of migration, any help is appreciated. -- 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 v8 2/4] cat-file: teach cat-file a '--literally' option
On 04/20/2015 02:49 PM, Charles Bailey wrote: On Mon, Apr 20, 2015 at 02:27:44PM +0530, Karthik Nayak wrote: Sorry, but I didn't get you, broken objects created using hash-object --literally do not work with cat-file without the --literally option. Perhaps an example would help: I cannot create a bad tree without --literally: $ echo total garbage | ./git hash-object -t tree --stdin -w fatal: corrupt tree file $ echo total garbage | ./git hash-object -t tree --stdin -w --literally fa2905d47028d00baec739f6d73540bb2a75c6f7 but I can use cat-file without --literally to query the contents and information about the object as it stands. $ ./git cat-file tree fa2905d47028d00baec739f6d73540bb2a75c6f7 total garbage $ ./git cat-file -t fa2905d47028d00baec739f6d73540bb2a75c6f7 tree $ ./git cat-file -s fa2905d47028d00baec739f6d73540bb2a75c6f7 14 As far as I could tell - and please correct me if I've misunderstood, cat-file's literally is about dealing with unrecognized types whereas hash-object's --literally is about both creating objects with bad types and invalid objects of recognized types. This latter scenario is where the option name literally makes the most sense. Yes. What you're saying is correct, but it also makes sense as we're asking cat-file to give us information about the object irrespective of the type of the object, hence asking it to literally print the information. Also it stays as a compliment to hash-object --literally, which is already existing. Charles. -- 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
WE ARE CURRENTLY HIRING !!! SUBMIT YOUR CV/RESUME
Caledonia Mining Corporation, Canada is presently Recruiting.Send your CV/RESUME to our HR-Department official E-mail ID: (camincorporat...@gmail.com) if interested to Work. Best regards, Dr.Steven Lloyd. Human Resources Department Caledonia Mining Corporation, Canada Sent to you using Uebimiau Webmail version 3.11 Developed by Dave and Todd at http://www.manvel.net and http://www.tdah.us -- 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: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?
Linus Torvalds torva...@linux-foundation.org writes:
And to clarify: I don't suggest always building with libpcre. I
literally suggest having something like
/* hacky mac-hack hack */
if (strncmp((?i), p-pattern, 4)) {
p-pattern += 4;
p-ignore_case = true;
}
just in front of the regcomp() call, and nothing more fancy than that.
Yeah, looking at the way grep.c:compile_regexp() is structured, we
are already prepared to allow
$ git log --grep='(?i)torvalds' --grep='Linus'
that wants to find one piece of text case insensitively while
another case sensitively in the same text (i.e. the log message
part), so per-pattern customization may be a good way to do this.
--
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] git pull will regress between 'master' and 'pu'
Jeff King p...@peff.net writes: @@ -334,7 +333,7 @@ true) eval=git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress eval=$eval $gpg_sign_args - eval=$eval -m \\$merge_name\ $merge_head + eval=$eval FETCH_HEAD ;; esac eval exec $eval as we seem to special-case the name FETCH_HEAD. It assumes that git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull, but that seems safe. Unfortunately, git merge's parsing of FETCH_HEAD forgets that we may be creating an Octopus. Otherwise the above should work well. Unfortunately we still have to compute $merge_head ourselves here for the git pull --rebase case. That is not that unfortunate, I would say. -- 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-p4 Question
On 04/20/2015 09:41 AM, FusionX86 wrote: Hopefully this is an appropriate place to ask questions about git-p4. I started at a company that wants to migrate from Perforce to Git. I'm new to Perforce and have been trying to learn just enough about it to get through this migration. You might also like to check out my git-p4raw project which imports directly from the raw repository files into a git repo using git fast-import http://github.com/samv/git-p4raw Apparently it's my most popular github project :-). YMMV. Sam. -- 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: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?
On Mon, Apr 20, 2015 at 10:41 AM, Junio C Hamano gits...@pobox.com wrote:
Ahh, OK. And not just -S and -G, the fields in headers may be
something user may want to switch independently?
So personally, I hate extra command line flags for this. I'd much
rather see is use something in the regular expression itself, and make
*that* be the way you do it, and make it be the preferred format.
Otherwise, you'll always have the issue that you want *part* to be
case-ignoring, and another entry not, and then it's just messy with
the ignore case being some other thing.
And we support that with perl regexps, but those are only enabled with
libpcre. I wonder if we could just make some simple pattern extension
that we make work even *without* libpcre.
IOW, instead of making people use -regexp-ignore-case, could we just
say that we *always* support the syntax of appending (?i) in front
of the regexp. So that your
git log --regexp-ignore-case --author=tiM --grep=wip
example would be
git log --author=(?i)tiM --grep=wip
and it would match the _author_ with ignoring case, but the
--grep=wip part would be an exact grep.
Right now the above already works (I think) if you:
- build with USE_LIBPCRE
- add that --perl-regexp switch.
but what I'm suggesting is that we'd make a special case for the
magical perl modifier pattern at the beginning for (?i), and make it
work even without USE_LIBPCRE, and without specifying --perl-regexp.
We'd just special-case that pattern (and perhaps _only_ that special
four-byte sequence of (?i) at the beginning of the search string),
but perhaps we could support '(?s)' too?
Hmm? I realize that this would be theoretically an incompatible
change, but it would be very convenient and if we document it well it
might be ok. I doubt people really search for (?i) at the beginning
of strings _except_ if they already know about the perl syntax and
want it.
And to clarify: I don't suggest always building with libpcre. I
literally suggest having something like
/* hacky mac-hack hack */
if (strncmp((?i), p-pattern, 4)) {
p-pattern += 4;
p-ignore_case = true;
}
just in front of the regcomp() call, and nothing more fancy than that.
Linus
--
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 v8 1/4] sha1_file.c: support reading from a loose object of unknown type
On 04/18/2015 02:40 AM, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
On Fri, Apr 17, 2015 at 09:21:31AM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
If there _is_ a performance implication to worry about here, I think it
would be that we are doing an extra malloc/free.
Thanks for reminding me; yes, that also worried me.
As an aside, I worried about the extra allocation for reading the header
in the first place. But it looks like we only do this on the --literally
code path (and otherwise use the normal unpack_sha1_header). Still, I
wonder if we could make this work automagically. That is, speculatively
unpack the first N bytes, assuming we hit the end-of-header. If not,
then go to a strbuf as the slow path. Then it would be fine to cover all
cases; the normal ones would be fast, and only ridiculous things would
incur the extra allocation.
Yes, that was what I was hoping to see eventually ;-)
Sorry for the delay, So after reading what Jeff said I tried to
implement it, this might not be a final version of the change, more of a
RFC version. What do you'll think?
@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream,
unsigned char *map, unsigned long ma
return git_inflate(stream, 0);
}
+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned
char *map,
+ unsigned long mapsize, void *buffer,
+ unsigned long bufsiz, struct
strbuf *header)
+{
+ unsigned char *cp;
+ int status;
+ int i = 0;
+
+ status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+
+ for (cp = buffer; cp stream-next_out; cp++)
+ if (!*cp) {
+ /* Found the NUL at the end of the header */
+ return 0;
+ }
+
+ strbuf_add(header, buffer, stream-next_out - (unsigned char
*)buffer);
+ do {
+ status = git_inflate(stream, 0);
+ strbuf_add(header, buffer, stream-next_out - (unsigned
char *)buffer);
+ for (cp = buffer; cp stream-next_out; cp++)
+ if (!*cp)
+ /* Found the NUL at the end of the header */
+ return 0;
+ stream-next_out = buffer;
+ stream-avail_out = bufsiz;
+ } while (status != Z_STREAM_END);
+ return -1;
+}
+
@@ -2555,17 +2610,24 @@ static int sha1_loose_object_info(const unsigned
char *sha1,
return -1;
if (oi-disk_sizep)
*oi-disk_sizep = mapsize;
- if (unpack_sha1_header(stream, map, mapsize, hdr, sizeof(hdr)) 0)
+ if ((flags LOOKUP_LITERALLY)) {
+ if (unpack_sha1_header_to_strbuf(stream, map, mapsize,
hdr, sizeof(hdr), hdrbuf) 0)
+ status = error(unable to unpack %s header with
--literally,
+ sha1_to_hex(sha1));
+ } else if (unpack_sha1_header(stream, map, mapsize, hdr,
sizeof(hdr)) 0)
status = error(unable to unpack %s header,
sha1_to_hex(sha1));
- else if ((status = parse_sha1_header(hdr, size)) 0)
+ if (hdrbuf.len) {
+ if ((status = parse_sha1_header_extended(hdrbuf.buf, oi,
flags)) 0)
+ status = error(unable to parse %s header with
--literally,
+ sha1_to_hex(sha1));
+ } else if ((status = parse_sha1_header_extended(hdr, oi, flags))
0)
status = error(unable to parse %s header,
sha1_to_hex(sha1));
and
--
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] git pull will regress between 'master' and 'pu'
On Mon, Apr 20, 2015 at 11:59:04AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: @@ -334,7 +333,7 @@ true) eval=git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress eval=$eval $gpg_sign_args - eval=$eval -m \\$merge_name\ $merge_head + eval=$eval FETCH_HEAD ;; esac eval exec $eval as we seem to special-case the name FETCH_HEAD. It assumes that git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull, but that seems safe. Unfortunately, git merge's parsing of FETCH_HEAD forgets that we may be creating an Octopus. Otherwise the above should work well. That sounds like a bug we should fix regardless. Unfortunately we still have to compute $merge_head ourselves here for the git pull --rebase case. That is not that unfortunate, I would say. I guess not. It is only a few lines of sed. And having the details there does let us customize the error cases. My main worry would just be a maintenance one: that somebody modifies git-pull to calculate merge_head differently, but it turns out that we ignore it when calling git-merge. But that's probably not that likely to matter in practice. -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 v8 1/4] sha1_file.c: support reading from a loose object of unknown type
On Tue, Apr 21, 2015 at 12:13:30AM +0530, karthik nayak wrote:
+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char
*map,
+ unsigned long mapsize, void *buffer,
+ unsigned long bufsiz, struct strbuf
*header)
+{
+ unsigned char *cp;
+ int status;
+ int i = 0;
+
+ status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
I wonder if we would feel comfortable just running this NUL-check as
part of unpack_sha1_header (i.e., in all code paths). It _shouldn't_
trigger in normal use, but I wonder if there would be any downsides
(e.g., maliciously crafted objects getting us to allocate memory or
something; I think it is fairly easy to convince git to allocate memory,
though).
+ for (cp = buffer; cp stream-next_out; cp++)
+ if (!*cp) {
+ /* Found the NUL at the end of the header */
+ return 0;
+ }
I think we can spell this as:
if (memchr(buffer, '\0', stream-next_out - buffer))
return 0;
which is shorter and possibly more efficient.
In theory we could also just start trying to parse the type/size header,
and notice there when we don't find the NUL. That's probably not worth
doing, though. The parsing is separated from the unpacking here, so it
would require combining those two operations in a single function. And
the extra NUL search here is likely not very expensive.
-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: Why does git log --author=pattern not work with regexp-ignore-case and other regexp-related options?
Tim Friske m...@tifr.de writes: I wonder why git log --author=pattern does not work with the regexp-ignore-case option and the other regexp-related options Wouldn't it be useful to make ... I think the reason is because nobody bothered to make it so. That does not necessarily say what you suggest is not useful, but if it were so very much useful in the real world, perhaps somebody may have already been motivated enough to make it so, and the fact that it has not happened might be an indirect indication of its predicted usefulness. I dunno. In any case, I do not offhand see how it would _hurt_ if we added such a feature. The only reason it may hurt existing users would be that people may depend on the current behaviour, trusting that exactly spelling --author=Tim option, when using case-ignoring matching of --grep=pattern to find the pattern in the log string filters out the other tim whose name is spelled with lowercase. Your proposed new world order _will_ break such users. But I do not think it is very likely to become a real-world issue. Of course, if the implementation is done poorly, it _will_ hurt the overall performance or maintainability and that would make such an implementation unacceptable, but that is a separate matter---it does not reject the feature, just a specific poor implementation. So a patch to do so cleanly with proper tests is very much welcomed. The same comment applies to your other wouldn't it be wonderful if -Gpattern became case-insensitive with an option? topic (but as I already said, -Sstring is *not* -Gstring with --fixed-strings). 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 v4] git-p4: Use -m when running p4 changes
On 20/04/15 16:25, Lex Spoon wrote: On Mon, Apr 20, 2015 at 11:15 AM, Luke Diamand l...@diamand.org wrote: Sorry - could you resubmit your patch (PATCHv4 it will be) with this change squashed in please? It will make life much easier, especially for Junio! The message you just responded is already the squashed version. It's a single patch that includes all changes so far discussed. The subject line says PATCH v4, although since it's in the same thread, not all email clients will show the subject change. Not sure how I missed that! It looks good, now, Ack! Thanks! Luke -- 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] git pull will regress between 'master' and 'pu'
Jeff King p...@peff.net writes: On Mon, Apr 20, 2015 at 11:59:04AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: @@ -334,7 +333,7 @@ true) eval=git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress eval=$eval $gpg_sign_args - eval=$eval -m \\$merge_name\ $merge_head + eval=$eval FETCH_HEAD ;; esac eval exec $eval as we seem to special-case the name FETCH_HEAD. It assumes that git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull, but that seems safe. Unfortunately, git merge's parsing of FETCH_HEAD forgets that we may be creating an Octopus. Otherwise the above should work well. That sounds like a bug we should fix regardless. But I am not sure how it should behave. git fetch $there A B C followed by git merge FETCH_HEAD merges only A, and I do not know if people have come to depend on this behaviour. I suspect there may be larger fallout from such a change, namely, what should git log FETCH_HEAD do? Should it traverse the history starting from all things that are not marked as not-for-merge, or should it just say git rev-parse FETCH_HEAD and use only the first one as the starting point? I would argue that it would be more consistent with how we envision the git merge FETCH_HEAD should work if git log FETCH_HEAD traversed from all fetched HEAD for merging, but surely it is a huge potential incompatibility. For that matter, git rev-parse FETCH_HEAD and even get_sha1() should yield all fetched HEAD for merging if we want to be consistent. I haven't thought this through yet but it does not look pretty. -- 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-p4 Question
On 20/04/15 17:41, FusionX86 wrote: Hello, Hopefully this is an appropriate place to ask questions about git-p4. I started at a company that wants to migrate from Perforce to Git. I'm new to Perforce and have been trying to learn just enough about it to get through this migration. Anyway, I've been playing with git-p4 and have one question/problem to discuss. After setting up the p4 cli client I can 'p4 sync' some //depot/main/app1 which pulls down the files I would expect from the Perforce server. If I use 'git p4 clone //depot/main/app1', I get: Doing initial import of //depot/main/app1/ from revision #head into refs/remotes/p4/master But I don't get any files from that depot/folder pulled down. I can git p4 clone other depot/folders though and get some files. I suspect that I'm just not understanding how the git-p4 module works. You could try doing the clone with '-v' to get a bit more information. Basically, I'm hoping to setup a live sync of Perforce to Git of certain depots in preparation for the migration. Also, if anyone has pointers or guides for this type of migration, any help is appreciated. I've done something similar in the past. You'll want to enable the --preserve-user option, for which you will need admin rights. If it's a one-way mirror (p4-to-git) then just run git-p4 periodically (if you use cron, then try to avoid having two or more instances running at the same time). If you want it to be two-way then it gets a bit more complicated. You might also want to consider using git fusion, which is Perforce's take on this problem. I've not used it myself. From past experience though I would say the biggest problem is getting developers to switch from the P4 mindset (centralized; code review hard to do or ignored) to the git mindset (decentralized; code review actively supported by the version control system). -- 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 -- 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] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
On Sat, Apr 18, 2015 at 01:35:51PM +1000, Stefan Saasen wrote: Here are the timings for the two patches: [...] Thanks, that matches what I was hoping for. My tweaked version of your second patch is: [...] - return find_pack_entry(sha1, e) freshen_file(e.p-pack_name); + if (!find_pack_entry(sha1, e)) + return 0; + if (e.p-freshened) + return 1; + return e.p-freshened = freshen_file(e.p-pack_name); } Whooops, yeah, setting the flag is probably helpful. :) We usually try to avoid assignments in a return like this, so I've written it out a little more verbosely in my final version. I'll send those patches in a moment. [1/2]: sha1_file: freshen pack objects before loose [2/2]: sha1_file: only freshen packs once per run Is there a chance to backport those changes to the 2.2+ branches? That's up to Junio. These patches can be applied straight to the jk/prune-mtime topic. Usually he would then merge the topic up to maint, which at this would potentially become the next v2.3.x. If an issue is critical (e.g., a security vulnerability), he'll sometimes merge and roll maintenance releases for older versions. But I don't know if this counts as critical (it is for you, certainly, but I don't think that many people are affected, as the crucial factor here is really the slow NFS filesystem operations). -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 1/2] sha1_file: freshen pack objects before loose
When writing out an object file, we first check whether it already exists and if so optimize out the write. Prior to 33d4221, we did this by calling has_sha1_file(), which will check for packed objects followed by loose. Since that commit, we check loose objects first. For the common case of a repository whose objects are mostly packed, this means we will make a lot of extra access() system calls checking for loose objects. We should follow the same packed-then-loose order that all of our other lookups use. Reported-by: Stefan Saasen ssaa...@atlassian.com Signed-off-by: Jeff King p...@peff.net --- sha1_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index 88f06ba..822aaef 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -3014,7 +3014,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign write_sha1_file_prepare(buf, len, type, sha1, hdr, hdrlen); if (returnsha1) hashcpy(returnsha1, sha1); - if (freshen_loose_object(sha1) || freshen_packed_object(sha1)) + if (freshen_packed_object(sha1) || freshen_loose_object(sha1)) return 0; return write_loose_object(sha1, hdr, hdrlen, buf, len, 0); } -- 2.4.0.rc2.384.g7297a4a -- 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] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
Jeff King p...@peff.net writes: ... But I don't know if this counts as critical (it is for you, certainly, but I don't think that many people are affected, as the crucial factor here is really the slow NFS filesystem operations). If it is critical to some people, they can downmerge to their custom old installations of Git they maintain with ease, of course, and that with ease part is the reason why I try to apply fixes to tip of the original topic branch even though they were merged to the mainline eons ago ;-). Thanks. The patches look good from cursory reading. -- 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] git pull will regress between 'master' and 'pu'
Junio C Hamano gits...@pobox.com writes: This is primarily note-to-self; even though I haven't got around bisecting yet, I think I know I did some bad change myself. git pull $URL $tag seems to: * fail to invoke the editor without --edit. * show the summary merge log message twice. We seem to be making a good progress on the discussion on this specific issue, but a larger tangent of this is that we do not seem to have test coverage to catch this regression. As we are planning to rewrite git pull, we need to make sure we have enough test coverage to ensure that nothing will regress before doing so. 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
[PATCH 2/2] sha1_file: only freshen packs once per run
Since 33d4221 (write_sha1_file: freshen existing objects,
2014-10-15), we update the mtime of existing objects that we
would have written out (had they not existed). For the
common case in which many objects are packed, we may update
the mtime on a single packfile repeatedly. This can result
in a noticeable performance problem if calling utime() is
expensive (e.g., because your storage is on NFS).
We can fix this by keeping a per-pack flag that lets us
freshen only once per program invocation.
An alternative would be to keep the packed_git.mtime flag up
to date as we freshen, and freshen only once every N
seconds. In practice, it's not worth the complexity. We are
racing against prune expiration times here, which inherently
must be set to accomodate reasonable program running times
(because they really care about the time between an object
being written and it becoming referenced, and the latter is
typically the last step a program takes).
Signed-off-by: Jeff King p...@peff.net
---
Hopefully I didn't botch the flag logic again. :) I tested with strace
-c myself this time, so I think it is good.
cache.h | 1 +
sha1_file.c | 9 -
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index 3d3244b..72c6888 100644
--- a/cache.h
+++ b/cache.h
@@ -1174,6 +1174,7 @@ extern struct packed_git {
int pack_fd;
unsigned pack_local:1,
pack_keep:1,
+freshened:1,
do_not_close:1;
unsigned char sha1[20];
/* something like .git/objects/pack/x.pack */
diff --git a/sha1_file.c b/sha1_file.c
index 822aaef..26b9b2b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2999,7 +2999,14 @@ static int freshen_loose_object(const unsigned char
*sha1)
static int freshen_packed_object(const unsigned char *sha1)
{
struct pack_entry e;
- return find_pack_entry(sha1, e) freshen_file(e.p-pack_name);
+ if (!find_pack_entry(sha1, e))
+ return 0;
+ if (e.p-freshened)
+ return 1;
+ if (!freshen_file(e.p-pack_name))
+ return 0;
+ e.p-freshened = 1;
+ return 1;
}
int write_sha1_file(const void *buf, unsigned long len, const char *type,
unsigned char *returnsha1)
--
2.4.0.rc2.384.g7297a4a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
What happen if show_http_message fails to reencode?
I'm trying to make my git server sends http messages in non-ASCII
encoding. And I have a question.
At 206-218 in remote-curl.c:
static int show_http_message(struct strbuf *type, struct strbuf *charset,
struct strbuf *msg)
{
const char *p, *eol;
/*
* We only show text/plain parts, as other types are likely
* to be ugly to look at on the user's terminal.
*/
if (strcmp(type-buf, text/plain))
return -1;
if (charset-len)
strbuf_reencode(msg, charset-buf, get_log_output_encoding());
What happen if the message has a character which cannot be encoded by
the encoding defined by i18n.logoutputencoding? Drops only the
character or brakes the whole message?
-EungJun
--
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
