Re: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects
Duy Nguyen writes: > I think this applies to general case as well, not just shallow. > Imagine I have a disconnected commit that points to the latest tree > (i.e. it contains most of latest changes). Because it's disconnected, > it'll be ignored by the server side. But if the servide side does > mark_tree_interesting on this commit, a bunch of blobs might be > excluded from sending. I think you meant mark_tree_UNinteresting. > ... So perhaps we could go over have_obj list > again, if it's not processed and is > > - a tree-ish, mark_tree_uninteresting > - a blob, just mark unintesting > > and this does regardless of shallow state or edges. As a general idea, I agree it may be worth trying out to see if your concern that the "have" list may be so big that this approach may be more costly than it is worth. If the recipient is known to have something, we do not have to send it. The things that we decide not to send are not necessarily what the recipient has, which introduces a twist you need to watch out for if we want to go that route. If the recipient is known to have something, a thin transfer can send a delta against it. You do not want to send the commits before the shallow boundary (i.e. the parents of the commits listed in .git/shallow) because the recipient does not want them, and that means you may have to use a different mark to record that fact. The recipient does not have them, we do not want to send them, and they cannot be used as a delta base for what we do send. Which is quite different from the ordinary "uninteresting" objects, those we decide not to send because the recipient has them. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] pack-objects: do not print usage when repacking
On Wed, Aug 7, 2013 at 4:00 PM, Stefan Beller wrote: > --- > builtin/pack-objects.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c > index 1742ea1..0bd8f3b 100644 > --- a/builtin/pack-objects.c > +++ b/builtin/pack-objects.c > @@ -2585,7 +2585,7 @@ int cmd_pack_objects(int argc, const char **argv, const > char *prefix) > base_name = argv[0]; > argc--; > } > - if (pack_to_stdout != !base_name || argc) > + if (!(repack_flags & REPACK_IN_PROGRESS) && (pack_to_stdout != > !base_name || argc)) > usage_with_options(pack_usage, pack_objects_options); > > rp_av[rp_ac++] = "pack-objects"; Hello Stefan, I'm not sure I understand that and why it's needed, would you mind explain it to me ? (and/or maybe add it to the commit message) 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: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects
Duy Nguyen writes: > Haven't found time to read the rest yet, but this I can answer. > .git/shallow records graft points. If a commit is in .git/shallow and > it exists in the repository, the commit is considered to have no > parents regardless of what's recorded in repository. So .git/shallow > refers to the new roots, not the missing bits. 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: Remove old forgotten command: whatchanged
Stefan Beller writes: > Well if we make sure the whatchanged command can easily be reproduced > with the log command, we could add the missing parameters to it, hence > no change for the user. (git whatchanged == git log --raw --no-merges or > git log --wc [to be done yet]). > > So I did not mean to introduce a change for users! I certainly did *not* read that from between the lines of what you wrote: + int cmd_whatchanged(int argc, const char **argv, const char *prefix) + { + return cmd_log(argc, argv, prefix) + } In principle, I agree that it is a good idea to try to share enough code, while keeping the flexiblity and clarity of the code for maintainability. In the extreme, you could rewrite these two functions like so: static int cmd_lw_helper( int argc, const char **argv, const char *prefix, int whoami) { struct rev_info rev; struct setup_revision_opt opt; init_grep_defaults(); git_config(git_log_config, NULL); init_revisions(&rev, prefix); if (whoami == 'l') { /* log */ rev.always_show_header = always_show_header; } else { /* whatchanged */ rev.diff = diff; rev.simplify_history = simplify_history; } memset(opt, 0, sizeof(opt)); opt.def = "HEAD"; opt.revarg_opt = REVARG_COMMITTISH; cmd_log_init(argc, argv, prefix, &rev, &opt); if (whoami == 'w') { if (!rev.diffopt.output_format) rev.diffopt.output_format = DIFF_FORMAT_RAW; } return cmd_log_walk(&rev); } int cmd_log(int argc, const char **argv, const char *prefix) { return cmd_lw_helper(argc, argv, prefix, 'l'); } int cmd_whatchanged(int argc, const char **argv, const char *prefix) { return cmd_lw_helper(argc, argv, prefix, 'w'); } but at that point, the cost you have to pay when you need to update one of them but not the other becomes higher. As whatchanged is kept primarily for people who learned Git by word of mouth reading the kernel mailing list and are used to that command. Its external interface and what it does is not likely to drastically change. On the other hand, "log" is a primary Porcelain and we would expect constant improvements. Between the "share more code for reuse" and "keep the flexibility and clarity for maintainability", it is a subtle balancing act. Personally I think the current code strikes a good balance by not going to the extreme, given that "change one (i.e. log) but not the other" is a very likely pattern for the evolution of these two commands. -- 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] status: always show tracking branch even no change
If the current branch has an upstream branch, and there are changes between the current branch and its upstream, some commands (such as "git status", "git status -bs", and "git checkout") will report their relationship. E.g. $ git status # On branch master # Your branch is ahead of 'origin/master' by 1 commit. # (use "git push" to publish your local commits) # ... $ git status -bs ## master...origin/master [ahead 1] ... $ git checkout master Already on 'master' Your branch is ahead of 'origin/master' by 1 commit. (use "git push" to publish your local commits) But if there is no difference between the current branch and its upstream, the relationship will not be reported, and it's hard to tell whether the current branch has a tracking branch or not. And what's worse, when the 'push.default' config variable is set to `matching`, it's hard to tell whether the current branch has already been pushed out or not at all [1]. With this patch, "git status" will report relationship between the current branch and its upstream counterpart even if there is no difference. $ git status # On branch master # Your branch is identical to 'origin/master'. # ... $ git status -bs ## master...origin/master ... $ git checkout master Already on 'master' Your branch is identical to 'origin/master'. [1]: http://thread.gmane.org/gmane.comp.version-control.git/198703 Signed-off-by: Jiang Xin --- remote.c | 22 ++-- t/t6040-tracking-info.sh | 54 wt-status.c | 13 +--- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/remote.c b/remote.c index 2433467..825f278 100644 --- a/remote.c +++ b/remote.c @@ -1740,6 +1740,10 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) const char *rev_argv[10], *base; int rev_argc; + /* Set both num_theirs and num_ours as undetermined. */ + *num_theirs = -1; + *num_ours = -1; + /* * Nothing to report unless we are marked to build on top of * somebody else. @@ -1758,14 +1762,16 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) theirs = lookup_commit_reference(sha1); if (!theirs) return 0; + *num_theirs = 0; if (read_ref(branch->refname, sha1)) return 0; ours = lookup_commit_reference(sha1); if (!ours) return 0; + *num_ours = 0; - /* are we the same? */ + /* are we the same? both num_theirs and num_ours have been set to 0. */ if (theirs == ours) return 0; @@ -1786,8 +1792,6 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) prepare_revision_walk(&revs); /* ... and count the commits on each side. */ - *num_ours = 0; - *num_theirs = 0; while (1) { struct commit *c = get_revision(&revs); if (!c) @@ -1812,12 +1816,18 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb) int num_ours, num_theirs; const char *base; - if (!stat_tracking_info(branch, &num_ours, &num_theirs)) - return 0; + if (!stat_tracking_info(branch, &num_ours, &num_theirs)) { + if (num_ours || num_theirs) + return 0; + } base = branch->merge[0]->dst; base = shorten_unambiguous_ref(base, 0); - if (!num_theirs) { + if (!num_ours && !num_theirs) { + strbuf_addf(sb, + _("Your branch is identical to '%s'.\n"), + base); + } else if (!num_theirs) { strbuf_addf(sb, Q_("Your branch is ahead of '%s' by %d commit.\n", "Your branch is ahead of '%s' by %d commits.\n", diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh index ec2b516..eafce7d 100755 --- a/t/t6040-tracking-info.sh +++ b/t/t6040-tracking-info.sh @@ -28,18 +28,20 @@ test_expect_success setup ' git reset --hard HEAD^ && git checkout -b b4 origin && advance e && - advance f + advance f && + git checkout -b b5 origin ) && git checkout -b follower --track master && advance g ' -script='s/^..\(b.\)[0-9a-f]*\[\([^]]*\)\].*/\1 \2/p' +script='s/^..\(b.\)[0-9a-f]*\(\[\([^]]*\)\]\)\{0,1\}.*/\1 \3/p' cat >expect <<\EOF b1 ahead 1, behind 1 b2 ahead 1, behind 1 b3 behind 1 b4 ahead 2 +b5 EOF test_expect_success 'branch -v' ' @@ -56,6 +58,7 @@ b1 origin/master: ahead 1, behind 1 b2 origin/master: ahead 1, behind 1 b3 origin/master: behind 1 b4 origin/master: ahead 2 +b5 origin/master EOF test_expect_success 'bra
Re: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects
On Fri, Jul 12, 2013 at 5:01 AM, Matthijs Kooijman wrote: > Hi folks, > > while playing with shallow fetches, I've found that in some > circumstances running git fetch with --depth can return too many objects > (in particular, _all_ the objects for the requested revisions are > returned, even when some of those objects are already known to the > client). > > This happens when a client issues a fetch with a depth bigger or equal > to the number of commits the server is ahead of the client. In this > case, the revisions to be sent over will be completely detached from any > revisions the client already has (history-wise), causing the server to > effectively ignore all objects the client has (as advertised using its > have lines) and just send over _all_ objects (needed for the revisions > it is sending over). > > I've traced this down to the way do_rev_list in upload-pack.c works. If > I've poured over the code enough to understand it, this is what happens: > - The new shallow roots are made into graft points without parents. > - The "want" commits are added to the pending list (revs->pending) > - The "have" commits are marked uninteresting and added to the pending list > - prepare_revision_walk is called, which adds everything from the >pending list into the commmit list (revs->commits) > - limit_list is called, which traverses the history of each interesting >commit in the commit list (i.e., all want revisions), up to excluding >the first uninteresting commit (i.e. a have revision). The result of >this is the new commit list. > >This means the commit list now contains all commits that the client >wants, up to (excluding) any commits he already has or up to >(including) any (new) shallow roots. > - mark_edges_uninteresting is called, which marks the tree of every >parent of each edge in the commit list as uninteresting (in practice, >this marks the tree of each uninteresting parent, since those are by >definition the only kinds of revisions that can be beyond the edge). > - All trees and blobs that are referenced by trees in the commit list >but are not marked as uninteresting, are passed to git-pack-objects >to put into the pack. > > Normally, the list of commits to send over is connected to the > client's existing commits (which are marked as uninteresting). This > means that only the trees of those uninteresting ("have") commits that > are actually (direct) predecessors of the commits to send over are > marked as uninteresting. This is probably useful, since it prevents > having to go over all trees the client has (for other branches, for > example) and instead limits to the trees that are the most likely to > contain duplicate (or similar, for delta-ing) objects. > > However, in the "detached shallow fetch" case, this assumption is no > longer valid. There will be no uninteresting commits as parents for > the commit list, since all edge commits will be shallow roots (hence > have no parents). Ideally, one would find out which of the "detached" > "have" revisions are the closest to the new shallow roots, but with the > current code these shallow roots have their parents cut off long before > this code even runs, so this is probably not feasible. I think this applies to general case as well, not just shallow. Imagine I have a disconnected commit that points to the latest tree (i.e. it contains most of latest changes). Because it's disconnected, it'll be ignored by the server side. But if the servide side does mark_tree_interesting on this commit, a bunch of blobs might be excluded from sending. I used to (ab)use git and store a bunch of tags point to trees. These trees share a lot. Still, fetching a new tag means pulling all objects of the new tree even though it only needs a few new blobs and trees. So perhaps we could go over have_obj list again, if it's not processed and is - a tree-ish, mark_tree_uninteresting - a blob, just mark unintesting and this does regardless of shallow state or edges. The only downside is mark_tree_uninteresting is recursive so in unpacks lots of trees if have_obj is long, or the worktree is really big. Commit bitmap should help reduce the cost if have_obj is a committish, at least. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Remove old forgotten command: whatchanged
Junio C Hamano wrote: > The only thing it does is to scratch an irrelevant itch by people > who peek the codebase and find an old command whose existence does > not even hurt them. They may have too much time on their hand, but > that is not an excuse to waste other people's time by adding extra > makework on our plate, or changing the behaviour for people who use > the command without explanation. It's a maintenance burden that confuses users. I'd encourage you to read git-whatchanged(1), and tell me why nobody has updated it. We're not maintaining a museum. -- 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/4] backup_file dummy function
On Wed, Aug 7, 2013 at 9:00 PM, Stefan Beller wrote: > --- > pack-write.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/pack-write.c b/pack-write.c > index e6aa7e3..b728ea2 100644 > --- a/pack-write.c > +++ b/pack-write.c > @@ -344,6 +344,11 @@ struct sha1file *create_tmp_packfile(char > **pack_tmp_name) > return sha1fd(fd, *pack_tmp_name); > } > > +void backup_file(char *filename) > +{ > + > +} > + I think the function looks like this (before it got lost after rebase) +static void backup_file(const char *path) +{ + struct stat st; + if (repack_flags & REPACK_IN_PROGRESS && + !stat(path, &st)) { + struct strbuf old = STRBUF_INIT; + strbuf_addf(&old, "%s.old", path); + if (rename(path, old.buf)) + die_errno("unable to rename pack %s", path); + string_list_append(&backup_files, strbuf_detach(&old, NULL)); + } +} + -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Build in git-repack
On Wed, Aug 7, 2013 at 10:48 PM, Junio C Hamano wrote: > Matthieu Moy writes: > >> [ It's cool you're working on this, I'd really like a git-repack in C. >> That would fix this >> http://thread.gmane.org/gmane.comp.version-control.git/226458 ] >> >> Stefan Beller writes: >> >>> From: Nguyễn Thái Ngọc Duy >>> >>> pack-objects learns a few more options to take over what's been done >>> by git-repack.sh. cmd_repack() becomes a wrapper around >>> cmd_pack_objects(). >> >> I think the patch would read easier if these were split into two >> patches: one doing the real stuff in pack-objects, and then getting rid >> of git-repack.sh to replace it with a trivial built-in. >> >> Actually, I'm wondering why pack-objects requires so much changes. >> git-repack.sh was already a relatively small wrapper around >> pack-objects, and did not need the new options you add, so why are they >> needed? In particular adding the new --update-info option that just does >> >>> +if (repack_flags & REPACK_UPDATE_INFO) >>> +update_server_info(0); >> >> seems overkill to me: why don't you just let cmd_repack call >> update_server_info(0)? > > My feeling exactly. I would rather see a patch that does not touch > pack-objects at all, and use run_command() interface to spawn it. > Once we do have to pack, the necessary processing cycle will dwarf > the fork/exec latency anyway, no? I'm not opposed to run_command(). I think the reason I wanted to move repack functionality to pack-objects is to avoid reading sha-1 from pack-objects and reconstruct the paths again from the sha-1. But for simplicity, perhaps we should not touch pack-objects at all. Then we could have builtin/repack.c instead of stuffing cmd_repack in builtin/pack-objects.c @Stefan, if you want to push this work, feel free to take it as _your_ patch, rewrite as will. You don't need to retain my name. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git exproll: steps to tackle gc aggression
On Wed, Aug 7, 2013 at 7:10 AM, Martin Fick wrote: >> I wonder if a simpler approach may be nearly efficient as >> this one: keep the largest pack out, repack the rest at >> fetch/push time so there are at most 2 packs at a time. >> Or we we could do the repack at 'gc --auto' time, but >> with lower pack threshold (about 10 or so). When the >> second pack is as big as, say half the size of the >> first, merge them into one at "gc --auto" time. This can >> be easily implemented in git-repack.sh. > > It would definitely be better than the current gc approach. > > However, I suspect it is still at least one to two orders of > magnitude off from where it should be. To give you a real > world example, on our server today when gitexproll ran on > our kernel/msm repo, it consolidated 317 pack files into one > almost 8M packfile (it compresses/dedupes shockingly well, > one of those new packs was 33M). Our largest packfile in > that repo is 1.5G! > > So let's now imagine that the second closest packfile is > only 100M, it would keep getting consolidated with 8M worth > of data every day (assuming the same conditions and no extra > compression). That would take (750M-100M)/8M ~ 81 days to > finally build up large enough to no longer consolidate the > new packs with the second largest pack file daily. During > those 80+ days, it will be on average writing 325M too much > per day (when it should be writing just 8M). > > So I can see the appeal of a simple solution, unfortunately > I think one layer would still "suck" though. And if you are > going to add even just one extra layer, I suspect that you > might as well go the full distance since you probably > already need to implement the logic to do so? I see. It looks like your way is the best way to go. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 12/19] read-cache: read index-v5
On Wed, Aug 7, 2013 at 3:23 PM, Thomas Gummerer wrote: >> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer >> wrote: >>> +static void ce_queue_push(struct cache_entry **head, >>> +struct cache_entry **tail, >>> +struct cache_entry *ce) >>> +{ >>> + if (!*head) { >>> + *head = *tail = ce; >>> + (*tail)->next = NULL; >>> + return; >>> + } >>> + >>> + (*tail)->next = ce; >>> + ce->next = NULL; >>> + *tail = (*tail)->next; >> >> No no no. "next" is for name-hash.c don't "reuse" it here. And I don't >> think you really need to maintain a linked list of cache_entries by >> the way. More on read_entries comments below.. > > You're right, I don't need it for the reading code. I do need to keep a > list of cache-entries for writing though, where a linked list is best > suited for the task. I'll use a next_ce pointer for that. Hmm.. yeah you need to maintain temporary structures before writing out direntries, then fileentries table. We can't just write as we traverse through the cache in one go. Maybe a new field in cache_entry for the linked list, or maintain the linked list off cache_entry.. whichever is easier for you. > I've tried implementing both versions with the internal tree structure, > see below for timings. > > simpleapi is the code that was posted to the list, HEAD uses a > tree-structure for the directories internally, and replaces strcmp with > cache_name_compare and leaves out the prefix. This tree uses dummy > entries for the sub-directories. > > Dummy entries are single NUL-bytes mixed with the cache-entries, which > should give optimal performance. I haven't thought much about > corruption of the index, but I don't think we would need any additional > crc checksums or similar. > > The performance advantage seems very slim to none when using the dummy > entries to avoid the comparison though, so I don't think it makes sense > to make the index more complicated for a very small speed-up. Agreed. > Testsimpleapi HEAD > this tree > - > 0003.2: v[23]: update-index 3.22(2.61+0.58) 3.20(2.56+0.61) > -0.6%3.22(2.67+0.53) +0.0% > 0003.3: v[23]: grep nonexistent -- subdir 1.65(1.36+0.27) 1.65(1.34+0.30) > +0.0%1.67(1.34+0.31) +1.2% > 0003.4: v[23]: ls-files -- subdir 1.50(1.20+0.29) 1.54(1.18+0.34) > +2.7%1.53(1.22+0.30) +2.0% > 0003.6: v4: update-index2.69(2.28+0.39) 2.70(2.21+0.47) > +0.4%2.70(2.27+0.41) +0.4% > 0003.7: v4: grep nonexistent -- subdir 1.33(0.98+0.34) 1.36(1.01+0.33) > +2.3%1.34(1.03+0.30) +0.8% > 0003.8: v4: ls-files -- subdir 1.21(0.86+0.33) 1.23(0.91+0.30) > +1.7%1.22(0.90+0.30) +0.8% > 0003.10: v5: update-index 2.12(1.58+0.52) 2.20(1.67+0.52) > +3.8%2.19(1.66+0.51) +3.3% > 0003.11: v5: ls-files 1.57(1.21+0.34) 1.55(1.20+0.33) > -1.3%1.52(1.21+0.29) -3.2% > 0003.12: v5: grep nonexistent -- subdir 0.08(0.06+0.01) 0.07(0.04+0.02) > -12.5% 0.07(0.04+0.02) -12.5% > 0003.13: v5: ls-files -- subdir 0.07(0.04+0.01) 0.06(0.05+0.00) > -14.3% 0.07(0.05+0.01) +0.0% -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 12/19] read-cache: read index-v5
On Wed, Jul 17, 2013 at 3:11 PM, Thomas Gummerer wrote: > Duy Nguyen writes: > > [..] > >>> +static int read_entries(struct index_state *istate, struct directory_entry >>> **de, >>> + unsigned int *entry_offset, void **mmap, >>> + unsigned long mmap_size, unsigned int *nr, >>> + unsigned int *foffsetblock) >>> +{ >>> + struct cache_entry *head = NULL, *tail = NULL; >>> + struct conflict_entry *conflict_queue; >>> + struct cache_entry *ce; >>> + int i; >>> + >>> + conflict_queue = NULL; >>> + if (read_conflicts(&conflict_queue, *de, mmap, mmap_size) < 0) >>> + return -1; >>> + for (i = 0; i < (*de)->de_nfiles; i++) { >>> + if (read_entry(&ce, >>> + *de, >>> + entry_offset, >>> + mmap, >>> + mmap_size, >>> + foffsetblock) < 0) >>> + return -1; >>> + ce_queue_push(&head, &tail, ce); >>> + *foffsetblock += 4; >>> + >>> + /* >>> +* Add the conflicted entries at the end of the index file >>> +* to the in memory format >>> +*/ >>> + if (conflict_queue && >>> + (conflict_queue->entries->flags & CONFLICT_CONFLICTED) >>> != 0 && >>> + !cache_name_compare(conflict_queue->name, >>> conflict_queue->namelen, >>> + ce->name, ce_namelen(ce))) { >>> + struct conflict_part *cp; >>> + cp = conflict_queue->entries; >>> + cp = cp->next; >>> + while (cp) { >>> + ce = convert_conflict_part(cp, >>> + conflict_queue->name, >>> + conflict_queue->namelen); >>> + ce_queue_push(&head, &tail, ce); >>> + conflict_part_head_remove(&cp); >>> + } >>> + conflict_entry_head_remove(&conflict_queue); >>> + } >> >> I start to wonder if separating staged entries is a good idea. It >> seems to make the code more complicated. The good point about conflict >> section at the end of the file is you can just truncate() it out. >> Another way is putting staged entries in fileentries, sorted >> alphabetically then by stage number, and a flag indicating if the >> entry is valid. When you remove resolve an entry, just set the flag to >> invalid (partial write), so that read code will skip it. >> >> I think this approach is reasonably cheap (unless there are a lot of >> conflicts) and it simplifies this piece of code. truncate() may be >> overrated anyway. In my experience, I "git add " as soon as I >> resolve (so that "git diff" shrinks). One entry at a time, one >> index write at a time. I don't think I ever resolve everything then >> "git add -u .", which is where truncate() shines because staged >> entries are removed all at once. We should optimize for one file >> resolution at a time, imo. > > Thanks for your comments. I'll address the other ones once we decided > to do with the conflicts. > > It does make the code quite a bit more complicated, but also has one > advantage that you overlooked. I did overlook, although my goal is to keep the code simpler, not more comlicated. The thinking is if we can find everything in fileentries table, the code here is simplified, so.. > We wouldn't truncate() when resolving > the conflicts. The resolve undo data is stored with the conflicts and > therefore we could just flip a bit and set the stage of the cache-entry > in the main index to 0 (always once we got partial writing). This can > be fast both in case we resolve one entry at a time and when we resolve > a lot of entries. The advantage is even bigger when we resolve one > entry at a time, when we otherwise would have to re-write the index for > each conflict resolution. If I understand it correctly, filentries can only contain stage-0 or stage-1 entries, "stage > 0" entries are stored in conflict data. Once a conflict is solved, you update the stage-1 entry in fileentries, turning it to stage-0 and recalculate the entry checksum. Conflict data remains there to function as the old REUC extension. Correct? First of all, if that's true, we only need 1 bit for stage in fileentries table. Secondly, you may get away with looking up to conflict data in this function by storing all stages in fileentries (now we need 2-bit stage), replicated in conflict data for reuc function. When you resolve conflict, you flip stage-1 to stage-0, and flip (a new bit) to mark stage-2 entry invalid so the code knows to skip it. Next time the index is rewritten, invalid entr
Re: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects
On Thu, Aug 8, 2013 at 8:01 AM, Junio C Hamano wrote: > Matthijs Kooijman writes: > >>> > In your discussion (including the comment), you talk about "shallow >>> > root" (I think that is the same as what we call "shallow boundary"), >>> I think so, yes. I mean to refer to the commits referenced in >>> .git/shallow, that have their parents "hidden". >> Could you confirm that I got the terms right here (or is the shallow >> boundary the first hidden commit?) > > As long as you are consistent it is fine. I _think_ boundary refers > to what is recorded in the .git/shallow file, so they are commits > that are missing from our repository, and their immediate children > are available. Haven't found time to read the rest yet, but this I can answer. .git/shallow records graft points. If a commit is in .git/shallow and it exists in the repository, the commit is considered to have no parents regardless of what's recorded in repository. So .git/shallow refers to the new roots, not the missing bits. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects
Matthijs Kooijman writes: >> > In your discussion (including the comment), you talk about "shallow >> > root" (I think that is the same as what we call "shallow boundary"), >> I think so, yes. I mean to refer to the commits referenced in >> .git/shallow, that have their parents "hidden". > Could you confirm that I got the terms right here (or is the shallow > boundary the first hidden commit?) As long as you are consistent it is fine. I _think_ boundary refers to what is recorded in the .git/shallow file, so they are commits that are missing from our repository, and their immediate children are available. > My proposal was to only apply the fix for all have revisions when the > previous history traversal came across some shallow boundary commits. If > this happens, then that shallow boundary commit will be a "new" one and > it will have prevented the history traversal from finding the full list > of relevant "have" commits. In this case, we should just use all "have" > commits instead. > > Now, looking at the code, I see a few options for detecting this case: > > 1 Modify mark_edges_uninteresting to return a boolean (or have an >output argument) if any of the commits in the list of commits to find >(not the edges) is a shallow boundary. > 2 Modify mark_edges_uninteresting to have a "show_shallow" argument >that gets called for every shallow boundary. The show_shallow >function passed would then simply keep a boolean if it is passed at >least once. > 3 Add another loop over the commits _after_ the call to >mark_edges_uninteresting, that simply looks for any shallow boundary >commit. > > The last option seems sensible to me, since it prevents modifying the > somewhat generic mark_edges_uninteresting function for this specific > usecase. On the other hand, it does mean that the list of commits is > looped twice, not sure what that means for performance. > > Before I go and implement one of these, which option seems best to you? My gut feeling without looking at any patch is that the simplest (i.e. 3.) would be the best among these three. But I suspect, with any of these approaches, you would need to be very careful futzing with the edge ones. It may have an interesting interactions with --thin transfer. -- 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/5] fetch: rename file-scope global "transport" to "gtransport"
Although many functions in this file take a "struct transport" as a parameter, "fetch_one()" assigns to the global singleton instance which is a file-scope static, in order to allow a parameterless signal handler unlock_pack() to access it. Rename the variable to gtransport to make sure these uses stand out. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index d784b2e..636b47f 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -36,7 +36,7 @@ static int tags = TAGS_DEFAULT, unshallow; static const char *depth; static const char *upload_pack; static struct strbuf default_rla = STRBUF_INIT; -static struct transport *transport; +static struct transport *gtransport; static const char *submodule_prefix = ""; static const char *recurse_submodules_default; @@ -95,8 +95,8 @@ static struct option builtin_fetch_options[] = { static void unlock_pack(void) { - if (transport) - transport_unlock_pack(transport); + if (gtransport) + transport_unlock_pack(gtransport); } static void unlock_pack_on_signal(int signo) @@ -818,13 +818,13 @@ static int do_fetch(struct transport *transport, static void set_option(const char *name, const char *value) { - int r = transport_set_option(transport, name, value); + int r = transport_set_option(gtransport, name, value); if (r < 0) die(_("Option \"%s\" value \"%s\" is not valid for %s"), - name, value, transport->url); + name, value, gtransport->url); if (r > 0) warning(_("Option \"%s\" is ignored for %s\n"), - name, transport->url); + name, gtransport->url); } static int get_one_remote_for_fetch(struct remote *remote, void *priv) @@ -949,8 +949,8 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) die(_("No remote repository specified. Please, specify either a URL or a\n" "remote name from which new revisions should be fetched.")); - transport = transport_get(remote, NULL); - transport_set_verbosity(transport, verbosity, progress); + gtransport = transport_get(remote, NULL); + transport_set_verbosity(gtransport, verbosity, progress); if (upload_pack) set_option(TRANS_OPT_UPLOADPACK, upload_pack); if (keep) @@ -983,10 +983,10 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) sigchain_push_common(unlock_pack_on_signal); atexit(unlock_pack); refspec = parse_fetch_refspec(ref_nr, refs); - exit_code = do_fetch(transport, refspec, ref_nr); + exit_code = do_fetch(gtransport, refspec, ref_nr); free_refspec(ref_nr, refspec); - transport_disconnect(transport); - transport = NULL; + transport_disconnect(gtransport); + gtransport = NULL; return exit_code; } -- 1.8.4-rc1-210-gf6d87e2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] fetch: refactor code that fetches leftover tags
Usually the upload-pack process running on the other side will give us all the reachable tags we need during the primary object transfer in do_fetch(). If that does not happen (e.g. the other side may be running a third-party implementation of upload-pack), we will run another fetch to pick up leftover tags that we know point at the commits reachable from our updated tips. Separate out the code to run this second fetch into a helper function. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 39a3fc8..0b21f07 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -745,6 +745,13 @@ struct transport *prepare_transport(struct remote *remote) return transport; } +static void backfill_tags(struct transport *transport, struct ref *ref_map) +{ + transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); + transport_set_option(transport, TRANS_OPT_DEPTH, "0"); + fetch_refs(transport, ref_map); +} + static int do_fetch(struct transport *transport, struct refspec *refs, int ref_count) { @@ -828,11 +835,8 @@ static int do_fetch(struct transport *transport, struct ref **tail = &ref_map; ref_map = NULL; find_non_local_tags(transport, &ref_map, &tail); - if (ref_map) { - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); - transport_set_option(transport, TRANS_OPT_DEPTH, "0"); - fetch_refs(transport, ref_map); - } + if (ref_map) + backfill_tags(transport, ref_map); free_refs(ref_map); } -- 1.8.4-rc1-210-gf6d87e2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] fetch: work around "transport-take-over" hack
A Git-aware "connect" transport allows the "transport_take_over" to redirect generic transport requests like fetch(), push_refs() and get_refs_list() to the native Git transport handling methods. The take-over process replaces transport->data with a fake data that these method implementations understand. While this hack works OK for a single request, it breaks when the transport needs to make more than one requests. transport->data that used to hold necessary information for the specific helper to work correctly is destroyed during the take-over process. One codepath that this matters is "git fetch" in auto-follow mode; when it does not get all the tags that ought to point at the history it got (which can be determined by looking at the peeled tags in the initial advertisement) from the primary transfer, it internally makes a second request to complete the fetch. Because "take-over" hack has already destroyed the data necessary to talk to the transport helper by the time this happens, the second request cannot make a request to the helper to make another connection to fetch these additional tags. Mark such a transport as "cannot_reuse", and use a separate transport to perform the backfill fetch in order to work around this breakage. Note that this problem does not manifest itself when running t5802, because our upload-pack gives you all the necessary auto-followed tags during the primary transfer. You would need to step through "git fetch" in a debugger, stop immediately after the primary transfer finishes and writes these auto-followed tags, remove the tag references and repack/prune the repository to convince the "find-non-local-tags" procedure that the primary transfer failed to give us all the necessary tags, and then let it continue, in order to trigger the bug in the secondary transfer this patch fixes. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 13 + transport.c | 2 ++ transport.h | 6 ++ 3 files changed, 21 insertions(+) diff --git a/builtin/fetch.c b/builtin/fetch.c index 0b21f07..57ab7e4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -37,6 +37,7 @@ static const char *depth; static const char *upload_pack; static struct strbuf default_rla = STRBUF_INIT; static struct transport *gtransport; +static struct transport *gsecondary; static const char *submodule_prefix = ""; static const char *recurse_submodules_default; @@ -97,6 +98,8 @@ static void unlock_pack(void) { if (gtransport) transport_unlock_pack(gtransport); + if (gsecondary) + transport_unlock_pack(gsecondary); } static void unlock_pack_on_signal(int signo) @@ -747,9 +750,19 @@ struct transport *prepare_transport(struct remote *remote) static void backfill_tags(struct transport *transport, struct ref *ref_map) { + if (transport->cannot_reuse) { + gsecondary = prepare_transport(transport->remote); + transport = gsecondary; + } + transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); fetch_refs(transport, ref_map); + + if (gsecondary) { + transport_disconnect(gsecondary); + gsecondary = NULL; + } } static int do_fetch(struct transport *transport, diff --git a/transport.c b/transport.c index e15db98..de25588 100644 --- a/transport.c +++ b/transport.c @@ -875,6 +875,8 @@ void transport_take_over(struct transport *transport, transport->push_refs = git_transport_push; transport->disconnect = disconnect_git; transport->smart_options = &(data->options); + + transport->cannot_reuse = 1; } static int is_local(const char *url) diff --git a/transport.h b/transport.h index ea70ea7..96e0ede 100644 --- a/transport.h +++ b/transport.h @@ -27,6 +27,12 @@ struct transport { */ unsigned got_remote_refs : 1; + /* +* Transports that call take-over destroys the data specific to +* the transport type while doing so, and cannot be reused. +*/ + unsigned cannot_reuse : 1; + /** * Returns 0 if successful, positive if the option is not * recognized or is inapplicable, and negative if the option -- 1.8.4-rc1-210-gf6d87e2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] fetch: refactor code that prepares a transport
Make a helper function prepare_transport() that returns a transport to talk to a given remote. The set_option() helper that used to always affect the file-scope global "gtransport" now takes a transport as its parameter. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 46 ++ 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 636b47f..39a3fc8 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -720,6 +720,31 @@ static int truncate_fetch_head(void) return 0; } +static void set_option(struct transport *transport, const char *name, const char *value) +{ + int r = transport_set_option(transport, name, value); + if (r < 0) + die(_("Option \"%s\" value \"%s\" is not valid for %s"), + name, value, transport->url); + if (r > 0) + warning(_("Option \"%s\" is ignored for %s\n"), + name, transport->url); +} + +struct transport *prepare_transport(struct remote *remote) +{ + struct transport *transport; + transport = transport_get(remote, NULL); + transport_set_verbosity(transport, verbosity, progress); + if (upload_pack) + set_option(transport, TRANS_OPT_UPLOADPACK, upload_pack); + if (keep) + set_option(transport, TRANS_OPT_KEEP, "yes"); + if (depth) + set_option(transport, TRANS_OPT_DEPTH, depth); + return transport; +} + static int do_fetch(struct transport *transport, struct refspec *refs, int ref_count) { @@ -816,17 +841,6 @@ static int do_fetch(struct transport *transport, return retcode; } -static void set_option(const char *name, const char *value) -{ - int r = transport_set_option(gtransport, name, value); - if (r < 0) - die(_("Option \"%s\" value \"%s\" is not valid for %s"), - name, value, gtransport->url); - if (r > 0) - warning(_("Option \"%s\" is ignored for %s\n"), - name, gtransport->url); -} - static int get_one_remote_for_fetch(struct remote *remote, void *priv) { struct string_list *list = priv; @@ -949,15 +963,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) die(_("No remote repository specified. Please, specify either a URL or a\n" "remote name from which new revisions should be fetched.")); - gtransport = transport_get(remote, NULL); - transport_set_verbosity(gtransport, verbosity, progress); - if (upload_pack) - set_option(TRANS_OPT_UPLOADPACK, upload_pack); - if (keep) - set_option(TRANS_OPT_KEEP, "yes"); - if (depth) - set_option(TRANS_OPT_DEPTH, depth); - + gtransport = prepare_transport(remote); if (argc > 0) { int j = 0; refs = xcalloc(argc + 1, sizeof(const char *)); -- 1.8.4-rc1-210-gf6d87e2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] work around "transport-take-over" hack
"git fetch" sometimes needs to make a real request to the transport after a single "fetch_refs" request, in order to follow tags that the other end should have sent as part of the primary transfer with "follow-tags" request. However, a transport that defines "connect" has a gross hack that destroys its internal state to render it unusable after processing a single request, breaking this. This is my attempt to work it around. I am not too proud of it, but after trying two other approaches to fix it closer to the real cause (i.e. the implementation of Git-aware transport helper) and seeing that both of them ended up being even less appealing and not worth posting, I think this is probably the best fix to the issue. Junio C Hamano (5): t5802: add test for connect helper fetch: rename file-scope global "transport" to "gtransport" fetch: refactor code that prepares a transport fetch: refactor code that fetches leftover tags fetch: work around "transport-take-over" hack builtin/fetch.c | 85 ++- t/t5802-connect-helper.sh | 72 +++ transport.c | 2 ++ transport.h | 6 4 files changed, 134 insertions(+), 31 deletions(-) create mode 100755 t/t5802-connect-helper.sh -- 1.8.4-rc1-210-gf6d87e2 -- 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/5] t5802: add test for connect helper
This is an attempt to reproduce a problem reported for a third-party custom "connect" remote helper. The conjecture is that sometimes "git fetch" wants to make two connections (one for the primary transfer with 'follow-tags' option set, and then after noticing that some tags are not packed because the primary transfer did not have to send any commit that is pointed by them, another to explicitly ask for the missing tags), and their "connect" helper is not called in the second request, breaking the "fetch" as a whole. Unfortunately this test script does not trigger the alleged failure and happily passes when talking to upload-pack from git-core (see patch 5/5 for details). Signed-off-by: Junio C Hamano --- t/t5802-connect-helper.sh | 72 +++ 1 file changed, 72 insertions(+) create mode 100755 t/t5802-connect-helper.sh diff --git a/t/t5802-connect-helper.sh b/t/t5802-connect-helper.sh new file mode 100755 index 000..878faf2 --- /dev/null +++ b/t/t5802-connect-helper.sh @@ -0,0 +1,72 @@ +#!/bin/sh + +test_description='ext::cmd remote "connect" helper' +. ./test-lib.sh + +test_expect_success setup ' + test_tick && + git commit --allow-empty -m initial && + test_tick && + git commit --allow-empty -m second && + test_tick && + git commit --allow-empty -m third && + test_tick && + git tag -a -m "tip three" three && + + test_tick && + git commit --allow-empty -m fourth +' + +test_expect_success clone ' + cmd=$(echo "echo >&2 ext::sh invoked && %S .." | sed -e "s/ /% /g") && + git clone "ext::sh -c %S% ." dst && + git for-each-ref refs/heads/ refs/tags/ >expect && + ( + cd dst && + git config remote.origin.url "ext::sh -c $cmd" && + git for-each-ref refs/heads/ refs/tags/ + ) >actual && + test_cmp expect actual +' + +test_expect_success 'update following tag' ' + test_tick && + git commit --allow-empty -m fifth && + test_tick && + git tag -a -m "tip five" five && + git for-each-ref refs/heads/ refs/tags/ >expect && + ( + cd dst && + git pull && + git for-each-ref refs/heads/ refs/tags/ >../actual + ) && + test_cmp expect actual +' + +test_expect_success 'update backfilled tag' ' + test_tick && + git commit --allow-empty -m sixth && + test_tick && + git tag -a -m "tip two" two three^1 && + git for-each-ref refs/heads/ refs/tags/ >expect && + ( + cd dst && + git pull && + git for-each-ref refs/heads/ refs/tags/ >../actual + ) && + test_cmp expect actual +' + +test_expect_success 'update backfilled tag without primary transfer' ' + test_tick && + git tag -a -m "tip one " one two^1 && + git for-each-ref refs/heads/ refs/tags/ >expect && + ( + cd dst && + git pull && + git for-each-ref refs/heads/ refs/tags/ >../actual + ) && + test_cmp expect actual +' + +test_done -- 1.8.4-rc1-210-gf6d87e2 -- 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
gitweb, How to script alias to a directory? for cloning over http
Hello, I need some guide that can explain me the following: I wish to know the correct rule to make work cloning over http with my configuration, without taking all the web server to server only as github I setup gitweb to it can show in a directory by example, localhost/gitweb or 192.168.1.20/gitweb ; and works fine, including rewrite rule Config file under conf.d from apache config files: Alias /gitweb /usr/share/gitweb SetEnv GITWEB_CONFIG /etc/gitweb.conf SetEnv GIT_PROJECT_ROOT /var/git SetEnv GIT_HTTP_EXPORT_ALL Options ExecCGI FollowSymLinks Indexes AddHandler cgi-script .cgi Allow from all Order allow,deny DirectoryIndex index.cgi # Pretty gitweb URLs need rewrite engine on an enabled RewriteEngine on # rule condition indicates get filenames RewriteCond %{REQUEST_FILENAME} !-f # rule condition indicates get listing directories RewriteCond %{REQUEST_FILENAME} !-d # rule condition to show pretty short urls RewriteRule ^.* /gitweb/gitweb.cgi/$0 [L,PT] -- Fénix -- 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: Remove old forgotten command: whatchanged
On 08/07/2013 07:50 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> I'd deprecate it first for a year or such and remove it then. >> In the meantime we could implement already remove the code >> and change it to: >> >> + int cmd_whatchanged(int argc, const char **argv, const char *prefix) >> + { >> +return cmd_log(argc, argv, prefix) >> + } >> >> Also we should make sure everything git whatchanged can do, >> can easily be done with git log . > > That's even worse than "deprecating". > > Your first step already changes the behaviour for people who really > use the command, without telling them _why_ the behaviour has > suddenly changed at all, while not helping *ANYBODY*. > > The only thing it does is to scratch an irrelevant itch by people > who peek the codebase and find an old command whose existence does > not even hurt them. They may have too much time on their hand, but > that is not an excuse to waste other people's time by adding extra > makework on our plate, or changing the behaviour for people who use > the command without explanation. > > Feeling irritated somewhat... > Well if we make sure the whatchanged command can easily be reproduced with the log command, we could add the missing parameters to it, hence no change for the user. (git whatchanged == git log --raw --no-merges or git log --wc [to be done yet]). So I did not mean to introduce a change for users! But in general I like the idea to *remove* lines of code. Stefan signature.asc Description: OpenPGP digital signature
Re: Repo with only one file
On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt wrote: > Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of >> these scripts and I'd like to keep the commit history. >> >> Ok, so: >> % find -type f ! -iname "webban.pl" | while read f; do git >> filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f" >> HEAD ; done >> >> Which basically did it. But, I've got this one commit that seems to be >> orphaned - it doesn't change any files. > > Try this: > > git filter-branch HEAD -- webban.pl > % git filter-branch HEAD -- webban.pl Cannot create a new backup. A previous backup already exists in refs/original/ Force overwriting the backup with -f % git filter-branch -f HEAD -- webban.pl Rewrite 1e04b18c256c996312f167be808733bcc755f1e3 (9/9) WARNING: Ref 'refs/heads/master' is unchanged -- 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: Remove old forgotten command: whatchanged
On Aug 7, 2013, at 11:31, John Keeping wrote: On Wed, Aug 07, 2013 at 11:01:57AM -0700, Kyle J. McKay wrote: On Aug 7, 2013, at 09:00, Ramkumar Ramachandra wrote: Hi, This is the difference between whatchanged and log: diff --git a/whatchanged b/log index fa1b223..004d9aa 100644 --- a/tmp/whatchanged +++ b/tmp/log @@ -1,4 +1,4 @@ -int cmd_whatchanged(int argc, const char **argv, const char *prefix) +int cmd_log(int argc, const char **argv, const char *prefix) { struct rev_info rev; struct setup_revision_opt opt; @@ -7,13 +7,10 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix) git_config(git_log_config, NULL); init_revisions(&rev, prefix); - rev.diff = 1; - rev.simplify_history = 0; + rev.always_show_header = 1; memset(&opt, 0, sizeof(opt)); opt.def = "HEAD"; opt.revarg_opt = REVARG_COMMITTISH; cmd_log_init(argc, argv, prefix, &rev, &opt); - if (!rev.diffopt.output_format) - rev.diffopt.output_format = DIFF_FORMAT_RAW; return cmd_log_walk(&rev); } Should we remove it? I use it all the time. Is there some log option to get exactly the same output? It doesn't appear that there is. The closest looks like "log --name-status" but it omits the modes and hash values. Is it not identical to "log --raw --no-merges"? A quick test says "mostly", but whatchanged doesn't show empty commits whereas log does seem to; e.g. in git.git: diff -u <(git whatchanged) <(git log --raw --no-merges) That's probably close enough, but if that's all "whatchanged" does is basically be an alias for "log --raw --no-merges" that skips commits with no changes then the help for whatchanged ought to mention that. I still think "git whatchanged" is a lot clearer than "git log --raw -- no-merges" though. -- 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: Remove old forgotten command: whatchanged
On Wed, Aug 07, 2013 at 11:01:57AM -0700, Kyle J. McKay wrote: > On Aug 7, 2013, at 09:00, Ramkumar Ramachandra wrote: > > Hi, > > > > This is the difference between whatchanged and log: > > > > diff --git a/whatchanged b/log > > index fa1b223..004d9aa 100644 > > --- a/tmp/whatchanged > > +++ b/tmp/log > > @@ -1,4 +1,4 @@ > > -int cmd_whatchanged(int argc, const char **argv, const char *prefix) > > +int cmd_log(int argc, const char **argv, const char *prefix) > > { > >struct rev_info rev; > >struct setup_revision_opt opt; > > @@ -7,13 +7,10 @@ int cmd_whatchanged(int argc, const char **argv, > > const char *prefix) > >git_config(git_log_config, NULL); > > > >init_revisions(&rev, prefix); > > - rev.diff = 1; > > - rev.simplify_history = 0; > > + rev.always_show_header = 1; > >memset(&opt, 0, sizeof(opt)); > >opt.def = "HEAD"; > >opt.revarg_opt = REVARG_COMMITTISH; > >cmd_log_init(argc, argv, prefix, &rev, &opt); > > - if (!rev.diffopt.output_format) > > - rev.diffopt.output_format = DIFF_FORMAT_RAW; > >return cmd_log_walk(&rev); > > } > > > > Should we remove it? > > I use it all the time. Is there some log option to get exactly the > same output? It doesn't appear that there is. The closest looks like > "log --name-status" but it omits the modes and hash values. Is it not identical to "log --raw --no-merges"? A quick test says "mostly", but whatchanged doesn't show empty commits whereas log does seem to; e.g. in git.git: diff -u <(git whatchanged) <(git log --raw --no-merges) -- 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 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
On Tue, Aug 06, 2013 at 02:11:56PM -0700, Junio C Hamano wrote: > Thanks, will replace the top two commits and queue. Looks like we > are getting ready for 'next'? I'm a bit curious about if we should move towards a reentrent libgit (which would for example make multithreading easier) or not. If so, I suggest that this patch only use die() in builtin/. However I know that there's a lot of die() all over libgit today, I'm curious about what direction we're heading. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.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: Remove old forgotten command: whatchanged
On Aug 7, 2013, at 09:00, Ramkumar Ramachandra wrote: Hi, This is the difference between whatchanged and log: diff --git a/whatchanged b/log index fa1b223..004d9aa 100644 --- a/tmp/whatchanged +++ b/tmp/log @@ -1,4 +1,4 @@ -int cmd_whatchanged(int argc, const char **argv, const char *prefix) +int cmd_log(int argc, const char **argv, const char *prefix) { struct rev_info rev; struct setup_revision_opt opt; @@ -7,13 +7,10 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix) git_config(git_log_config, NULL); init_revisions(&rev, prefix); - rev.diff = 1; - rev.simplify_history = 0; + rev.always_show_header = 1; memset(&opt, 0, sizeof(opt)); opt.def = "HEAD"; opt.revarg_opt = REVARG_COMMITTISH; cmd_log_init(argc, argv, prefix, &rev, &opt); - if (!rev.diffopt.output_format) - rev.diffopt.output_format = DIFF_FORMAT_RAW; return cmd_log_walk(&rev); } Should we remove it? I use it all the time. Is there some log option to get exactly the same output? It doesn't appear that there is. The closest looks like "log --name-status" but it omits the modes and hash values. -- 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: Remove old forgotten command: whatchanged
Stefan Beller writes: > I'd deprecate it first for a year or such and remove it then. > In the meantime we could implement already remove the code > and change it to: > > + int cmd_whatchanged(int argc, const char **argv, const char *prefix) > + { > + return cmd_log(argc, argv, prefix) > + } > > Also we should make sure everything git whatchanged can do, > can easily be done with git log . That's even worse than "deprecating". Your first step already changes the behaviour for people who really use the command, without telling them _why_ the behaviour has suddenly changed at all, while not helping *ANYBODY*. The only thing it does is to scratch an irrelevant itch by people who peek the codebase and find an old command whose existence does not even hurt them. They may have too much time on their hand, but that is not an excuse to waste other people's time by adding extra makework on our plate, or changing the behaviour for people who use the command without explanation. Feeling irritated somewhat... -- 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 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Am 06.08.2013 23:11, schrieb Junio C Hamano: > Thanks, will replace the top two commits and queue. Looks like we > are getting ready for 'next'? I hope so, I'm not aware of any open issues. -- 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: Remove old forgotten command: whatchanged
On 08/07/2013 06:00 PM, Ramkumar Ramachandra wrote: > Hi, > > This is the difference between whatchanged and log: > > diff --git a/whatchanged b/log > index fa1b223..004d9aa 100644 > --- a/tmp/whatchanged > +++ b/tmp/log > @@ -1,4 +1,4 @@ > -int cmd_whatchanged(int argc, const char **argv, const char *prefix) > +int cmd_log(int argc, const char **argv, const char *prefix) > { > struct rev_info rev; > struct setup_revision_opt opt; > @@ -7,13 +7,10 @@ int cmd_whatchanged(int argc, const char **argv, > const char *prefix) > git_config(git_log_config, NULL); > > init_revisions(&rev, prefix); > - rev.diff = 1; > - rev.simplify_history = 0; > + rev.always_show_header = 1; > memset(&opt, 0, sizeof(opt)); > opt.def = "HEAD"; > opt.revarg_opt = REVARG_COMMITTISH; > cmd_log_init(argc, argv, prefix, &rev, &opt); > - if (!rev.diffopt.output_format) > - rev.diffopt.output_format = DIFF_FORMAT_RAW; > return cmd_log_walk(&rev); > } > > Should we remove it? I'd deprecate it first for a year or such and remove it then. In the meantime we could implement already remove the code and change it to: + int cmd_whatchanged(int argc, const char **argv, const char *prefix) + { + return cmd_log(argc, argv, prefix) + } Also we should make sure everything git whatchanged can do, can easily be done with git log . Stefan signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/4] Build in git-repack
On 08/07/2013 05:48 PM, Junio C Hamano wrote: > Matthieu Moy writes: >> >> seems overkill to me: why don't you just let cmd_repack call >> update_server_info(0)? > > My feeling exactly. I would rather see a patch that does not touch > pack-objects at all, and use run_command() interface to spawn it. > Once we do have to pack, the necessary processing cycle will dwarf > the fork/exec latency anyway, no? > Thanks for clarification. That was my initial idea as well, to not touch the pack-objects logic. However Duy send that patch (basically as is, I just made it apply again), and I guessed that I'd get to results faster with an already existing approach. I will reconsider and try to remove the additional logic from pack-objects again (so it will not get touched) and move it to the repack itself. It is just a way to understand, 'what needs to be done'. Stefan signature.asc Description: OpenPGP digital signature
Re: [RFC] status: show tracking branch even no difference
2013/8/7 Matthieu Moy : > Jiang Xin writes: > >> With this patch, "git status" will report relationship between current >> branch and its upstream counterpart even if there is no difference. >> >> $ git status >> # On branch master >> # Your branch is identical to its tracking branch: 'origin/master'. > > Why not, but we try to say "remote-tracking branch" instead of just > "tracking". Adding "remote-" in your wording may make the line a bit > long, but it may be sufficient to say > > # Your branch is identical to 'origin/master' That's better. Thanks. > > That's consistant with other messages like > > # Your branch is ahead of '%s' by %d commits > > (And this would deserve a test) Will add some test cases in t6040 if this patch has value. -- Jiang Xin -- 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
Remove old forgotten command: whatchanged
Hi, This is the difference between whatchanged and log: diff --git a/whatchanged b/log index fa1b223..004d9aa 100644 --- a/tmp/whatchanged +++ b/tmp/log @@ -1,4 +1,4 @@ -int cmd_whatchanged(int argc, const char **argv, const char *prefix) +int cmd_log(int argc, const char **argv, const char *prefix) { struct rev_info rev; struct setup_revision_opt opt; @@ -7,13 +7,10 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix) git_config(git_log_config, NULL); init_revisions(&rev, prefix); - rev.diff = 1; - rev.simplify_history = 0; + rev.always_show_header = 1; memset(&opt, 0, sizeof(opt)); opt.def = "HEAD"; opt.revarg_opt = REVARG_COMMITTISH; cmd_log_init(argc, argv, prefix, &rev, &opt); - if (!rev.diffopt.output_format) - rev.diffopt.output_format = DIFF_FORMAT_RAW; return cmd_log_walk(&rev); } Should we remove it? Ram -- 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 log --follow filename" (with diff.renameLimit=0) not working after subtree add
Problem: "git log --follow filename" is not working for files after a big move. Scenario: Just performed a "git subtree add" where big projects were moved into another repository, and I'm not able to view history for a single file. Sample output: Output after subtree add (no output) > git log --follow --oneline file.py Output from same command in old repository (the one that was merged with subtree, this is the output i expect to continue having atfer a merge) > git log --follow --oneline file.py cefa32b Convert doc strings to Sphinx 748b2dd Added target for C++ tests 1f2b52e Using default git_source_server for everything after moving to Stash ... [more commits] A couple of notes: - Made sure I used --follow - Using options diff.renameLimit=0 and diff.renames=true - Tried options -C and -M (even though there were no changes, should be a 100% rename match) - "git subtree add" was made without --squash to keep history - "git blame file.py" works fine, detects all changes and rename history - Commits added after the merge are tracked, but log stops at merge - Tested with git 1.8.3.4 - This guy in StackOverflow seems to have the same problem: http://stackoverflow.com/questions/4393527/why-might-git-log-not-show-history-for-a-moved-file-and-what-can-i-do-about-it Relevant output for "git config -l" in repository: core.symlinks=false core.autocrlf=false color.diff=auto color.status=auto color.branch=auto color.interactive=true pack.packsizelimit=2g help.format=html http.sslcainfo=/bin/curl-ca-bundle.crt sendemail.smtpserver=/bin/msmtp.exe diff.astextplain.textconv=astextplain rebase.autosquash=true core.packedgitlimit=128m diff.renamelimit=0 diff.renames=true core.repositoryformatversion=0 core.filemode=false core.bare=false core.logallrefupdates=true core.symlinks=false core.ignorecase=true core.hidedotfiles=dotGitOnly -- Diogo Campos -- 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: [RFC] status: show tracking branch even no difference
Jiang Xin writes: > With this patch, "git status" will report relationship between current > branch and its upstream counterpart even if there is no difference. > > $ git status > # On branch master > # Your branch is identical to its tracking branch: 'origin/master'. Why not, but we try to say "remote-tracking branch" instead of just "tracking". Adding "remote-" in your wording may make the line a bit long, but it may be sufficient to say # Your branch is identical to 'origin/master' That's consistant with other messages like # Your branch is ahead of '%s' by %d commits (And this would deserve a test) -- 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: [PATCH 1/4] Build in git-repack
Matthieu Moy writes: > [ It's cool you're working on this, I'd really like a git-repack in C. > That would fix this > http://thread.gmane.org/gmane.comp.version-control.git/226458 ] > > Stefan Beller writes: > >> From: Nguyễn Thái Ngọc Duy >> >> pack-objects learns a few more options to take over what's been done >> by git-repack.sh. cmd_repack() becomes a wrapper around >> cmd_pack_objects(). > > I think the patch would read easier if these were split into two > patches: one doing the real stuff in pack-objects, and then getting rid > of git-repack.sh to replace it with a trivial built-in. > > Actually, I'm wondering why pack-objects requires so much changes. > git-repack.sh was already a relatively small wrapper around > pack-objects, and did not need the new options you add, so why are they > needed? In particular adding the new --update-info option that just does > >> +if (repack_flags & REPACK_UPDATE_INFO) >> +update_server_info(0); > > seems overkill to me: why don't you just let cmd_repack call > update_server_info(0)? My feeling exactly. I would rather see a patch that does not touch pack-objects at all, and use run_command() interface to spawn it. Once we do have to pack, the necessary processing cycle will dwarf the fork/exec latency anyway, no? -- 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
[RFC] status: show tracking branch even no difference
If the current branch has an upstream branch, and there are differences between the current branch and its upstream, some commands (such as "git status", "git status -bs", and "git checkout") will report their relationship. E.g. $ git status # On branch master # Your branch is ahead of 'origin/master' by 1 commit. # (use "git push" to publish your local commits) # ... $ git status -bs ## master...origin/master [ahead 1] ... $ git checkout master Already on 'master' Your branch is ahead of 'origin/master' by 1 commit. (use "git push" to publish your local commits) But if there is no difference between the current branch and its upstream, the relationship will not be reported, and it's hard to tell whether the current branch has a tracking branch or not. And what's worse, when the 'push.default' config variable is set to `matching`, it's hard to tell whether current branch is pushed out or not [1]. With this patch, "git status" will report relationship between current branch and its upstream counterpart even if there is no difference. $ git status # On branch master # Your branch is identical to its tracking branch: 'origin/master'. # ... $ git status -bs ## master...origin/master ... $ git checkout master Already on 'master' Your branch is identical to its tracking branch: 'origin/master'. [1]: http://thread.gmane.org/gmane.comp.version-control.git/198703 Signed-off-by: Jiang Xin --- remote.c| 22 -- wt-status.c | 13 ++--- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/remote.c b/remote.c index 2433467..8d6f278 100644 --- a/remote.c +++ b/remote.c @@ -1740,6 +1740,10 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) const char *rev_argv[10], *base; int rev_argc; + /* Set both num_theirs and num_ours as undetermined. */ + *num_theirs = -1; + *num_ours = -1; + /* * Nothing to report unless we are marked to build on top of * somebody else. @@ -1758,14 +1762,16 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) theirs = lookup_commit_reference(sha1); if (!theirs) return 0; + *num_theirs = 0; if (read_ref(branch->refname, sha1)) return 0; ours = lookup_commit_reference(sha1); if (!ours) return 0; + *num_ours = 0; - /* are we the same? */ + /* are we the same? both num_theirs and num_ours are set to 0. */ if (theirs == ours) return 0; @@ -1786,8 +1792,6 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) prepare_revision_walk(&revs); /* ... and count the commits on each side. */ - *num_ours = 0; - *num_theirs = 0; while (1) { struct commit *c = get_revision(&revs); if (!c) @@ -1812,12 +1816,18 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb) int num_ours, num_theirs; const char *base; - if (!stat_tracking_info(branch, &num_ours, &num_theirs)) - return 0; + if (!stat_tracking_info(branch, &num_ours, &num_theirs)) { + if (num_ours || num_theirs) + return 0; + } base = branch->merge[0]->dst; base = shorten_unambiguous_ref(base, 0); - if (!num_theirs) { + if (!num_ours && !num_theirs) { + strbuf_addf(sb, + _("Your branch is identical to its tracking branch: '%s'.\n"), + base); + } else if (!num_theirs) { strbuf_addf(sb, Q_("Your branch is ahead of '%s' by %d commit.\n", "Your branch is ahead of '%s' by %d commits.\n", diff --git a/wt-status.c b/wt-status.c index ff4b324..56f3c19 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1381,9 +1381,11 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) if (s->is_initial) color_fprintf(s->fp, header_color, _("Initial commit on ")); if (!stat_tracking_info(branch, &num_ours, &num_theirs)) { - color_fprintf(s->fp, branch_color_local, "%s", branch_name); - fputc(s->null_termination ? '\0' : '\n', s->fp); - return; + if (num_ours || num_theirs) { + color_fprintf(s->fp, branch_color_local, "%s", branch_name); + fputc(s->null_termination ? '\0' : '\n', s->fp); + return; + } } base = branch->merge[0]->dst; @@ -1392,6 +1394,11 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) color_fprintf(s->fp, header_color, "..."); color_fprintf(s->fp, branch_color_remote, "%s", base);
Re: [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP
Stefan Beller writes: > On 08/07/2013 01:31 AM, Junio C Hamano wrote: >> The parseopt parsing for OPT__FORCE() is implemented in terms of >> OPT_BOOLEAN() and users of it can take advantage of the "counting >> up" behaviour to implement increasing levels of forcefulness by >> differentiating "git cmd -f" and "git cmd -f -f". >> >> Clarify this by explicitly using OPT_COUNTUP() instead. >> >> Signed-off-by: Junio C Hamano >> --- >> >> * This _should_ be done with a similar audit of existing callers, >>but I ran out of concentration. >> >> parse-options.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/parse-options.h b/parse-options.h >> index 78f52c2..1eeb0d9 100644 >> --- a/parse-options.h >> +++ b/parse-options.h >> @@ -238,7 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, >> const char *, int); >> { OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \ >>PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 } >> #define OPT__DRY_RUN(var, h) OPT_BOOL('n', "dry-run", (var), (h)) >> -#define OPT__FORCE(var, h)OPT_BOOLEAN('f', "force", (var), (h)) >> +#define OPT__FORCE(var, h)OPT_COUNTUP('f', "force", (var), (h)) >> #define OPT__ABBREV(var) \ >> { OPTION_CALLBACK, 0, "abbrev", (var), N_("n"), \ >>N_("use digits to display SHA-1s"), \ >> > > We need the COUNTUP, because in builtin/clean.c we have > OPT__FORCE(&force, N_("force")), > ... > if (force > 1) > rm_flags = 0; Good that I marked it as RFH ;-) Thanks. > So a OPT_BOOL definitely doesn't cut it. > Now that I started reviewing the OPT_FORCE parts, I realize > there is still an error in the patch, which needed correction. > (branch, commit, name-rev: ease up boolean conditions): > > - if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + > !!unset_upstream > 1) > + if (force_create + list + unset_upstream + > + !!delete + !!rename + !!new_upstream > 1) > usage_with_options(builtin_branch_usage, options); > > force_create is set via OPT_FORCE as well, so we cannot remove the !! before > the force_create, > hence we'd only remove it from list and unset_upstream, which are set by > OPT_BOOL. Good. Will replace. Thanks. > -- 8< -- > From: Stefan Beller > Date: Wed, 7 Aug 2013 09:32:25 +0200 > Subject: [PATCH] branch, commit, name-rev: ease up boolean conditions > > Now that the variables are set by OPT_BOOL, which makes sure > to have the values being 0 or 1 after parsing, we do not need > the double negation to map any other value to 1 for integer > variables. > > Signed-off-by: Stefan Beller > Signed-off-by: Junio C Hamano > --- > builtin/branch.c | 3 ++- > builtin/commit.c | 2 +- > builtin/name-rev.c | 2 +- > 3 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/builtin/branch.c b/builtin/branch.c > index 4daed0b..0903763 100644 > --- a/builtin/branch.c > +++ b/builtin/branch.c > @@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char > *prefix) > if (with_commit || merge_filter != NO_FILTER) > list = 1; > > - if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + > !!unset_upstream > 1) > + if (!!delete + !!rename + !!force_create + !!new_upstream + > + list + unset_upstream > 1) > usage_with_options(builtin_branch_usage, options); > > if (abbrev == -1) > diff --git a/builtin/commit.c b/builtin/commit.c > index c20426b..b0f86c8 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const > char *argv[], > if (patch_interactive) > interactive = 1; > > - if (!!also + !!only + !!all + !!interactive > 1) > + if (also + only + all + interactive > 1) > die(_("Only one of --include/--only/--all/--interactive/--patch > can be used.")); > if (argc == 0 && (also || (only && !amend))) > die(_("No paths with --include/--only does not make sense.")); > diff --git a/builtin/name-rev.c b/builtin/name-rev.c > index a908a34..20fcf8c 100644 > --- a/builtin/name-rev.c > +++ b/builtin/name-rev.c > @@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char > *prefix) > > git_config(git_default_config, NULL); > argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0); > - if (!!all + !!transform_stdin + !!argc > 1) { > + if (all + transform_stdin + !!argc > 1) { > error("Specify either a list, or --all, not both!"); > usage_with_options(name_rev_usage, opts); > } -- 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/4] Build in git-repack
[ It's cool you're working on this, I'd really like a git-repack in C. That would fix this http://thread.gmane.org/gmane.comp.version-control.git/226458 ] Stefan Beller writes: > From: Nguyễn Thái Ngọc Duy > > pack-objects learns a few more options to take over what's been done > by git-repack.sh. cmd_repack() becomes a wrapper around > cmd_pack_objects(). I think the patch would read easier if these were split into two patches: one doing the real stuff in pack-objects, and then getting rid of git-repack.sh to replace it with a trivial built-in. Actually, I'm wondering why pack-objects requires so much changes. git-repack.sh was already a relatively small wrapper around pack-objects, and did not need the new options you add, so why are they needed? In particular adding the new --update-info option that just does > + if (repack_flags & REPACK_UPDATE_INFO) > + update_server_info(0); seems overkill to me: why don't you just let cmd_repack call update_server_info(0)? -- 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
[PATCH 0/4] Re: Rewriting git-repack.sh in C
Hi Duy, I applied your patch on the current master and added 3 patches, so git compiles and the testsuite works without additional breakages. The functionality is obviously not yet completed as the backup_file function is still empty. What do you intend that function will do? Stefan Nguyễn Thái Ngọc Duy (1): Build in git-repack Stefan Beller (3): backup_file dummy function pack-objects: do not print usage when repacking repack: add unpack-unreachable Makefile | 2 +- builtin.h | 1 + builtin/pack-objects.c | 284 - bulk-checkin.c | 2 +- contrib/examples/git-repack.sh | 194 git-repack.sh | 194 git.c | 1 + pack-write.c | 18 ++- pack.h | 4 +- 9 files changed, 497 insertions(+), 203 deletions(-) create mode 100755 contrib/examples/git-repack.sh delete mode 100755 git-repack.sh -- 1.8.4.rc1.25.g2793d0a -- 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/4] Build in git-repack
From: Nguyễn Thái Ngọc Duy pack-objects learns a few more options to take over what's been done by git-repack.sh. cmd_repack() becomes a wrapper around cmd_pack_objects(). --- Makefile | 2 +- builtin.h | 1 + builtin/pack-objects.c | 279 - bulk-checkin.c | 2 +- contrib/examples/git-repack.sh | 194 git-repack.sh | 194 git.c | 1 + pack-write.c | 13 +- pack.h | 4 +- 9 files changed, 488 insertions(+), 202 deletions(-) create mode 100755 contrib/examples/git-repack.sh delete mode 100755 git-repack.sh diff --git a/Makefile b/Makefile index 3588ca1..8bd122b 100644 --- a/Makefile +++ b/Makefile @@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh SCRIPT_SH += git-quiltimport.sh SCRIPT_SH += git-rebase.sh SCRIPT_SH += git-remote-testgit.sh -SCRIPT_SH += git-repack.sh SCRIPT_SH += git-request-pull.sh SCRIPT_SH += git-stash.sh SCRIPT_SH += git-submodule.sh @@ -596,6 +595,7 @@ BUILT_INS += git-get-tar-commit-id$X BUILT_INS += git-init$X BUILT_INS += git-merge-subtree$X BUILT_INS += git-peek-remote$X +BUILT_INS += git-repack$X BUILT_INS += git-repo-config$X BUILT_INS += git-show$X BUILT_INS += git-stage$X diff --git a/builtin.h b/builtin.h index 8afa2de..b56cf07 100644 --- a/builtin.h +++ b/builtin.h @@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix); extern int cmd_remote(int argc, const char **argv, const char *prefix); extern int cmd_remote_ext(int argc, const char **argv, const char *prefix); extern int cmd_remote_fd(int argc, const char **argv, const char *prefix); +extern int cmd_repack(int argc, const char **argv, const char *prefix); extern int cmd_repo_config(int argc, const char **argv, const char *prefix); extern int cmd_rerere(int argc, const char **argv, const char *prefix); extern int cmd_reset(int argc, const char **argv, const char *prefix); diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index f069462..1742ea1 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -18,10 +18,17 @@ #include "refs.h" #include "streaming.h" #include "thread-utils.h" +#include "sigchain.h" static const char *pack_usage[] = { N_("git pack-objects --stdout [options...] [< ref-list | < object-list]"), N_("git pack-objects [options...] base-name [< ref-list | < object-list]"), + N_("git pack-objects --repack [options...]"), + NULL +}; + +static char const * const repack_usage[] = { + N_("git repack [options]"), NULL }; @@ -103,6 +110,15 @@ static struct object_entry *locate_object_entry(const unsigned char *sha1); static uint32_t written, written_delta; static uint32_t reused, reused_delta; +#define REPACK_IN_PROGRESS (1 << 0) +#define REPACK_UPDATE_INFO (1 << 1) +#define REPACK_ALL_INTO_ONE(1 << 2) +#define REPACK_REMOVE_REDUNDANT(1 << 3) + +static int repack_flags, nr_written_packs; +static int repack_usedeltabaseoffset; +static struct string_list written_packs; +static struct string_list backup_files; static void *get_delta(struct object_entry *entry) { @@ -792,9 +808,19 @@ static void write_pack_file(void) snprintf(tmpname, sizeof(tmpname), "%s-", base_name); finish_tmp_packfile(tmpname, pack_tmp_name, written_list, nr_written, - &pack_idx_opts, sha1); + &pack_idx_opts, sha1, + repack_flags & REPACK_IN_PROGRESS ? + &backup_files : NULL); free(pack_tmp_name); - puts(sha1_to_hex(sha1)); + if (repack_flags & REPACK_IN_PROGRESS) { + int len = strlen(tmpname); + char *s = xmalloc(len + 2); + memcpy(s, tmpname, len - 4); + memcpy(s + len - 4, ".pack", 6); + string_list_append(&written_packs, s); + nr_written_packs++; + } else + puts(sha1_to_hex(sha1)); } /* mark written objects as written to previous pack */ @@ -2359,7 +2385,8 @@ static void get_object_list(int ac, const char **av) save_commit_buffer = 0; setup_revisions(ac, av, &revs, NULL); - while (fgets(line, sizeof(line), stdin) != NULL) { + while (!(repack_flags & REPACK_IN_PROGRESS) && + fgets(line, sizeof(line), stdin) != NULL) { int len = strlen(line); if (
[PATCH 3/4] pack-objects: do not print usage when repacking
--- builtin/pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1742ea1..0bd8f3b 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2585,7 +2585,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) base_name = argv[0]; argc--; } - if (pack_to_stdout != !base_name || argc) + if (!(repack_flags & REPACK_IN_PROGRESS) && (pack_to_stdout != !base_name || argc)) usage_with_options(pack_usage, pack_objects_options); rp_av[rp_ac++] = "pack-objects"; -- 1.8.4.rc1.25.g2793d0a -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] repack: add unpack-unreachable
--- builtin/pack-objects.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 0bd8f3b..0fe01fc 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2795,6 +2795,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix) OPT_BOOL('q', NULL, &quiet, "be quiet"), OPT_BOOL('l', NULL, &local, "pass --local to git-pack-objects"), + { OPTION_CALLBACK, 0, "unpack-unreachable", NULL, N_("time"), + N_("unpack unreachable objects newer than "), + PARSE_OPT_OPTARG, option_parse_unpack_unreachable }, { OPTION_ARGUMENT, 0, "window", NULL, "n", "size of the window used for delta compression", 0 }, { OPTION_ARGUMENT, 0, "window-memory", NULL, "n", -- 1.8.4.rc1.25.g2793d0a -- 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/4] backup_file dummy function
--- pack-write.c | 5 + 1 file changed, 5 insertions(+) diff --git a/pack-write.c b/pack-write.c index e6aa7e3..b728ea2 100644 --- a/pack-write.c +++ b/pack-write.c @@ -344,6 +344,11 @@ struct sha1file *create_tmp_packfile(char **pack_tmp_name) return sha1fd(fd, *pack_tmp_name); } +void backup_file(char *filename) +{ + +} + void finish_tmp_packfile(char *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, -- 1.8.4.rc1.25.g2793d0a -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug about Japanese' Documentation
# I am sorry. I forget title and signiture. - Hello. I find 2 bug about Japanese' Documentation . There are Documentation -> Book 's url. ( Please see the details below). I want to bug-fix about this misspell. Do you have Documentation (about Japanese language) on the GitHub? (1) http://git-scm.com/book/ja/Git-%E3%81%AE%E5%9F%BA%E6%9C%AC-%E4%BD%9C%E6%A5%AD%E3%81%AE%E3%82%84%E3%82%8A%E7%9B%B4%E3%81%97 (2) http://git-scm.com/book/ja/Git-%E3%81%AE%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81%E6%A9%9F%E8%83%BD-%E3%83%AA%E3%83%A2%E3%83%BC%E3%83%88%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81 thanks Sekai Kobayashi silentsilent...@gmail.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
[no subject]
Hello. I find 2 bug about Japanese' Documentation . There are Documentation -> Book 's url. ( Please see the details below). I want to bug-fix about this misspell. Do you have Documentation (about Japanese language) on the GitHub? (1) http://git-scm.com/book/ja/Git-%E3%81%AE%E5%9F%BA%E6%9C%AC-%E4%BD%9C%E6%A5%AD%E3%81%AE%E3%82%84%E3%82%8A%E7%9B%B4%E3%81%97 (2) http://git-scm.com/book/ja/Git-%E3%81%AE%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81%E6%A9%9F%E8%83%BD-%E3%83%AA%E3%83%A2%E3%83%BC%E3%83%88%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81 -- 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: [PATCHv3] git-tag man: when to use lightweight or annotated tags
On 07/29/2013 08:02 PM, Daniele Segato wrote: On 07/26/2013 09:36 PM, Jonathan Nieder wrote: Eventually the description section should probably be tweaked to start by explaining what the command is actually for. ;-) Elaborating from this suggestion you gave me I tried to rewrite/rearrange the description moving things around a little. Here's what I've come out with, what do you think about it? DESCRIPTION --- A tag is a non-mutable reference name (in `refs/tags/`) to an object (usually a commit). If one of `-d/-l/-v` options is given the command will delete, list or verify tags. If one of `-a`, `-s`, or `-u ` is passed, the command creates both the reference and a 'tag' object containing a creation date, the tagger name and e-mail, a tag message and an optional GnuPG signature. Unless `-m ` or `-F ` is given, an editor is started for the user to type in the tag message. Otherwise just a tag reference for the SHA-1 object name of the commit object is created (i.e. a lightweight tag). Unless `-f` is given, the named tag must not yet exist. If `-m ` or `-F ` is given and `-a`, `-s`, and `-u ` are absent, `-a` is implied. A GnuPG signed tag object will be created when `-s` or `-u ` is used. When `-u ` is not used, the committer identity for the current user is used to find the GnuPG key for signing. The configuration variable `gpg.program` is used to specify custom GnuPG binary. Tag objects (created with `-a`, `s`, or `-u`) are called "annotated" tags; whereas a "lightweight" tag is simply a name for an object (usually a commit object). Annotated tags are meant for release while lightweight tags are meant for private or temporary object labels. For this reason, some git commands for naming objects (like `git describe`) will ignore lightweight tags by default. I suppose there's no interest in this anymore thanks anyway, Regards, Daniele Segato -- 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: Repo with only one file
Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of > these scripts and I'd like to keep the commit history. > > Ok, so: > % find -type f ! -iname "webban.pl" | while read f; do git > filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f" > HEAD ; done > > Which basically did it. But, I've got this one commit that seems to be > orphaned - it doesn't change any files. Try this: git filter-branch HEAD -- webban.pl -- Hannes -- 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: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects
Hi Junio, I haven't got a reply to my mail yet. Could you have a look, so I can update and resubmit my patch? On Fri, Jul 12, 2013 at 09:11:57AM +0200, Matthijs Kooijman wrote: > > [administrivia: you seem to have mail-followup-to that points at you > > and the list; is that really needed???] > > In your discussion (including the comment), you talk about "shallow > > root" (I think that is the same as what we call "shallow boundary"), > I think so, yes. I mean to refer to the commits referenced in > .git/shallow, that have their parents "hidden". Could you confirm that I got the terms right here (or is the shallow boundary the first hidden commit?) > > but in this added block, there is nothing that checks CLIENT_SHALLOW > > or SHALLOW flags to special case that. > > > > Is it a good idea to unconditionally do this for all "have" > > revisions? > That's what I meant in my mail with "applying the fix unconditionally" - > there is probably some check needed (I discussed a few options in the > mail as well). > > Note that this entire do_rev_list function is only called when there are > shallow revisions involved, so there is also a basic "only when shallow" > check in place. My proposal was to only apply the fix for all have revisions when the previous history traversal came across some shallow boundary commits. If this happens, then that shallow boundary commit will be a "new" one and it will have prevented the history traversal from finding the full list of relevant "have" commits. In this case, we should just use all "have" commits instead. Now, looking at the code, I see a few options for detecting this case: 1 Modify mark_edges_uninteresting to return a boolean (or have an output argument) if any of the commits in the list of commits to find (not the edges) is a shallow boundary. 2 Modify mark_edges_uninteresting to have a "show_shallow" argument that gets called for every shallow boundary. The show_shallow function passed would then simply keep a boolean if it is passed at least once. 3 Add another loop over the commits _after_ the call to mark_edges_uninteresting, that simply looks for any shallow boundary commit. The last option seems sensible to me, since it prevents modifying the somewhat generic mark_edges_uninteresting function for this specific usecase. On the other hand, it does mean that the list of commits is looped twice, not sure what that means for performance. Before I go and implement one of these, which option seems best to you? Gr. Matthijs -- 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] die_with_status: use "printf '%s\n'", not "echo"
Matthieu Moy writes: > Thomas Rast writes: > >>> + grep -v " " error >> >> Umm, doesn't that only test that _some_ line in the error does not >> contain a tab? > > Indeed. It does work as the error message is just a one-liner, but let's > be robust anyway. Err, right. I actually applied the patch and verified that the test "erroneously" passed without the fix, but that's simply because I use bash instead of dash. The error message indeed is only one line, the rest of the rebase output goes to stdout. -- Thomas Rast trast@{inf,student}.ethz.ch -- 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] die_with_status: use "printf '%s\n'", not "echo"
Some implementations of 'echo' (e.g. dash's built-in) interpret backslash sequences in their arguments. This triggered at least one bug: the error message of "rebase -i" was turning \t in commit messages into actual tabulations. There may be others. Using "printf '%s\n'" instead avoids this bad behavior, and is the form used by the "say" function. Noticed-by: David Kastrup Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- Changed the "grep" function to be more accurate. The test still catches the old failure and pass after the fix. git-sh-setup.sh | 2 +- t/t3404-rebase-interactive.sh | 13 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 7a964ad..e15be51 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -53,7 +53,7 @@ die () { die_with_status () { status=$1 shift - echo >&2 "$*" + printf >&2 '%s\n' "$*" exit "$status" } diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 49ccb38..4dbeddb 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -976,4 +976,17 @@ test_expect_success 'rebase -i with --strategy and -X' ' test $(cat file1) = Z ' +test_expect_success 'rebase -i error on commits with \ in message' ' + current_head=$(git rev-parse HEAD) + test_when_finished "git rebase --abort; git reset --hard $current_head; rm -f error" && + test_commit TO-REMOVE will-conflict old-content && + test_commit "\temp" will-conflict new-content dummy && + ( + EDITOR=true && + export EDITOR && + test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error + ) && + test_expect_code 1 grep " emp" error +' + test_done -- 1.8.3.3.797.gb72c616 -- 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] die_with_status: use "printf '%s\n'", not "echo"
Thomas Rast writes: >> +grep -v " " error > > Umm, doesn't that only test that _some_ line in the error does not > contain a tab? Indeed. It does work as the error message is just a one-liner, but let's be robust anyway. > Whereas you need to test that _no_ line contains emp, or some > such. Perhaps as > > ! grep -v " emp" error Err, then the -v should be remove. Also, I'll use test_expect_code 1 instead of ! to catch other grep failures, just in case. Thanks, new patch comming. -- 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: [PATCH] l10n: de.po: translate 5 messages
On Wed, Aug 07, 2013 at 11:17:08AM +0200, Thomas Rast wrote: > This results from Peff's c17592a (commit: tweak empty cherry pick advice > for sequencer, 2013-07-26): > > diff --git a/builtin/commit.c b/builtin/commit.c > index a17a5df..39717d5 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -62,8 +62,18 @@ >"If you wish to commit it anyway, use:\n" >"\n" >"git commit --allow-empty\n" > +"\n"); > + > +static const char empty_cherry_pick_advice_single[] = > +N_("Otherwise, please use 'git reset'\n"); > + > +static const char empty_cherry_pick_advice_multi[] = > +N_("If you wish to skip this commit, use:\n" >"\n" > -"Otherwise, please use 'git reset'\n"); > +"git reset\n" > +"\n" > +"Then \"git cherry-pick --continue\" will resume cherry-picking\n" > +"the remaining commits.\n"); > >static const char *use_message_buffer; >static const char commit_editmsg[] = "COMMIT_EDITMSG"; > > [...] > > So it seems that concatenating sentences indeed falls into a gray area > between "avoid sentence lego" and "split at paragraphs". And Peff's > style of splitting it saves the translators work because the first part > of the message is shared among two code paths. Yeah, I was mainly trying to avoid repeating myself, since the first part of the message would be the same (and I did not want them to fall out of sync). However, I do not mind changing it if translators would prefer to see the whole message as a single string. I agree it's a gray area. -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] l10n: de.po: translate 5 messages
Ralf Thielow writes: > The changes look good to me. The following is purely about the original English messages. > #: builtin/commit.c:62 > -#, fuzzy > msgid "" > "The previous cherry-pick is now empty, possibly due to conflict > resolution.\n" > "If you wish to commit it anyway, use:\n" > "\n" > "git commit --allow-empty\n" > @@ -4014,25 +4012,30 @@ msgstr "" > "Konfliktauflösung.\n" > "Wenn Sie dies trotzdem committen wollen, benutzen Sie:\n" > "\n" > "git commit --allow-empty\n" > "\n" > -"Andernfalls benutzen Sie bitte 'git reset'\n" > > #: builtin/commit.c:69 > msgid "Otherwise, please use 'git reset'\n" > -msgstr "" > +msgstr "Andernfalls benutzen Sie bitte 'git reset'\n" This results from Peff's c17592a (commit: tweak empty cherry pick advice for sequencer, 2013-07-26): diff --git a/builtin/commit.c b/builtin/commit.c index a17a5df..39717d5 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -62,8 +62,18 @@ "If you wish to commit it anyway, use:\n" "\n" "git commit --allow-empty\n" +"\n"); + +static const char empty_cherry_pick_advice_single[] = +N_("Otherwise, please use 'git reset'\n"); + +static const char empty_cherry_pick_advice_multi[] = +N_("If you wish to skip this commit, use:\n" "\n" -"Otherwise, please use 'git reset'\n"); +"git reset\n" +"\n" +"Then \"git cherry-pick --continue\" will resume cherry-picking\n" +"the remaining commits.\n"); static const char *use_message_buffer; static const char commit_editmsg[] = "COMMIT_EDITMSG"; I was wondering for a while if this was a smart move, based on the usual observation that it is better to translate things in one chunk because many languages have more interdependencies than English does. The gettext manual says: puts ("Apollo 13 scenario: Stack overflow handling failed."); puts ("On the next stack overflow we will crash!!!"); Should these two statements merged into a single one? I would recommend to merge them if the two sentences are related to each other, because then it makes it easier for the translator to understand and translate both. On the other hand, if one of the two messages is a stereotypic one, occurring in other places as well, you will do a favour to the translator by not merging the two. [...] Translatable strings should be limited to one paragraph; don't let a single message be longer than ten lines. So it seems that concatenating sentences indeed falls into a gray area between "avoid sentence lego" and "split at paragraphs". And Peff's style of splitting it saves the translators work because the first part of the message is shared among two code paths. -- Thomas Rast trast@{inf,student}.ethz.ch -- 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] die_with_status: use "printf '%s\n'", not "echo"
Matthieu Moy writes: > At least GNU echo interprets backslashes in its arguments. > > This triggered at least one bug: the error message of "rebase -i" was > turning \t in commit messages into actual tabulations. There may be > others. > > Using "printf '%s\n'" instead avoids this bad behavior, and is the form > used by the "say" function. > > Noticed-by: David Kastrup > Signed-off-by: Matthieu Moy [...] > +test_expect_success 'rebase -i error on commits with \ in message' ' > + current_head=$(git rev-parse HEAD) > + test_when_finished "git rebase --abort; git reset --hard $current_head; > rm -f error" && > + test_commit TO-REMOVE will-conflict old-content && > + test_commit "\temp" will-conflict new-content dummy && > + ( > + EDITOR=true && > + export EDITOR && > + test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error > + ) && > + grep -v " " error Umm, doesn't that only test that _some_ line in the error does not contain a tab? Whereas you need to test that _no_ line contains emp, or some such. Perhaps as ! grep -v " emp" error > +' > + > test_done -- Thomas Rast trast@{inf,student}.ethz.ch -- 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] replace: forbid replacing an object with one of a different type
Christian Couder writes: > Users replacing an object with one of a different type were not > prevented to do so, even if it was obvious, and stated in the doc, > that bad things would result from doing that. > > To avoid mistakes, it is better to just forbid that though. > > The doc will be updated in a later patch. > > Signed-off-by: Christian Couder Feel free to steal some of my other email for the commit message, to write down for posterity that reverting would not really be a useful step. The patch looks good to me. > If this patch is considered useful, I will update the doc and > maybe add tests. > > builtin/replace.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/builtin/replace.c b/builtin/replace.c > index 59d3115..0246ab3 100644 > --- a/builtin/replace.c > +++ b/builtin/replace.c > @@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const > char *replace_ref, > int force) > { > unsigned char object[20], prev[20], repl[20]; > + enum object_type obj_type, repl_type; > char ref[PATH_MAX]; > struct ref_lock *lock; > > @@ -100,6 +101,14 @@ static int replace_object(const char *object_ref, const > char *replace_ref, > if (check_refname_format(ref, 0)) > die("'%s' is not a valid ref name.", ref); > > + obj_type = sha1_object_info(object, NULL); > + repl_type = sha1_object_info(repl, NULL); > + if (obj_type != repl_type) > + die("Object ref '%s' is of type '%s'\n" > + "while replace ref '%s' is of type '%s'.", > + object_ref, typename(obj_type), > + replace_ref, typename(repl_type)); > + > if (read_ref(ref, prev)) > hashclr(prev); > else if (!force) -- Thomas Rast trast@{inf,student}.ethz.ch -- 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 12/19] read-cache: read index-v5
Duy Nguyen writes: > A little bit more.. > > On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer > wrote: >> +static void ce_queue_push(struct cache_entry **head, >> +struct cache_entry **tail, >> +struct cache_entry *ce) >> +{ >> + if (!*head) { >> + *head = *tail = ce; >> + (*tail)->next = NULL; >> + return; >> + } >> + >> + (*tail)->next = ce; >> + ce->next = NULL; >> + *tail = (*tail)->next; > > No no no. "next" is for name-hash.c don't "reuse" it here. And I don't > think you really need to maintain a linked list of cache_entries by > the way. More on read_entries comments below.. You're right, I don't need it for the reading code. I do need to keep a list of cache-entries for writing though, where a linked list is best suited for the task. I'll use a next_ce pointer for that. >> +} >> + >> +static struct cache_entry *ce_queue_pop(struct cache_entry **head) >> +{ >> + struct cache_entry *ce; >> + >> + ce = *head; >> + *head = (*head)->next; >> + return ce; >> +} > > Same as ce_queue_push. > >> +static int read_entries(struct index_state *istate, struct directory_entry >> **de, >> + unsigned int *entry_offset, void **mmap, >> + unsigned long mmap_size, unsigned int *nr, >> + unsigned int *foffsetblock) >> +{ >> + struct cache_entry *head = NULL, *tail = NULL; >> + struct conflict_entry *conflict_queue; >> + struct cache_entry *ce; >> + int i; >> + >> + conflict_queue = NULL; >> + if (read_conflicts(&conflict_queue, *de, mmap, mmap_size) < 0) >> + return -1; >> + for (i = 0; i < (*de)->de_nfiles; i++) { >> + if (read_entry(&ce, >> + *de, >> + entry_offset, >> + mmap, >> + mmap_size, >> + foffsetblock) < 0) >> + return -1; >> + ce_queue_push(&head, &tail, ce); >> + *foffsetblock += 4; >> + >> + /* >> +* Add the conflicted entries at the end of the index file >> +* to the in memory format >> +*/ >> + if (conflict_queue && >> + (conflict_queue->entries->flags & CONFLICT_CONFLICTED) >> != 0 && >> + !cache_name_compare(conflict_queue->name, >> conflict_queue->namelen, >> + ce->name, ce_namelen(ce))) { >> + struct conflict_part *cp; >> + cp = conflict_queue->entries; >> + cp = cp->next; >> + while (cp) { >> + ce = convert_conflict_part(cp, >> + conflict_queue->name, >> + conflict_queue->namelen); >> + ce_queue_push(&head, &tail, ce); >> + conflict_part_head_remove(&cp); >> + } >> + conflict_entry_head_remove(&conflict_queue); >> + } > > I start to wonder if separating staged entries is a good idea. It > seems to make the code more complicated. The good point about conflict > section at the end of the file is you can just truncate() it out. > Another way is putting staged entries in fileentries, sorted > alphabetically then by stage number, and a flag indicating if the > entry is valid. When you remove resolve an entry, just set the flag to > invalid (partial write), so that read code will skip it. > > I think this approach is reasonably cheap (unless there are a lot of > conflicts) and it simplifies this piece of code. truncate() may be > overrated anyway. In my experience, I "git add " as soon as I > resolve (so that "git diff" shrinks). One entry at a time, one > index write at a time. I don't think I ever resolve everything then > "git add -u .", which is where truncate() shines because staged > entries are removed all at once. We should optimize for one file > resolution at a time, imo. > >> + } >> + >> + *de = (*de)->next; >> + >> + while (head) { >> + if (*de != NULL >> + && strcmp(head->name, (*de)->pathname) > 0) { >> + read_entries(istate, >> +de, >> +entry_offset, >> +mmap, >> +mmap_size, >> +nr, >> +foffsetblock); >> + } else { >> + ce = ce_queue_pop(&head); >> + set_index_entry(istate, *nr, ce); >> +
Re: [PATCH v2 12/19] read-cache: read index-v5
Duy Nguyen writes: > On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer > wrote: >> +struct directory_entry { >> + struct directory_entry *next; >> + struct directory_entry *next_hash; >> + struct cache_entry *ce; >> + struct cache_entry *ce_last; >> + struct conflict_entry *conflict; >> + struct conflict_entry *conflict_last; >> + unsigned int conflict_size; >> + unsigned int de_foffset; >> + unsigned int de_cr; >> + unsigned int de_ncr; >> + unsigned int de_nsubtrees; >> + unsigned int de_nfiles; >> + unsigned int de_nentries; >> + unsigned char sha1[20]; >> + unsigned short de_flags; >> + unsigned int de_pathlen; >> + char pathname[FLEX_ARRAY]; >> +}; >> + >> +struct conflict_part { >> + struct conflict_part *next; >> + unsigned short flags; >> + unsigned short entry_mode; >> + unsigned char sha1[20]; >> +}; >> + >> +struct conflict_entry { >> + struct conflict_entry *next; >> + unsigned int nfileconflicts; >> + struct conflict_part *entries; >> + unsigned int namelen; >> + unsigned int pathlen; >> + char name[FLEX_ARRAY]; >> +}; >> + >> +struct ondisk_conflict_part { >> + unsigned short flags; >> + unsigned short entry_mode; >> + unsigned char sha1[20]; >> +}; > > These new structs should probably be in read-cache-v5.c, or read-cache.h Makes sense, thanks. >> #define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + >> 1) >> +#define directory_entry_size(len) (offsetof(struct >> directory_entry,pathname) + (len) + 1) >> +#define conflict_entry_size(len) (offsetof(struct conflict_entry,name) + >> (len) + 1) > > These are used by read-cache-v5.c only so far. I'd say move them to > read-cache.h or read-cache-v5.c together with the new structs. Thanks. >> +struct ondisk_cache_entry { >> + unsigned short flags; >> + unsigned short mode; >> + struct cache_time mtime; >> + unsigned int size; >> + int stat_crc; >> + unsigned char sha1[20]; >> +}; >> + >> +struct ondisk_directory_entry { >> + unsigned int foffset; >> + unsigned int cr; >> + unsigned int ncr; >> + unsigned int nsubtrees; >> + unsigned int nfiles; >> + unsigned int nentries; >> + unsigned char sha1[20]; >> + unsigned short flags; >> +}; > > Perhaps use uint32_t, uint16_t and friends for all on-disk structures? We got this in the makefile, so I think we should be fine without it. It still makes sense for clarity though I think. ifdef NO_UINTMAX_T BASIC_CFLAGS += -Duintmax_t=uint32_t endif While at it I'll make the code for v[234] use them too. >> +static struct directory_entry *read_directories(unsigned int *dir_offset, >> + unsigned int *dir_table_offset, >> + void *mmap, >> + int mmap_size) >> +{ >> + int i, ondisk_directory_size; >> + uint32_t *filecrc, *beginning, *end; >> + struct directory_entry *current = NULL; >> + struct ondisk_directory_entry *disk_de; >> + struct directory_entry *de; >> + unsigned int data_len, len; >> + char *name; >> + >> + /* >> +* Length of pathname + nul byte for termination + size of >> +* members of ondisk_directory_entry. (Just using the size >> +* of the struct doesn't work, because there may be padding >> +* bytes for the struct) >> +*/ >> + ondisk_directory_size = sizeof(disk_de->flags) >> + + sizeof(disk_de->foffset) >> + + sizeof(disk_de->cr) >> + + sizeof(disk_de->ncr) >> + + sizeof(disk_de->nsubtrees) >> + + sizeof(disk_de->nfiles) >> + + sizeof(disk_de->nentries) >> + + sizeof(disk_de->sha1); >> + name = ptr_add(mmap, *dir_offset); >> + beginning = ptr_add(mmap, *dir_table_offset); >> + end = ptr_add(mmap, *dir_table_offset + 4); >> + len = ntoh_l(*end) - ntoh_l(*beginning) - ondisk_directory_size - 5; >> + disk_de = ptr_add(mmap, *dir_offset + len + 1); >> + de = directory_entry_from_ondisk(disk_de, name, len); >> + de->next = NULL; >> + >> + data_len = len + 1 + ondisk_directory_size; >> + filecrc = ptr_add(mmap, *dir_offset + data_len); >> + if (!check_crc32(0, ptr_add(mmap, *dir_offset), data_len, >> ntoh_l(*filecrc))) >> + goto unmap; >> + >> + *dir_table_offset += 4; >> + *dir_offset += data_len + 4; /* crc code */ >> + >> + current = de; >> + for (i = 0; i < de->de_nsubtrees; i++) { >> + current->next = read_directories(dir_offset, >> dir_table_offset, >> + mmap, mmap_size); >> + while (current->next) >> +
Re: [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP
On 08/07/2013 01:31 AM, Junio C Hamano wrote: > The parseopt parsing for OPT__FORCE() is implemented in terms of > OPT_BOOLEAN() and users of it can take advantage of the "counting > up" behaviour to implement increasing levels of forcefulness by > differentiating "git cmd -f" and "git cmd -f -f". > > Clarify this by explicitly using OPT_COUNTUP() instead. > > Signed-off-by: Junio C Hamano > --- > > * This _should_ be done with a similar audit of existing callers, >but I ran out of concentration. > > parse-options.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/parse-options.h b/parse-options.h > index 78f52c2..1eeb0d9 100644 > --- a/parse-options.h > +++ b/parse-options.h > @@ -238,7 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, const > char *, int); > { OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \ > PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 } > #define OPT__DRY_RUN(var, h) OPT_BOOL('n', "dry-run", (var), (h)) > -#define OPT__FORCE(var, h)OPT_BOOLEAN('f', "force", (var), (h)) > +#define OPT__FORCE(var, h)OPT_COUNTUP('f', "force", (var), (h)) > #define OPT__ABBREV(var) \ > { OPTION_CALLBACK, 0, "abbrev", (var), N_("n"), \ > N_("use digits to display SHA-1s"), \ > We need the COUNTUP, because in builtin/clean.c we have OPT__FORCE(&force, N_("force")), ... if (force > 1) rm_flags = 0; So a OPT_BOOL definitely doesn't cut it. Now that I started reviewing the OPT_FORCE parts, I realize there is still an error in the patch, which needed correction. (branch, commit, name-rev: ease up boolean conditions): - if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1) + if (force_create + list + unset_upstream + + !!delete + !!rename + !!new_upstream > 1) usage_with_options(builtin_branch_usage, options); force_create is set via OPT_FORCE as well, so we cannot remove the !! before the force_create, hence we'd only remove it from list and unset_upstream, which are set by OPT_BOOL. -- 8< -- From: Stefan Beller Date: Wed, 7 Aug 2013 09:32:25 +0200 Subject: [PATCH] branch, commit, name-rev: ease up boolean conditions Now that the variables are set by OPT_BOOL, which makes sure to have the values being 0 or 1 after parsing, we do not need the double negation to map any other value to 1 for integer variables. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/branch.c | 3 ++- builtin/commit.c | 2 +- builtin/name-rev.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 4daed0b..0903763 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if (with_commit || merge_filter != NO_FILTER) list = 1; - if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1) + if (!!delete + !!rename + !!force_create + !!new_upstream + + list + unset_upstream > 1) usage_with_options(builtin_branch_usage, options); if (abbrev == -1) diff --git a/builtin/commit.c b/builtin/commit.c index c20426b..b0f86c8 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const char *argv[], if (patch_interactive) interactive = 1; - if (!!also + !!only + !!all + !!interactive > 1) + if (also + only + all + interactive > 1) die(_("Only one of --include/--only/--all/--interactive/--patch can be used.")); if (argc == 0 && (also || (only && !amend))) die(_("No paths with --include/--only does not make sense.")); diff --git a/builtin/name-rev.c b/builtin/name-rev.c index a908a34..20fcf8c 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix) git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0); - if (!!all + !!transform_stdin + !!argc > 1) { + if (all + transform_stdin + !!argc > 1) { error("Specify either a list, or --all, not both!"); usage_with_options(name_rev_usage, opts); } -- 1.8.4.rc0.16.g7fca822.dirty signature.asc Description: OpenPGP digital signature