Git 1.8.2 l10n round 2
Hi, Leaders of Git language teams please note that a new "git.pot" is generated from v1.8.1.3-568-g5bf72 in the master branch. See commit: l10n: Update git.pot (35 new, 14 removed messages) L10n for git 1.8.2 round 2: Generate po/git.pot from v1.8.1.3-568-g5bf72. Signed-off-by: Jiang Xin This update is for the l10n of the upcoming git 1.8.2. You can get it from the usual place: https://github.com/git-l10n/git-po/ -- 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
Re: [PATCH] Documentation: filter-branch env-filter example
Am 2/13/2013 20:47, schrieb Tade: > filter-branch --env-filter example that shows how to change the email address > in all commits by a certain developer. > --- You should sign off your patch. Use a full real name, please. > Documentation/git-filter-branch.txt | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/Documentation/git-filter-branch.txt > b/Documentation/git-filter-branch.txt > index dfd12c9..2664cec 100644 > --- a/Documentation/git-filter-branch.txt > +++ b/Documentation/git-filter-branch.txt > @@ -329,6 +329,19 @@ git filter-branch --msg-filter ' > ' HEAD~10..HEAD > > > +You can modify committer/author personal information using `--env-filter`. > +For example, to update some developer's email address use this command: > + > + > +git filter-branch --env-filter ' > +if [ $GIT_AUTHOR_EMAIL =j...@old.example.com ] This should read if [ "$GIT_AUTHOR_EMAIL" = j...@old.example.com ] (double quotes, spaces around '='). The paragraph before the example talks about both author and committer, but the example handles only the author; it should handle the committer as well. > +then > +GIT_AUTHOR_EMAIL=j...@new.example.com > +fi > +export GIT_AUTHOR_EMAIL > +' -- --all > + > + The place where you inserted the example is reasonable, IMO. > To restrict rewriting to only part of the history, specify a revision > range in addition to the new branch name. The new branch name will > point to the top-most revision that a 'git rev-list' of this range -- 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: Very Urgent.
EASY LOAN. esp.ce.gov.br> writes: > > > Hello, do you know that you can now get the LOAN you deserve without the fear of been denied by any one? We are > currently offering Personal and Business Loan at 2% interest rate per annual. For urgent attention > E-mail: kdf.ls001 xyan.us > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
On 14 February 2013 15:36, Junio C Hamano wrote: >> That is, currently git add defaults to not staging file deletions, and >> we provide command line flags to include them. The consensus in the >> thread is that it is better to stage them by default; it seems >> reasonable to me that if we stage deletions by default we should >> provide flags to _not_ stage them. If that was the entirety of the >> change, would your position from that thread, "if we need this >> optional, then it is not worth doing this", still hold? > > If that is the change we are going to make, and if you can guarantee > that nobody who is used to the historical behaviour will complain, > then I am fine with it, but I think the latter part of the condition > will not hold. Does the impossibility of asserting that no-one will complain put this in the 'too hard' bucket? >> Some people would be adversely affected by this change, but any >> objections I can come up with are not game stoppers. >> - It is possible newcomers might stumble at deleted files being staged >> for commit by a command called 'add',... > > New people are fair game; we haven't trained them with the > "inconsistent" behaviour, and the default being different from > historical behaviour will not affect them adversely. > >> - For people who rely heavily on file deletions remaining out of the >> index, providing a flag allows them to keep their workflow. > > Allowing to do the things they used to be able to do is a bare > minimum. You are still forcing them to do things differently. The implication here is that a relatively small number of people will be inconvenienced by needing to specify extra flags/set up an alias. Compare this to the many for whom the expected behaviour is now default, and we have a net win. >> Finally, making this change makes sense from a consistency point of >> view. > > That is a given. Otherwise we wouldn't be even discussing this. Obviously I agree. I was actually bringing up a point about patch mode and it got incorporated into the bigger picture; patch mode includes deletions by default and I don't even know if you can turn that behaviour off. So, when we talk about git add -u and git add -A, we should also mention git add -p. Regards, Andrew Ardill -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
Andrew Ardill writes: >> We've discussed that before. >> >> http://thread.gmane.org/gmane.comp.version-control.git/171811/focus=171818 > > Something that I couldn't find discussed was the option of, rather > than providing a config to 'turn it off', inverting the current > default/flags combo. > > That is, currently git add defaults to not staging file deletions, and > we provide command line flags to include them. The consensus in the > thread is that it is better to stage them by default; it seems > reasonable to me that if we stage deletions by default we should > provide flags to _not_ stage them. If that was the entirety of the > change, would your position from that thread, "if we need this > optional, then it is not worth doing this", still hold? If that is the change we are going to make, and if you can guarantee that nobody who is used to the historical behaviour will complain, then I am fine with it, but I think the latter part of the condition will not hold. > Some people would be adversely affected by this change, but any > objections I can come up with are not game stoppers. > - It is possible newcomers might stumble at deleted files being staged > for commit by a command called 'add',... New people are fair game; we haven't trained them with the "inconsistent" behaviour, and the default being different from historical behaviour will not affect them adversely. > - For people who rely heavily on file deletions remaining out of the > index, providing a flag allows them to keep their workflow. Allowing to do the things they used to be able to do is a bare minimum. You are still forcing them to do things differently. > - For scripts that rely on this behaviour, a flag allows it to be > updated, though it may break in the meantime. Likewise. > Finally, making this change makes sense from a consistency point of > view. That is a given. Otherwise we wouldn't be even discussing this. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
On 14 February 2013 02:27, Junio C Hamano wrote: >> If we need to support this behaviour than I would suppose a config >> option is required. A default config transition path similar to git >> push defaults would probably work well, in the case where breaking >> these expectations is unacceptable. > > We've discussed that before. > > http://thread.gmane.org/gmane.comp.version-control.git/171811/focus=171818 Something that I couldn't find discussed was the option of, rather than providing a config to 'turn it off', inverting the current default/flags combo. That is, currently git add defaults to not staging file deletions, and we provide command line flags to include them. The consensus in the thread is that it is better to stage them by default; it seems reasonable to me that if we stage deletions by default we should provide flags to _not_ stage them. If that was the entirety of the change, would your position from that thread, "if we need this optional, then it is not worth doing this", still hold? Some people would be adversely affected by this change, but any objections I can come up with are not game stoppers. - It is possible newcomers might stumble at deleted files being staged for commit by a command called 'add', but if they can already grok the concept of staging then including deletions in that is trivial. If they don't understand staging then we have a different issue. - For people who rely heavily on file deletions remaining out of the index, providing a flag allows them to keep their workflow. No data would be lost, and most accidents would be easily recoverable. - For scripts that rely on this behaviour, a flag allows it to be updated, though it may break in the meantime. (I would presume that not many of these scripts exist in the first place, but I don't really know) Finally, making this change makes sense from a consistency point of view. For example, we don't track file renames because (and I paraphrase) we can work that out from the content that is moved. However if I rename a file and then 'git add .' I see that a new file is added, not that it has been renamed! Manually adding the deletion to the index causes git to correctly detect the rename, however this is unintuitive and not consistent with how git works and is communicated in general. Git add is also inconsistent with git add -p (although that might be due to unclear documentation for -p). When in patch mode, git add will propose deletions get added to the index as well, not just additions and modifications. Regards, Andrew Ardill -- 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: inotify to minimize stat() calls
Am 13.02.2013 23:55, schrieb Jeff King: > On Wed, Feb 13, 2013 at 09:25:59PM +0100, Karsten Blees wrote: > >> Alternatively, we could simply create normal cache_entries for the >> directories that are linked via ce->next, but have a trailing '/' in >> their name? >> >> Reference counting sounds good to me, at least better than allocating >> directory entries per cache entry * parent dirs. > > I think that is more or less what my patch does, but it splits the > ref-counted fake cache_entries out into a separate hash of "struct > dir_hash_entry" rather than storing it in the regular hash. Which IMHO > is a bit cleaner for two reasons: > > 1. You do not have to pay the memory price of storing fake > cache_entries (the name+refcount struct for each directory is much > smaller than a real cache_entry). > Yes, but considering the small number of directories compared to files, I think this is a relatively small price to pay. > 2. It makes the code a bit simpler, as you do not have to do any > "check for trailing /" magic on the result of index_name_exists to > determine if it is a "real" name or just a fake dir entry. > True for dir.c. On the other hand, you need a lot of new find / find_or_create logic in name-hash.c. Just to illustrate what I mean, here's a quick sketch (there's still a segfault somewhere, but I don't have time to debug right now...). Note that hash_index_entry_directories works from right to left - if the immediate parent directory is there, there's no need to check the parent's parent. cache_entry.dir points to the parent directory so that we don't need to lookup all path components for reference counting when adding / removing entries. As directory entries are 'real' cache_entries, we can reuse the existing index_name_exists and hash_index_entry code. I feel slightly guilty for abusing ce_size as reference counter...well :-) --- cache.h | 4 +++- name-hash.c | 80 - 2 files changed, 39 insertions(+), 45 deletions(-) diff --git a/cache.h b/cache.h index 665b512..2bc1372 100644 --- a/cache.h +++ b/cache.h @@ -131,7 +131,7 @@ struct cache_entry { unsigned int ce_namelen; unsigned char sha1[20]; struct cache_entry *next; - struct cache_entry *dir_next; + struct cache_entry *dir; char name[FLEX_ARRAY]; /* more */ }; @@ -285,6 +285,8 @@ extern void add_name_hash(struct index_state *istate, struct cache_entry *ce); static inline void remove_name_hash(struct cache_entry *ce) { ce->ce_flags |= CE_UNHASHED; + if (ce->dir && !(--ce->dir->ce_size)) + remove_name_hash(ce->dir); } diff --git a/name-hash.c b/name-hash.c index d8d25c2..01e8320 100644 --- a/name-hash.c +++ b/name-hash.c @@ -32,6 +32,9 @@ static unsigned int hash_name(const char *name, int namelen) return hash; } +static struct cache_entry *lookup_index_entry(struct index_state *istate, const char *name, int namelen, int icase); +static void hash_index_entry(struct index_state *istate, struct cache_entry *ce); + static void hash_index_entry_directories(struct index_state *istate, struct cache_entry *ce) { /* @@ -40,30 +43,25 @@ static void hash_index_entry_directories(struct index_state *istate, struct cach * closing slash. Despite submodules being a directory, they never * reach this point, because they are stored without a closing slash * in the cache. -* -* Note that the cache_entry stored with the directory does not -* represent the directory itself. It is a pointer to an existing -* filename, and its only purpose is to represent existence of the -* directory in the cache. It is very possible multiple directory -* hash entries may point to the same cache_entry. */ - unsigned int hash; - void **pos; + int len = ce_namelen(ce); + if (len && ce->name[len - 1] == '/') + len--; + while (len && ce->name[len - 1] != '/') + len--; + if (!len) + return; - const char *ptr = ce->name; - while (*ptr) { - while (*ptr && *ptr != '/') - ++ptr; - if (*ptr == '/') { - ++ptr; - hash = hash_name(ce->name, ptr - ce->name); - pos = insert_hash(hash, ce, &istate->name_hash); - if (pos) { - ce->dir_next = *pos; - *pos = ce; - } - } + ce->dir = lookup_index_entry(istate, ce->name, len, ignore_case); + if (!ce->dir) { + ce->dir = xcalloc(1, cache_entry_size(len)); + memcpy(ce->dir->name, ce->name, len); + ce->dir->ce_namelen = len; + ce->dir->name[len] = 0; + ha
[BUG] Veryfing signatures in git log fails when language is not english
Hi, any functionality that depends on exact exit msg of program can potentially fail because of that ᛯ export |grep LANG declare -x LANG="pl_PL.UTF-8" ᛯ ~/src/os/git/git log --format="%G? %h" |head -2 0d19377 5b9d7f8 ᛯ unset LANG ᛯ ~/src/os/git/git log --format="%G? %h" |head -2 G 0d19377 G 5b9d7f8 tested against maint (d32805d) and master (5bf72ed) maybe git should set up some output-changing variables before calling external programs? I think setting LC_ALL=C should be enougth. -- Mariusz Gronczewski (XANi) GnuPG: 0xEA8ACE64 signature.asc Description: PGP signature
Re: Cumulative "git read-tree -m" rejected with overwriting warning
Marcin Owsiany writes: > "the index file saves and restores with all this information, so you > can merge things incrementally," > > which I took to mean that I can read from multiple trees one by one > before writing the tree. That "incrementally" refers to "after a three-way merge stops with conflicts in multiple files, you can start from file A, concentrate only on that file, resolve it, and then go on to resolve conflicts in the next file B, continue, until you resolve the conflicts in the last file Z". Until you resolve a single tree-way merge fully and write the results out as a tree, you cannot merge in the next tree. That is why N-way octopus is internally implemented as a series of three-way 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
Cumulative "git read-tree -m" rejected with overwriting warning
Hello, Consider the following use case: git init seq 0 9 > f git add f git commit -m start git checkout -b b1 perl -pi -e 's,0,b1,' f git commit -a -m b1 git checkout -b b2 perl -pi -e 's,9,b2,' f git commit -a -m b2 git checkout master git merge b1 b1 As the changes to "f" don't conflict, the octopus merge deals with them just fine, and I get a merge in a single commit object. Now, let's say my b1 and b2 branches are a bit more special, and apart from the main contents (i.e. the "f" file), each branch contains a couple of files with branch-specific metadata. The names of those files are the same in each branch. (People familiar with topgit can probably guess I'm talking about topgit branches here.) I'd like to merge all such branches into "master" in one octopus-like commit, but the metadata files introduce a conflict during simple octopus merge. However I don't care about that metadata when the branches are merged into "master", so I'm trying to write a script which would do such merge while discarding the metadata files. The problem is, I cannot get it to work, when two branches modify the same file (like "f" above), even if the changes don't conflict. I get either: error: Entry 'f' would be overwritten by merge. Cannot merge. - when trying to use the two-arg form of "git read-tree -m -i" or: f: needs merge - when trying to use the three-arg form Attached is the minimal test case that reproduces the problem. It might just be that git-read-tree cannot do what I need, and I'm just misinterpreting the meaning of: "the index file saves and restores with all this information, so you can merge things incrementally," which I took to mean that I can read from multiple trees one by one before writing the tree. Could anyone give me some hints? -- Marcin Owsiany http://marcin.owsiany.pl/ GnuPG: 2048R/02F946FC 35E9 1344 9F77 5F43 13DD 6423 DBF4 80C6 02F9 46FC "Every program in development at MIT expands until it can read mail." -- Unknown testcase.sh Description: Bourne shell script
Re: inotify to minimize stat() calls
On Wed, Feb 13, 2013 at 09:25:59PM +0100, Karsten Blees wrote: > Am 13.02.2013 19:18, schrieb Jeff King: > > Moreover, looking at it again, I > > don't think my patch produces the right behavior: we have a single > > dir_next pointer, even though the same ce_entry may appear under many > > directory hashes. So the cache_entries that has to "dir/foo/" and those > > that hash to "dir/bar/" may get confused, because they will also both be > > found under "dir/", and both try to create a linked list from the > > dir_next pointer. > > > > Indeed. In the worst case, this causes an endless loop if ce->dir_next == ce > ---8<--- > mkdir V > mkdir V/XQANY > mkdir WURZAUP > touch V/XQANY/test > git init > git config core.ignorecase true > git add . > git status > ---8<--- Great, thanks for the test case. I can trivially replicate the endless loop. The patch I sent earlier fixes that. So it's at least a step in the (possible) right direction. I'm slightly concerned that there is some other case that is expecting the directories in the main hash, but I think I got them all. > Note: "V/", "V/XQANY/" and "WURZAUP/" all have the same hash_name. > Although I found those strange values by brute force, hash collisions > in 32 bit values are not that uncommon in real life :-) Cute. :) > Alternatively, we could simply create normal cache_entries for the > directories that are linked via ce->next, but have a trailing '/' in > their name? > > Reference counting sounds good to me, at least better than allocating > directory entries per cache entry * parent dirs. I think that is more or less what my patch does, but it splits the ref-counted fake cache_entries out into a separate hash of "struct dir_hash_entry" rather than storing it in the regular hash. Which IMHO is a bit cleaner for two reasons: 1. You do not have to pay the memory price of storing fake cache_entries (the name+refcount struct for each directory is much smaller than a real cache_entry). 2. It makes the code a bit simpler, as you do not have to do any "check for trailing /" magic on the result of index_name_exists to determine if it is a "real" name or just a fake dir entry. -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: [RFC v2] git-multimail: a replacement for post-receive-email
On 02/13/2013 03:56 PM, Matthieu Moy wrote: > Michael Haggerty writes: > >> A while ago, I submitted an RFC for adding a new email notification >> script to "contrib" [1]. The reaction seemed favorable and it was >> suggested that the new script should replace post-receive-email rather >> than be added separately, ideally with some kind of migration support. > > I think replacing the old post-receive-email is a sane goal in the long > run, but a good first step would be to have git-multimail merged in > contrib, and start considering the old script as deprecated (keeping the > old script doesn't harm IMHO, it's a one-file, 3 commits/year script, > not really a maintainance burden). > > I started playing with git-multimail. In short, I do like it but had to > fight a bit with python to get it to work, and couldn't get it to do > exactly what I expect. Pull request attached :-). Thanks very much for your feedback and patches. > Installation troubles: > > I had an old python installation (Red Hat package, and I'm not root), > that did not include the email.utils package, so I couldn't use my > system's python. I found no indication about python version in README, > so I installed the latest python by hand, just to find out that > git-multimail wasn't compatible with Python 3.x. 2to3 can fix > automatically a number of 3.x compatibility issues, but not all of them > so I gave up and installed Python 2.7. What version of Python was it that caused problems? I just discovered that the script wouldn't have worked with Python 2.4, where "email.utils" used to be called "email.Utils". But I pushed a fix to GitHub: ddb1796660 Accommodate older versions of Python's email module. With this change, I think that git-multimail will work with any version of Python 2.4 <= x < 3.0. So if your original problem was with Python 2.4, maybe you could try the new version and see if it works with that interpreter. > I think adding a short "dependencies" section in the README (or in an > INSTALL file) saying which Python version works could save new users the > trouble (I see the sheebang inside the scripts says python2 but since I > couldn't use my system's python and called > "path/to/python git_multimail.py", this didn't help). Yes, I'm working on a "Requirements" section with that information and more. I'd like to list a minimum git version too, but it would be quite a bit of work to figure out when each command and each option was added. It would be helpful if anybody who has used the script with an old version of git lets me know whether they were successful or not. > Making the script > portable with python 2 and 3 would be awesome ;-). Agreed, but I doubt I will be able to get to it very soon. > Missing feature: > > git-multimail can send a summary for each push, with the "git log --oneline" > of the new revisions, and then 1 mail per patch with the complete log > and the patch. > > I'd like to have the intermediate: allow the summary email to include > the complete log (not just --oneline). My colleagues already think they > receive too many emails so I don't think they'd like the "one email per > commit" way, but the 1 line summary is really short OTOH. > > I wrote a quick and hopefully not-too-dirty implementation of it, > there's a pull request here: > > https://github.com/mhagger/git-multimail/pull/6 > > essentially, it boils down to: > > @@ -835,6 +837,17 @@ class ReferenceChange(Change): > for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE): > yield line > > +if adds and self.showlog: > +yield '\n' > +yield 'Detailed log of added commits:\n\n' > +for line in read_lines( > +['git', 'log'] > ++ self.logopts > ++ ['%s..%s' % (self.old.commit, self.new.commit,)], > +keepends=True, > +): > +yield line > + > # The diffstat is shown from the old revision to the new > # revision. This is to show the truth of what happened in > # this change. There's no point showing the stat from the > Thanks for the patch. I like the idea, but I think the implementation is incorrect. Your code will not only list new commits but will also list commits that were already in the repository on another branch (e.g., if an existing feature branch is merged into master, all of the commits on the feature branch will be listed). (Or was that your intention?) But even worse, it will fail to list commits that were added at the same time that a branch was created (e.g., if I create a feature branch with a number of commits on it and then push it for the first time). Probably the Push object has to negotiate with its constituent ReferenceChange objects to figure out which one is responsible for summarizing each of the commits newly added
[PATCH v3 3/3] t9903: add extra tests for bash.showDirtyState
Add 3 extra tests for the bash.showDirtyState config option, the test now cover all combinations of the shell var being set/unset and the config option being missing/enabled/disabled, given a dirty file. * Renamed test 'disabled by config' to 'shell variable set with config disabled' for consistency Signed-off-by: Martin Erik Werner --- t/t9903-bash-prompt.sh | 38 +- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index dd9ac6a..2101d91 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - before root commit' ' test_cmp expected "$actual" ' -test_expect_success 'prompt - dirty status indicator - disabled by config' ' +test_expect_success 'prompt - dirty status indicator - shell variable unset with config disabled' ' printf " (master)" > expected && echo "dirty" > file && test_when_finished "git reset --hard" && test_config bash.showDirtyState false && ( + sane_unset GIT_PS1_SHOWDIRTYSTATE && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - dirty status indicator - shell variable unset with config enabled' ' + printf " (master)" > expected && + echo "dirty" > file && + test_when_finished "git reset --hard" && + test_config bash.showDirtyState true && + ( + sane_unset GIT_PS1_SHOWDIRTYSTATE && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config disabled' ' + printf " (master)" > expected && + echo "dirty" > file && + test_when_finished "git reset --hard" && + test_config bash.showDirtyState false && + ( + GIT_PS1_SHOWDIRTYSTATE=y && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config enabled' ' + printf " (master *)" > expected && + echo "dirty" > file && + test_when_finished "git reset --hard" && + test_config bash.showDirtyState true && + ( GIT_PS1_SHOWDIRTYSTATE=y && __git_ps1 > "$actual" ) && -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] t9903: add tests for bash.showUntrackedFiles
Add 4 test for the bash.showUntrackedFiles config option, the tests now cover all combinations of the shell var being set/unset and the config option being missing/enabled/disabled. Signed-off-by: Martin Erik Werner --- t/t9903-bash-prompt.sh | 40 1 file changed, 40 insertions(+) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index f17c1f8..dd9ac6a 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status indicator - untracked files test_cmp expected "$actual" ' +test_expect_success 'prompt - untracked files status indicator - shell variable unset with config disabled' ' + printf " (master)" > expected && + test_config bash.showUntrackedFiles false && + ( + sane_unset GIT_PS1_SHOWUNTRACKEDFILES && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - shell variable unset with config enabled' ' + printf " (master)" > expected && + test_config bash.showUntrackedFiles true && + ( + sane_unset GIT_PS1_SHOWUNTRACKEDFILES && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - shell variable set with config disabled' ' + printf " (master)" > expected && + test_config bash.showUntrackedFiles false && + ( + GIT_PS1_SHOWUNTRACKEDFILES=y && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - shell variable set with config enabled' ' + printf " (master %%)" > expected && + test_config bash.showUntrackedFiles true && + ( + GIT_PS1_SHOWUNTRACKEDFILES=y && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + test_expect_success 'prompt - untracked files status indicator - not shown inside .git directory' ' printf " (GIT_DIR!)" > expected && ( -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner writes: >> OK, I'll locally amend the patch. Thanks. > > Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded > commits + sign-off then, I can if you prefer that? You can if you wanted to. That would be less work for me ;-). -- 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 3/3] t9903: add extra tests for bash.showDirtyState
On Wed, 2013-02-13 at 11:53 -0800, Junio C Hamano wrote: > Martin Erik Werner writes: > > >> Strictly speaking, you have 6 not 4 combinations (shell variable > >> set/unset * config missing/set to false/set to true). I think these > >> additional tests cover should all 6 because "config missing" case > >> should already have had tests before bash.showDirtyState was added. > >> > > > > Indeed, I only mentioned 4 since the other ones existed already, and I > > didn't change them, but maybe it should be mentioned as "combined with > > previous tests (...) cover all 6 combinations (...)" then? > > It should be sufficient to change the third line of your original to > say "the config option being missing/enabled/disabled, given a dirty > file." and nothing else, I think. > > >> Sign-off? > > > > Ah, just forgot the -s flag on that commit, yes it should be Signed-off > > by me. > > OK, I'll locally amend the patch. Thanks. Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded commits + sign-off then, I can if you prefer that? -- Martin Erik Werner -- 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: inotify to minimize stat() calls
Am 13.02.2013 19:18, schrieb Jeff King: > Moreover, looking at it again, I > don't think my patch produces the right behavior: we have a single > dir_next pointer, even though the same ce_entry may appear under many > directory hashes. So the cache_entries that has to "dir/foo/" and those > that hash to "dir/bar/" may get confused, because they will also both be > found under "dir/", and both try to create a linked list from the > dir_next pointer. > Indeed. In the worst case, this causes an endless loop if ce->dir_next == ce ---8<--- mkdir V mkdir V/XQANY mkdir WURZAUP touch V/XQANY/test git init git config core.ignorecase true git add . git status ---8<--- Note: "V/", "V/XQANY/" and "WURZAUP/" all have the same hash_name. Although I found those strange values by brute force, hash collisions in 32 bit values are not that uncommon in real life :-) > Looking at Karsten's patch, it seems like it will not add a cache entry > if there is one of the same name. But I'm not sure if that is right, as > the old one might be CE_UNHASHED (or it might get removed later). You > actually want to be able to find each cache_entry that has a file under > the directory at the hash of that directory, so you can make sure it is > still valid. > Yes, the patch was just to show potential performance savings, I didn't consider CE_UNHASHED at all. > I think the best way forward is to actually create a separate hash table > for the directory lookups. I note that we only care about these entries > in directory_exists_in_index_icase, which is really about whether > something is there, versus what exactly is there. So could we maybe get > by with a separate hash table that stores a count of entries at each > directory, and increment/decrement the count when we add/remove entries? > Alternatively, we could simply create normal cache_entries for the directories that are linked via ce->next, but have a trailing '/' in their name? Reference counting sounds good to me, at least better than allocating directory entries per cache entry * parent dirs. -- 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] Documentation: filter-branch env-filter example
filter-branch --env-filter example that shows how to change the email address in all commits by a certain developer. --- Documentation/git-filter-branch.txt | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt index dfd12c9..2664cec 100644 --- a/Documentation/git-filter-branch.txt +++ b/Documentation/git-filter-branch.txt @@ -329,6 +329,19 @@ git filter-branch --msg-filter ' ' HEAD~10..HEAD +You can modify committer/author personal information using `--env-filter`. +For example, to update some developer's email address use this command: + + +git filter-branch --env-filter ' + if [ $GIT_AUTHOR_EMAIL =j...@old.example.com ] + then + GIT_AUTHOR_EMAIL=j...@new.example.com + fi + export GIT_AUTHOR_EMAIL +' -- --all + + To restrict rewriting to only part of the history, specify a revision range in addition to the new branch name. The new branch name will point to the top-most revision that a 'git rev-list' of this range -- 1.7.11.7 -- 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] Makefile: don't run rm without any files
Junio C Hamano wrote: > I amended the log message like so: > > commit bd9df384b16077337fffe9836c9255976b0e7b91 > Author: Matt Kraai > Date: Wed Feb 13 07:57:48 2013 -0800 > > Makefile: don't run rm without any files > > When COMPUTE_HEADER_DEPENDENCIES is set to "auto" and the compiler > does not support it, $(dep_dirs) becomes empty. "make clean" runs > "rm -rf $(dep_dirs)", which fails in such a case. To pedantic, that only fails on some platforms. The autoconf manual explains: It is not portable to invoke rm without options or operands. On the other hand, Posix now requires rm -f to silently succeed when there are no operands (useful for constructs like rm -rf $filelist without first checking if ‘$filelist’ was empty). But this was not always portable; at least NetBSD rm built before 2008 would fail with a diagnostic. Anyway, looks like a good fix. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: don't run rm without any files
Matt Kraai writes: > I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to "auto". > The automatic detection determines that the compiler doesn't support > it, so it's then set to "no". CHECK_HEADER_DEPENDENCIES isn't set > either, so about 20 lines below the dep_dirs assignment you quoted, > dep_dirs is cleared: > > ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes) > ifndef CHECK_HEADER_DEPENDENCIES > dep_dirs = > ... > > Should I submit an updated patch with a different commit message? I amended the log message like so: commit bd9df384b16077337fffe9836c9255976b0e7b91 Author: Matt Kraai Date: Wed Feb 13 07:57:48 2013 -0800 Makefile: don't run rm without any files When COMPUTE_HEADER_DEPENDENCIES is set to "auto" and the compiler does not support it, $(dep_dirs) becomes empty. "make clean" runs "rm -rf $(dep_dirs)", which fails in such a case. Signed-off-by: Matt Kraai Signed-off-by: Junio C Hamano -- 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] submodule: add 'deinit' command
Jens Lehmann writes: > Junio, this looks like a we have v5 as soon as we decide what to do > with the "not initialized" messages when '.' is used, right? OK. I myself do not deeply care if we end up special casing "." or not; I'll leave it up to you and other submodule folks. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner writes: >> Strictly speaking, you have 6 not 4 combinations (shell variable >> set/unset * config missing/set to false/set to true). I think these >> additional tests cover should all 6 because "config missing" case >> should already have had tests before bash.showDirtyState was added. >> > > Indeed, I only mentioned 4 since the other ones existed already, and I > didn't change them, but maybe it should be mentioned as "combined with > previous tests (...) cover all 6 combinations (...)" then? It should be sufficient to change the third line of your original to say "the config option being missing/enabled/disabled, given a dirty file." and nothing else, I think. >> Sign-off? > > Ah, just forgot the -s flag on that commit, yes it should be Signed-off > by me. OK, I'll locally amend the patch. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] t9903: add tests for bash.showUntrackedFiles
Martin Erik Werner writes: > So would it make sense to do: > GIT_PS1_SHOWUNTRACKEDFILES="dummy" && > unset GIT_PS1_SHOWUNTRACKEDFILES && > (...) > instead then? I think we have sane_unset exactly for this reason. -- 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: inotify to minimize stat() calls
On Wed, Feb 13, 2013 at 01:18:51PM -0500, Jeff King wrote: > I think the best way forward is to actually create a separate hash table > for the directory lookups. I note that we only care about these entries > in directory_exists_in_index_icase, which is really about whether > something is there, versus what exactly is there. So could we maybe get > by with a separate hash table that stores a count of entries at each > directory, and increment/decrement the count when we add/remove entries? > > The biggest problem I see with that is that we do indeed care a little > bit what is at the directory: we check the mode to see if it is a gitdir > or not. But I think we can maybe sneak around that: gitdirs have actual > entries in the index, whereas the directories do not. So we would find > them via index_name_exists; anything that is not there, but _is_ in the > special directory hash would therefore be a directory. > > I realize it got pretty esoteric there in the middle. I'll see if I can > work up a patch that expresses what I'm thinking. So here's a patch. It's mostly meant to illustrate what I'm thinking, and I have no clue if it introduces regressions. It does pass the test suite, but we have virtually no ignorecase tests. It seems to behave sanely when I set core.ignorecase on my Linux box, but I have no idea what it will do on a real case-insensitive system (nor even, to be honest, what kinds of scenarios should be tested for the dir-hashing stuff). --- diff --git a/cache.h b/cache.h index e493563..6630a35 100644 --- a/cache.h +++ b/cache.h @@ -131,7 +131,6 @@ struct cache_entry { unsigned int ce_namelen; unsigned char sha1[20]; struct cache_entry *next; - struct cache_entry *dir_next; char name[FLEX_ARRAY]; /* more */ }; @@ -267,26 +266,14 @@ extern void add_name_hash(struct index_state *istate, struct cache_entry *ce); unsigned name_hash_initialized : 1, initialized : 1; struct hash_table name_hash; + struct hash_table dir_hash; }; extern struct index_state the_index; /* Name hashing */ extern void add_name_hash(struct index_state *istate, struct cache_entry *ce); -/* - * We don't actually *remove* it, we can just mark it invalid so that - * we won't find it in lookups. - * - * Not only would we have to search the lists (simple enough), but - * we'd also have to rehash other hash buckets in case this makes the - * hash bucket empty (common). So it's much better to just mark - * it. - */ -static inline void remove_name_hash(struct cache_entry *ce) -{ - ce->ce_flags |= CE_UNHASHED; -} - +extern void remove_name_hash(struct index_state *istate, struct cache_entry *ce); #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS #define active_cache (the_index.cache) @@ -443,6 +430,7 @@ extern struct cache_entry *index_name_exists(struct index_state *istate, const c extern int unmerged_index(const struct index_state *); extern int verify_path(const char *path); extern struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int igncase); +extern int index_icase_dir_exists(struct index_state *istate, const char *name, int namelen); extern int index_name_pos(const struct index_state *, const char *name, int namelen); #define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */ #define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */ diff --git a/dir.c b/dir.c index 57394e4..f73ac34 100644 --- a/dir.c +++ b/dir.c @@ -927,29 +927,27 @@ static enum exist_status directory_exists_in_index_icase(const char *dirname, in */ static enum exist_status directory_exists_in_index_icase(const char *dirname, int len) { - struct cache_entry *ce = index_name_exists(&the_index, dirname, len + 1, ignore_case); - unsigned char endchar; - - if (!ce) - return index_nonexistent; - endchar = ce->name[len]; + struct cache_entry *ce = index_name_exists(&the_index, dirname, len, ignore_case); /* -* The cache_entry structure returned will contain this dirname -* and possibly additional path components. +* We found something in the index, which means it is either an actual +* file, or a gitdir. */ - if (endchar == '/') - return index_directory; + if (ce) { + if (S_ISGITLINK(ce->ce_mode)) + return index_gitdir; + /* We call a file "index_nonexistent" here, because the caller is +* asking about a directory. */ + return index_nonexistent; + } /* -* If there are no additional path components, then this cache_entry -* represents a submodule. Submodules, despite being directories, -* are stored in the cache without a closing slash. +* Otherwise, it might be a leading path of something that is in the +* index. We can look it up in the special dir hash.
Re: [PATCH v4] submodule: add 'deinit' command
Am 12.02.2013 18:11, schrieb Phil Hord: > On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann wrote: > + die_if_unmatched "$mode" >> + name=$(module_name "$sm_path") || exit >> + url=$(git config submodule."$name".url) >> + if test -z "$url" >> + then >> + say "$(eval_gettext "No url found for submodule path >> '\$sm_path' in .git/config")" > > Is it safe to shelter the user a little bit more from the git > internals here and say instead: > >Submodule '\$sm_path' is not initialized. Yeah, that makes much more sense. But I'd rather use the name too: Submodule '\$name' is not initialized for path '\$sm_path' > Also, I think this code will show this message for each submodule on > 'git submodule deinit .' But I think I would prefer to suppress it in > that case. If I have not explicitly stated which submodules to > deinit, then I do not think git should complain that some of them are > not initialized. Yes, that message gets printed for each uninitialized submodule. We could easily suppress that for '.', but it would be really hard to get that right for other wildcards like 'foo*'. (And e.g. running a "submodule update" also lists all submodules it skips because of an "update=none" setting, so I'm not sure if it's that important here) But if people really want that, I'd suppress that for the '.' case. Further opinions? >> + continue >> + fi >> + >> + # Remove the submodule work tree (unless the user already >> did it) >> + if test -d "$sm_path" >> + then >> + # Protect submodules containing a .git directory >> + if test -d "$sm_path/.git" >> + then >> + echo >&2 "$(eval_gettext "Submodule work >> tree $sm_path contains a .git directory")" >> + die "$(eval_gettext "(use 'rm -rf' if you >> really want to remove it including all of its history)")" > > I expect this is the right thing to do for now. But I wonder if we > can also move $sm_path/.git to $GIT_DIR/modules/$sm_path in this case > (though I think I am not spelling this path correctly). Would that be > ok? What extra work is needed to relocate the .git dir like this? Hmm, I'm a bit torn about automagically moving the repo somewhere else. While I think it is a sane solution for most users I suspect some users might hate us for doing that without asking (and with no option to turn that off). What about adding a separate "git submodule to-gitfile" command which does that and hinting that here? However I do have the feeling that this should be done in another commit. >> + die "$(eval_gettext "Submodule work tree >> $sm_path contains local modifications, use '-f' to discard them")" > > Nit, grammar: use a semicolon instead of a comma. Ok. And while looking at it I noticed that "$sm_path" should be "'\$sm_path'" here, same a few lines above ... sigh >> +test_expect_success 'set up a second submodule' ' >> + git submodule add ./init2 example2 && >> + git commit -m "submodle example2 added" > > Nit: submodule is misspelled Thanks. Junio, this looks like a we have v5 as soon as we decide what to do with the "not initialized" messages when '.' is used, right? My current diff to v4 looks like this: -8<- diff --git a/git-submodule.sh b/git-submodule.sh index f1b552f..27f8e12 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -591,7 +591,7 @@ cmd_deinit() url=$(git config submodule."$name".url) if test -z "$url" then - say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")" + say "$(eval_gettext "Submodule '\$name' is not initialized for path '\$sm_path'")" continue fi @@ -601,14 +601,14 @@ cmd_deinit() # Protect submodules containing a .git directory if test -d "$sm_path/.git" then - echo >&2 "$(eval_gettext "Submodule work tree $sm_path contains a .git directory")" + echo >&2 "$(eval_gettext "Submodule work tree '\$sm_path' contains a .git directory")" die "$(eval_gettext "(use 'rm -rf' if you really want to remove it including all of its history)")" fi if test -z "$force" then git rm -n "$sm_path" || - die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")" + die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modificatio
Re: inotify to minimize stat() calls
On Wed, Feb 13, 2013 at 07:15:47PM +0700, Nguyen Thai Ngoc Duy wrote: > On Wed, Feb 13, 2013 at 3:48 AM, Karsten Blees > wrote: > > 2.) 0.135 s is spent in name-hash.c/hash_index_entry_directories, > > reindexing the same directories over and over again. In the end, the > > hashtable contains 939k directory entries, even though the WebKit test repo > > only has 7k directories. Checking if a directory entry already exists could > > reduce that, i.e.: > > This function is only used when core.ignorecase = true. I probably > won't be able to test this, so I'll leave this to other people who > care about ignorecase. > > This function used to have lookup_hash, but it was removed by Jeff in > 2548183 (fix phantom untracked files when core.ignorecase is set - > 2011-10-06). There's a looong commit message which I'm too lazy to > read. Anybody who works on this should though. Yeah, the problem that commit tried to solve is that linking to a single cache entry through the hash is not enough, because we may remove cache items. Imagine you have "dir/one" and "dir/two", and you add them to the in-memory index in that order. The original code hashed "dir/" and inserted a link to the "dir/one" cache entry. When it came time to put in the "dir/two" entry, we noticed that there was already a "dir/" entry and did nothing. Then later, if we remove "dir/one", we do so by marking it with CE_UNHASHED. So a later query for "dir/" will see "nope, nothing here that wasn't CE_UNHASHED", which is wrong. We never recorded that "dir/two" existed under the hash for "dir/", so we can't know about it. My patch just stores the cache_entry for both under the "dir/" hash. As Karsten noticed, that can lead to a large number of hash entries, because adding "some/deep/hierarchy/with/files" will add 4 directory entries for just that single file. Moreover, looking at it again, I don't think my patch produces the right behavior: we have a single dir_next pointer, even though the same ce_entry may appear under many directory hashes. So the cache_entries that has to "dir/foo/" and those that hash to "dir/bar/" may get confused, because they will also both be found under "dir/", and both try to create a linked list from the dir_next pointer. Looking at Karsten's patch, it seems like it will not add a cache entry if there is one of the same name. But I'm not sure if that is right, as the old one might be CE_UNHASHED (or it might get removed later). You actually want to be able to find each cache_entry that has a file under the directory at the hash of that directory, so you can make sure it is still valid. And of course that still leaves the existing correctness problem I mentioned above. I think the best way forward is to actually create a separate hash table for the directory lookups. I note that we only care about these entries in directory_exists_in_index_icase, which is really about whether something is there, versus what exactly is there. So could we maybe get by with a separate hash table that stores a count of entries at each directory, and increment/decrement the count when we add/remove entries? The biggest problem I see with that is that we do indeed care a little bit what is at the directory: we check the mode to see if it is a gitdir or not. But I think we can maybe sneak around that: gitdirs have actual entries in the index, whereas the directories do not. So we would find them via index_name_exists; anything that is not there, but _is_ in the special directory hash would therefore be a directory. I realize it got pretty esoteric there in the middle. I'll see if I can work up a patch that expresses what I'm thinking. -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 v2 3/3] t9903: add extra tests for bash.showDirtyState
On Wed, 2013-02-13 at 08:28 -0800, Junio C Hamano wrote: > Martin Erik Werner writes: > > > Added 3 extra tests for the bash.showDirtyState config option, tests > > should now cover all combinations of the shell var being set/unset and > > the config option being enabled/disabled, given a dirty file. > > Strictly speaking, you have 6 not 4 combinations (shell variable > set/unset * config missing/set to false/set to true). I think these > additional tests cover should all 6 because "config missing" case > should already have had tests before bash.showDirtyState was added. > Indeed, I only mentioned 4 since the other ones existed already, and I didn't change them, but maybe it should be mentioned as "combined with previous tests (...) cover all 6 combinations (...)" then? > > * Renamed test 'disabled by config' to 'shell variable set with config > > disabled' for consistency > > --- > > Sign-off? Ah, just forgot the -s flag on that commit, yes it should be Signed-off by me. > > > t/t9903-bash-prompt.sh | 38 +- > > 1 file changed, 37 insertions(+), 1 deletion(-) > > > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > > index cb008e2..fa81b09 100755 > > --- a/t/t9903-bash-prompt.sh > > +++ b/t/t9903-bash-prompt.sh > > @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator > > - before root commit' ' > > test_cmp expected "$actual" > > ' > > > > -test_expect_success 'prompt - dirty status indicator - disabled by config' > > ' > > +test_expect_success 'prompt - dirty status indicator - shell variable > > unset with config disabled' ' > > printf " (master)" > expected && > > echo "dirty" > file && > > test_when_finished "git reset --hard" && > > test_config bash.showDirtyState false && > > ( > > + unset -v GIT_PS1_SHOWDIRTYSTATE && > > + __git_ps1 > "$actual" > > + ) && > > + test_cmp expected "$actual" > > +' > > + > > +test_expect_success 'prompt - dirty status indicator - shell variable > > unset with config enabled' ' > > + printf " (master)" > expected && > > + echo "dirty" > file && > > + test_when_finished "git reset --hard" && > > + test_config bash.showDirtyState true && > > + ( > > + unset -v GIT_PS1_SHOWDIRTYSTATE && > > + __git_ps1 > "$actual" > > + ) && > > + test_cmp expected "$actual" > > +' > > + > > +test_expect_success 'prompt - dirty status indicator - shell variable set > > with config disabled' ' > > + printf " (master)" > expected && > > + echo "dirty" > file && > > + test_when_finished "git reset --hard" && > > + test_config bash.showDirtyState false && > > + ( > > + GIT_PS1_SHOWDIRTYSTATE=y && > > + __git_ps1 > "$actual" > > + ) && > > + test_cmp expected "$actual" > > +' > > + > > +test_expect_success 'prompt - dirty status indicator - shell variable set > > with config enabled' ' > > + printf " (master *)" > expected && > > + echo "dirty" > file && > > + test_when_finished "git reset --hard" && > > + test_config bash.showDirtyState true && > > + ( > > GIT_PS1_SHOWDIRTYSTATE=y && > > __git_ps1 > "$actual" > > ) && -- 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 2/3] t9903: add tests for bash.showUntrackedFiles
On Wed, 2013-02-13 at 08:23 -0800, Junio C Hamano wrote: > Martin Erik Werner writes: > > > Add 4 test for the bash.showUntrackedFiles config option, covering all > > combinations of the shell var being set/unset and the config option > > being enabled/disabled. > > > > Signed-off-by: Martin Erik Werner > > --- > > t/t9903-bash-prompt.sh | 40 > > 1 file changed, 40 insertions(+) > > > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > > index f17c1f8..cb008e2 100755 > > --- a/t/t9903-bash-prompt.sh > > +++ b/t/t9903-bash-prompt.sh > > @@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status > > indicator - untracked files > > test_cmp expected "$actual" > > ' > > > > +test_expect_success 'prompt - untracked files status indicator - shell > > variable unset with config disabled' ' > > + printf " (master)" > expected && > > + test_config bash.showUntrackedFiles false && > > + ( > > + unset -v GIT_PS1_SHOWUNTRACKEDFILES && > > We do not use "unset -v" anywhere else in our system. Shells > mimicking SysV may choke on it. A Portable POSIX script can omit > "-v" when unsetting a variable. > > Also "unset" can return false when the variable is not set to begin > with with some shells. > > Neither of these matters for this particular case because we know we > are running this under bash in non-posix mode. I however wonder if > we can do something to prevent careless coders to copy and paste > this piece when updating other tests that are not limited to bash. > Commenting each and every use of "unset -v" does not sound like a > good solution and perhaps I am being unnecessarily worried too much. > Yeah, my (ba)sh foo is a bit limited, I was just basing on http://wiki.bash-hackers.org/commands/builtin/unset#portability_considerations which seemed to recommend using -v. So would it make sense to do: GIT_PS1_SHOWUNTRACKEDFILES="dummy" && unset GIT_PS1_SHOWUNTRACKEDFILES && (...) instead then? -- Martin Erik Werner -- 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] Makefile: don't run rm without any files
On Wed, Feb 13, 2013 at 08:51:45AM -0800, Junio C Hamano wrote: > Matt Kraai writes: > > > From: Matt Kraai > > > > "rm -f -r" fails on QNX when not passed any files to remove. > > I do not think it is limited to QNX. > > > the clean target, since dep_dirs is empty. > > And dep_dirs being empty under some circumstance shouldn't be > limited to QNX, either. > > I think your change does no harm, may be a good change if dep_dirs > goes empty, but the justification is lacking. What caused your > dep_dirs to become empty in the first place? > > I am scratching my head because I see > > OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ > $(XDIFF_OBJS) \ > $(VCSSVN_OBJS) \ > git.o > dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to "auto". The automatic detection determines that the compiler doesn't support it, so it's then set to "no". CHECK_HEADER_DEPENDENCIES isn't set either, so about 20 lines below the dep_dirs assignment you quoted, dep_dirs is cleared: ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes) ifndef CHECK_HEADER_DEPENDENCIES dep_dirs = ... Should I submit an updated patch with a different commit message? -- 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 3/4] count-objects: report garbage files in pack directory too
Junio C Hamano writes: >> +if (has_extension(de->d_name, ".idx") || >> +has_extension(de->d_name, ".pack") || >> +has_extension(de->d_name, ".keep")) >> +string_list_append(&garbage, path); > > It might be OK to put .pack and .keep in the same "if (A || B)" as > it may happen to be that they do not need any special treatment > right now, but I do not think this is a good idea in general. Actually I take this part back. I can see that the condition part grow over time but I do not think the body should. That is the whole point of collecting paths that cannot be judged as garbage by themselves into a list; we shouldn't be doing anything else by definition in the body. Everything else I said in the review still stands, though.. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: don't run rm without any files
Matt Kraai writes: > From: Matt Kraai > > "rm -f -r" fails on QNX when not passed any files to remove. I do not think it is limited to QNX. > the clean target, since dep_dirs is empty. And dep_dirs being empty under some circumstance shouldn't be limited to QNX, either. I think your change does no harm, may be a good change if dep_dirs goes empty, but the justification is lacking. What caused your dep_dirs to become empty in the first place? I am scratching my head because I see OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ $(XDIFF_OBJS) \ $(VCSSVN_OBJS) \ git.o dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS > Avoid this by merging two rm > command lines. > > Signed-off-by: Matt Kraai > --- > Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 5a2e02d..c2e3666 100644 > --- a/Makefile > +++ b/Makefile > @@ -2414,8 +2414,7 @@ clean: profile-clean > builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) > $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X > $(RM) $(TEST_PROGRAMS) > - $(RM) -r bin-wrappers > - $(RM) -r $(dep_dirs) > + $(RM) -r bin-wrappers $(dep_dirs) > $(RM) -r po/build/ > $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) > tags cscope* > $(RM) -r $(GIT_TARNAME) .doc-tmp-dir -- 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 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner writes: > Added 3 extra tests for the bash.showDirtyState config option, tests > should now cover all combinations of the shell var being set/unset and > the config option being enabled/disabled, given a dirty file. Strictly speaking, you have 6 not 4 combinations (shell variable set/unset * config missing/set to false/set to true). I think these additional tests cover should all 6 because "config missing" case should already have had tests before bash.showDirtyState was added. > * Renamed test 'disabled by config' to 'shell variable set with config > disabled' for consistency > --- Sign-off? > t/t9903-bash-prompt.sh | 38 +- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index cb008e2..fa81b09 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - > before root commit' ' > test_cmp expected "$actual" > ' > > -test_expect_success 'prompt - dirty status indicator - disabled by config' ' > +test_expect_success 'prompt - dirty status indicator - shell variable unset > with config disabled' ' > printf " (master)" > expected && > echo "dirty" > file && > test_when_finished "git reset --hard" && > test_config bash.showDirtyState false && > ( > + unset -v GIT_PS1_SHOWDIRTYSTATE && > + __git_ps1 > "$actual" > + ) && > + test_cmp expected "$actual" > +' > + > +test_expect_success 'prompt - dirty status indicator - shell variable unset > with config enabled' ' > + printf " (master)" > expected && > + echo "dirty" > file && > + test_when_finished "git reset --hard" && > + test_config bash.showDirtyState true && > + ( > + unset -v GIT_PS1_SHOWDIRTYSTATE && > + __git_ps1 > "$actual" > + ) && > + test_cmp expected "$actual" > +' > + > +test_expect_success 'prompt - dirty status indicator - shell variable set > with config disabled' ' > + printf " (master)" > expected && > + echo "dirty" > file && > + test_when_finished "git reset --hard" && > + test_config bash.showDirtyState false && > + ( > + GIT_PS1_SHOWDIRTYSTATE=y && > + __git_ps1 > "$actual" > + ) && > + test_cmp expected "$actual" > +' > + > +test_expect_success 'prompt - dirty status indicator - shell variable set > with config enabled' ' > + printf " (master *)" > expected && > + echo "dirty" > file && > + test_when_finished "git reset --hard" && > + test_config bash.showDirtyState true && > + ( > GIT_PS1_SHOWDIRTYSTATE=y && > __git_ps1 > "$actual" > ) && -- 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 2/3] t9903: add tests for bash.showUntrackedFiles
Martin Erik Werner writes: > Add 4 test for the bash.showUntrackedFiles config option, covering all > combinations of the shell var being set/unset and the config option > being enabled/disabled. > > Signed-off-by: Martin Erik Werner > --- > t/t9903-bash-prompt.sh | 40 > 1 file changed, 40 insertions(+) > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index f17c1f8..cb008e2 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status > indicator - untracked files > test_cmp expected "$actual" > ' > > +test_expect_success 'prompt - untracked files status indicator - shell > variable unset with config disabled' ' > + printf " (master)" > expected && > + test_config bash.showUntrackedFiles false && > + ( > + unset -v GIT_PS1_SHOWUNTRACKEDFILES && We do not use "unset -v" anywhere else in our system. Shells mimicking SysV may choke on it. A Portable POSIX script can omit "-v" when unsetting a variable. Also "unset" can return false when the variable is not set to begin with with some shells. Neither of these matters for this particular case because we know we are running this under bash in non-posix mode. I however wonder if we can do something to prevent careless coders to copy and paste this piece when updating other tests that are not limited to bash. Commenting each and every use of "unset -v" does not sound like a good solution and perhaps I am being unnecessarily worried too much. -- 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 v2] git-multimail: a replacement for post-receive-email
Andy Parkins writes: > On Wednesday 13 February 2013 14:56:25 Matthieu Moy wrote: >> Michael Haggerty writes: > >> I think adding a short "dependencies" section in the README (or in an >> INSTALL file) saying which Python version works could save new users the >> trouble (I see the sheebang inside the scripts says python2 but since I >> couldn't use my system's python and called >> "path/to/python git_multimail.py", this didn't help). Making the script >> portable with python 2 and 3 would be awesome ;-). > > For my 2p worth, I don't like seeing hooks called like this. Particular > those > that come as part of the standard installation. What do you mean by "like this" ? > I call mine by installing little scripts like this (on Debian): > > #!/bin/sh > # stored as $GIT_WORK_DIR/.git/hooks/post-receive-email > exec /bin/sh /usr/share/git-core/contrib/hooks/post-receive-email Yes, this is what I was doing (with path/to/python instead of /bin/sh, and git_multimail.py, or more precisely path/to/git_multimail.py, instead of post-receive-email). -- 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 v2 1/3] shell prompt: add bash.showUntrackedFiles option
Martin Erik Werner writes: > Add a config option 'bash.showUntrackedFiles' which allows enabling > the prompt showing untracked files on a per-repository basis. This is > useful for some repositories where the 'git ls-files ...' command may > take a long time. > > Signed-off-by: Martin Erik Werner > --- > contrib/completion/git-prompt.sh | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/contrib/completion/git-prompt.sh > b/contrib/completion/git-prompt.sh > index 9bef053..9b2eec2 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -43,7 +43,10 @@ > # > # If you would like to see if there're untracked files, then you can set > # GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there're untracked > -# files, then a '%' will be shown next to the branch name. > +# files, then a '%' will be shown next to the branch name. You can > +# configure this per-repository with the bash.showUntrackedFiles > +# variable, which defaults to true once GIT_PS1_SHOWUNTRACKEDFILES is > +# enabled. > # > # If you would like to see the difference between HEAD and its upstream, > # set GIT_PS1_SHOWUPSTREAM="auto". A "<" indicates you are behind, ">" > @@ -332,8 +335,10 @@ __git_ps1 () > fi > > if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then > - if [ -n "$(git ls-files --others > --exclude-standard)" ]; then > - u="%" > + if [ "$(git config --bool > bash.showUntrackedFiles)" != "false" ]; then > + if [ -n "$(git ls-files --others > --exclude-standard)" ]; then > + u="%" > + fi > fi > fi Somebody should simplify this deeply nested "if/then/if/then/fi/fi" sequence to a single if/then/fi statement, i.e. something like: if test -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" && test "$(git config --bool bash.showUntrackedFiles)" != false then u='%' fi And do the same for the other one this patch copies the above from. No need to re-roll this patch, though. It is a separate clean-up to be done on top, once this series is settles. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Makefile: don't run rm without any files
From: Matt Kraai "rm -f -r" fails on QNX when not passed any files to remove. This breaks the clean target, since dep_dirs is empty. Avoid this by merging two rm command lines. Signed-off-by: Matt Kraai --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 5a2e02d..c2e3666 100644 --- a/Makefile +++ b/Makefile @@ -2414,8 +2414,7 @@ clean: profile-clean builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) - $(RM) -r bin-wrappers - $(RM) -r $(dep_dirs) + $(RM) -r bin-wrappers $(dep_dirs) $(RM) -r po/build/ $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope* $(RM) -r $(GIT_TARNAME) .doc-tmp-dir -- 1.8.1.3.570.g3074c9d -- 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 3/4] count-objects: report garbage files in pack directory too
Nguyễn Thái Ngọc Duy writes: > prepare_packed_git_one() is modified to allow count-objects to hook a > report function to so we don't need to duplicate the pack searching > logic in count-objects.c. When report_pack_garbage is NULL, the > overhead is insignificant. > > The garbage is reported with warning() instead of error() in packed > garbage case because it's not an error to have garbage. Loose garbage > is still reported as errors and will be converted to warnings later. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- Thanks. Tests look good and the series is getting much closer. > diff --git a/sha1_file.c b/sha1_file.c > index 239bee7..5bedf78 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -21,6 +21,7 @@ > #include "sha1-lookup.h" > #include "bulk-checkin.h" > #include "streaming.h" > +#include "dir.h" > > #ifndef O_NOATIME > #if defined(__linux__) && (defined(__i386__) || defined(__PPC__)) > @@ -1000,6 +1001,57 @@ void install_packed_git(struct packed_git *pack) > packed_git = pack; > } > > +void (*report_garbage)(const char *desc, const char *path); > + > +static void report_helper(const struct string_list *list, > + int seen_bits, int first, int last) > +{ > + const char *msg; > + switch (seen_bits) { > + case 0: msg = "no corresponding .idx nor .pack"; break; > + case 1: msg = "no corresponding .idx"; break; > + case 2: msg = "no corresponding .pack"; break; That's dense. > + default: > + return; > + } > + for (; first <= last; first++) This looks odd. If you use the usual last+1 convention between the caller and callee, you do not have to do this, or call this function with "i - 1" and "list->nr -1" as the last parameter. > +static void report_pack_garbage(struct string_list *list) > +{ > + int i, baselen = -1, first = 0, seen_bits = 0; > + > + if (!report_garbage) > + return; > + > + sort_string_list(list); > + > + for (i = 0; i < list->nr; i++) { > + const char *path = list->items[i].string; > + if (baselen != -1 && > + strncmp(path, list->items[first].string, baselen)) { > + report_helper(list, seen_bits, first, i - 1); > + baselen = -1; > + seen_bits = 0; > + } > + if (baselen == -1) { > + const char *dot = strrchr(path, '.'); > + if (!dot) { > + report_garbage("garbage found", path); > + continue; > + } > + baselen = dot - path + 1; > + first = i; > + } > + if (!strcmp(path + baselen, "pack")) > + seen_bits |= 1; > + else if (!strcmp(path + baselen, "idx")) > + seen_bits |= 2; > + } > + report_helper(list, seen_bits, first, list->nr - 1); > +} > @@ -1009,6 +1061,7 @@ static void prepare_packed_git_one(char *objdir, int > local) > int len; > DIR *dir; > struct dirent *de; > + struct string_list garbage = STRING_LIST_INIT_DUP; > > sprintf(path, "%s/pack", objdir); > len = strlen(path); > ... > @@ -1043,8 +1106,20 @@ static void prepare_packed_git_one(char *objdir, int > local) > (p = add_packed_git(path, len + namelen, local)) != > NULL) > install_packed_git(p); > } > + > + if (!report_garbage) > + continue; > + > + if (has_extension(de->d_name, ".idx") || > + has_extension(de->d_name, ".pack") || > + has_extension(de->d_name, ".keep")) > + string_list_append(&garbage, path); It might be OK to put .pack and .keep in the same "if (A || B)" as it may happen to be that they do not need any special treatment right now, but I do not think this is a good idea in general. You would want to do things differently for ".idx", e.g. diff --git a/sha1_file.c b/sha1_file.c index 5bedf78..450521f 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1076,6 +1076,7 @@ static void prepare_packed_git_one(char *objdir, int local) while ((de = readdir(dir)) != NULL) { int namelen = strlen(de->d_name); struct packed_git *p; + int is_a_bad_idx = 0; if (len + namelen + 1 > sizeof(path)) { if (report_garbage) { @@ -1105,12 +1106,14 @@ static void prepare_packed_git_one(char *objdir, int local) */ (p = add_packed_git(path, len + namelen, local)) != NULL) install_packed_git(p); + else + is_a_bad_idx = 1; } if (!report_garbage) continue; -
Re: Git-aware HTTP transport docs
Scott Chacon writes: > I don't believe it was ever merged into the Git docs. I have a copy of it > here: > > https://www.dropbox.com/s/pwawp8kmwgyc3w2/http-protocol.txt Thanks for a pointer. It seems that it wasn't in a shape ready to be "merged" yet. Does somebody want to pick it up and polish it further? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
Andrew Ardill writes: > On 13 February 2013 11:34, Junio C Hamano wrote: >> The change could negatively affect people who expect that removing >> files that are not used for their purpose (e.g. a large file that is >> unnecessary for their build) will _not_ affect what they get from >> "git add ."; > > How big a problem is this? As you said below, it could be fairly big, if you expect a lot of people do not use "git add -u". > If we need to support this behaviour than I would suppose a config > option is required. A default config transition path similar to git > push defaults would probably work well, in the case where breaking > these expectations is unacceptable. We've discussed that before. http://thread.gmane.org/gmane.comp.version-control.git/171811/focus=171818 >> obviously they must have trained themselves not to do >> "git add -u" or "git commit -a". > > Many people use git add -p by default, so I would not be surprised > about people not using -u or -a. -- 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 v2] git-multimail: a replacement for post-receive-email
On Wednesday 13 February 2013 14:56:25 Matthieu Moy wrote: > Michael Haggerty writes: > I think adding a short "dependencies" section in the README (or in an > INSTALL file) saying which Python version works could save new users the > trouble (I see the sheebang inside the scripts says python2 but since I > couldn't use my system's python and called > "path/to/python git_multimail.py", this didn't help). Making the script > portable with python 2 and 3 would be awesome ;-). For my 2p worth, I don't like seeing hooks called like this. Particular those that come as part of the standard installation. I call mine by installing little scripts like this (on Debian): #!/bin/sh # stored as $GIT_WORK_DIR/.git/hooks/post-receive-email exec /bin/sh /usr/share/git-core/contrib/hooks/post-receive-email This means I don't have to make the sample script executable, it gets upgraded automatically as git gets upgraded, and the interpreter is easily changed by changing a file in my work directory, rather than altering a packaged file. I'd prefer to see the /usr/share/git-core/templates/hooks/ using a similar technique, as to my mind, installing a full copy of the sample script in every new repository is wasteful and leaves you with potentially out-of-date scripts when you update git. Andy -- Dr Andy Parkins andypark...@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
Re: [RFC v2] git-multimail: a replacement for post-receive-email
Michael Haggerty writes: > A while ago, I submitted an RFC for adding a new email notification > script to "contrib" [1]. The reaction seemed favorable and it was > suggested that the new script should replace post-receive-email rather > than be added separately, ideally with some kind of migration support. I think replacing the old post-receive-email is a sane goal in the long run, but a good first step would be to have git-multimail merged in contrib, and start considering the old script as deprecated (keeping the old script doesn't harm IMHO, it's a one-file, 3 commits/year script, not really a maintainance burden). I started playing with git-multimail. In short, I do like it but had to fight a bit with python to get it to work, and couldn't get it to do exactly what I expect. Pull request attached :-). Installation troubles: I had an old python installation (Red Hat package, and I'm not root), that did not include the email.utils package, so I couldn't use my system's python. I found no indication about python version in README, so I installed the latest python by hand, just to find out that git-multimail wasn't compatible with Python 3.x. 2to3 can fix automatically a number of 3.x compatibility issues, but not all of them so I gave up and installed Python 2.7. I think adding a short "dependencies" section in the README (or in an INSTALL file) saying which Python version works could save new users the trouble (I see the sheebang inside the scripts says python2 but since I couldn't use my system's python and called "path/to/python git_multimail.py", this didn't help). Making the script portable with python 2 and 3 would be awesome ;-). Missing feature: git-multimail can send a summary for each push, with the "git log --oneline" of the new revisions, and then 1 mail per patch with the complete log and the patch. I'd like to have the intermediate: allow the summary email to include the complete log (not just --oneline). My colleagues already think they receive too many emails so I don't think they'd like the "one email per commit" way, but the 1 line summary is really short OTOH. I wrote a quick and hopefully not-too-dirty implementation of it, there's a pull request here: https://github.com/mhagger/git-multimail/pull/6 essentially, it boils down to: @@ -835,6 +837,17 @@ class ReferenceChange(Change): for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE): yield line +if adds and self.showlog: +yield '\n' +yield 'Detailed log of added commits:\n\n' +for line in read_lines( +['git', 'log'] ++ self.logopts ++ ['%s..%s' % (self.old.commit, self.new.commit,)], +keepends=True, +): +yield line + # The diffstat is shown from the old revision to the new # revision. This is to show the truth of what happened in # this change. There's no point showing the stat from the -- 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
[ANN] First beta: Git export with hardlinks
Hi, my git_export_hardlink command should now be in a usable state. I'd appreciate any feedback: https://github.com/thkoch2001/git_export_hardlinks I still have to choose a license: BSD/GPL/? Jeff King: > It looks like you create the sha1->path mapping by asking the user to > provide , pairs, and then assuming that the exported > tree at exactly matches . Which it would in the > workflow you've proposed, but it is also easy for that not to be the > case (e.g., somebody munges a file in after it has been > exported). > > So it's a bit dangerous as a general purpose tool, IMHO. It's also a > slight pain in that you have to keep track of the tree sha1 for each > exported path somehow. You're right. I'd run a git reset --hard after each export to guarantee a pristine export. The tree sha1 of the exported tree might be part of the folder name of the export or in some meta file related to the export, like /deployments /2012-03-05_14-23-02_0b96bf5f72d2c282b31726b3fbff279a89220b15 /export <- exported tree goes here /meta <- git config file holding all relevant metadata: (who, when, tree, commit, ref) /index <- git index file corresponding to the exported tree (maybe?) Regards, Thomas Koch, http://www.koch.ro -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn init throws Not a git repository (or any of the parent directories): .git
On Wed, 13 Feb 2013 14:01:36 +0100 "amccl...@gmail.com" wrote: > I have problem with git svn init: > When I execute > git svn init svn+ssh://usern...@example.com/path/repo > I see: > fatal: Not a git repository (or any of the parent directories): .git > Already at toplevel, but .git not found > at /usr/lib/git-core/git-svn line 342 Are you doing `git init` first before running `git svn init ...`? -- 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: A good Git technique for referring back to original files
MikeW yahoo.co.uk> writes: > > Paul Campbell kemitix.net> writes: > > > > > Hi Mike, > > > > I think git-cvsimport and git-subtree could help you here. > > > > That looks very interesting, had not considered git subtree and it looks like > the right kind of method. > > Thanks. > Mike The only alternative I can think of is to scan through Working_SDK and replace the files there with symlinks back to the matching files within the original subprojects - such scripts exist ! Then any changes in Working_SDK will update the (baselined) originals in-place. But then no explicit use of git except for tracking work prior to pushing back to CVS. Oh well, thanks for ideas, will see which work best. Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-multimail] License unknown (#1)
On 02/12/2013 04:28 PM, Andy Parkins wrote: > On Sunday 27 January 2013 18:52:58 Michael Haggerty wrote: >> I have a question about the license of contrib/hooks/post-commit-email. >> [...] > > Keeping up with the git mailing list got a bit much, [...] Very understandable :-) > My apologies to everyone; I've been lax supporting the script -- I never > really expected it to be as widely used as it has been, I was always > expecting > someone would come along and replace it so I'm pleased that Michael has done > so (although it's a little disheartening to read of it being called "hacky", > when I tried very hard to make it as clear and modular as I could). FWIW I think the script was quite well-structured (within the limitations of shell scripting anyway) and I had no problem understanding it and translating it to Python. >> If somebody can explain what license the code is under and how they come >> to that conclusion, I would be very grateful. >> >> And if Andy Parkins (the original author) is listening, please indicate >> whether you had any intent *other* than GPLv2. > > I intended it to be under the same license as Git. I had read in one of the > patch submission files (which I can't seem to find now) that all submissions > were considered part of git. Thanks. Given that you submitted the original version under GPLv2, and later contributors certainly had no reason to assume any *other* license, I think any reasonable person will be satisfied that the whole script in its current form is under GPLv2. > If an explicit declaration is needed, I am happy to give it. Let me know > what > form this should take and I'll supply it. Personally, I am satisfied by your email statement. I will leave the derived work, git-multimail, also under GPLv2. Assuming that git-multimail supersedes yours, its explicit choice of license will make the situation clear for future users. Thanks! Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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: A good Git technique for referring back to original files
Paul Campbell kemitix.net> writes: > > Hi Mike, > > I think git-cvsimport and git-subtree could help you here. > That looks very interesting, had not considered git subtree and it looks like the right kind of method. Thanks. Mike > Hope that helps. > > -- > Paul > ... Super-Snip ... > > -- > Paul [W] Campbell > -- 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 3/3] t9903: add extra tests for bash.showDirtyState
Added 3 extra tests for the bash.showDirtyState config option, tests should now cover all combinations of the shell var being set/unset and the config option being enabled/disabled, given a dirty file. * Renamed test 'disabled by config' to 'shell variable set with config disabled' for consistency --- t/t9903-bash-prompt.sh | 38 +- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index cb008e2..fa81b09 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - before root commit' ' test_cmp expected "$actual" ' -test_expect_success 'prompt - dirty status indicator - disabled by config' ' +test_expect_success 'prompt - dirty status indicator - shell variable unset with config disabled' ' printf " (master)" > expected && echo "dirty" > file && test_when_finished "git reset --hard" && test_config bash.showDirtyState false && ( + unset -v GIT_PS1_SHOWDIRTYSTATE && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - dirty status indicator - shell variable unset with config enabled' ' + printf " (master)" > expected && + echo "dirty" > file && + test_when_finished "git reset --hard" && + test_config bash.showDirtyState true && + ( + unset -v GIT_PS1_SHOWDIRTYSTATE && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config disabled' ' + printf " (master)" > expected && + echo "dirty" > file && + test_when_finished "git reset --hard" && + test_config bash.showDirtyState false && + ( + GIT_PS1_SHOWDIRTYSTATE=y && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config enabled' ' + printf " (master *)" > expected && + echo "dirty" > file && + test_when_finished "git reset --hard" && + test_config bash.showDirtyState true && + ( GIT_PS1_SHOWDIRTYSTATE=y && __git_ps1 > "$actual" ) && -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] t9903: add tests for bash.showUntrackedFiles
Add 4 test for the bash.showUntrackedFiles config option, covering all combinations of the shell var being set/unset and the config option being enabled/disabled. Signed-off-by: Martin Erik Werner --- t/t9903-bash-prompt.sh | 40 1 file changed, 40 insertions(+) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index f17c1f8..cb008e2 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status indicator - untracked files test_cmp expected "$actual" ' +test_expect_success 'prompt - untracked files status indicator - shell variable unset with config disabled' ' + printf " (master)" > expected && + test_config bash.showUntrackedFiles false && + ( + unset -v GIT_PS1_SHOWUNTRACKEDFILES && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - shell variable unset with config enabled' ' + printf " (master)" > expected && + test_config bash.showUntrackedFiles true && + ( + unset -v GIT_PS1_SHOWUNTRACKEDFILES && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - shell variable set with config disabled' ' + printf " (master)" > expected && + test_config bash.showUntrackedFiles false && + ( + GIT_PS1_SHOWUNTRACKEDFILES=y && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - untracked files status indicator - shell variable set with config enabled' ' + printf " (master %%)" > expected && + test_config bash.showUntrackedFiles true && + ( + GIT_PS1_SHOWUNTRACKEDFILES=y && + __git_ps1 > "$actual" + ) && + test_cmp expected "$actual" +' + test_expect_success 'prompt - untracked files status indicator - not shown inside .git directory' ' printf " (GIT_DIR!)" > expected && ( -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] shell prompt: add bash.showUntrackedFiles option
Add a config option 'bash.showUntrackedFiles' which allows enabling the prompt showing untracked files on a per-repository basis. This is useful for some repositories where the 'git ls-files ...' command may take a long time. Signed-off-by: Martin Erik Werner --- contrib/completion/git-prompt.sh | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 9bef053..9b2eec2 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -43,7 +43,10 @@ # # If you would like to see if there're untracked files, then you can set # GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there're untracked -# files, then a '%' will be shown next to the branch name. +# files, then a '%' will be shown next to the branch name. You can +# configure this per-repository with the bash.showUntrackedFiles +# variable, which defaults to true once GIT_PS1_SHOWUNTRACKEDFILES is +# enabled. # # If you would like to see the difference between HEAD and its upstream, # set GIT_PS1_SHOWUPSTREAM="auto". A "<" indicates you are behind, ">" @@ -332,8 +335,10 @@ __git_ps1 () fi if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then - if [ -n "$(git ls-files --others --exclude-standard)" ]; then - u="%" + if [ "$(git config --bool bash.showUntrackedFiles)" != "false" ]; then + if [ -n "$(git ls-files --others --exclude-standard)" ]; then + u="%" + fi fi fi -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] Add bash.showUntrackedFiles config option
On Tue, 2013-02-12 at 14:29 -0800, Junio C Hamano wrote: > Martin Erik Werner writes: > > > Add a test case for the bash.showUntrackedFiles config option, which > > checks that the config option can disable the global effect of the > > GIT_PS1_SHOWUNTRACKEDFILES environmant variable. > > > > Signed-off-by: Martin Erik Werner > > --- > > t/t9903-bash-prompt.sh | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > > index f17c1f8..c9417b9 100755 > > --- a/t/t9903-bash-prompt.sh > > +++ b/t/t9903-bash-prompt.sh > > @@ -447,6 +447,17 @@ test_expect_success 'prompt - untracked files status > > indicator - not shown insid > > test_cmp expected "$actual" > > ' > > > > +test_expect_success 'prompt - untracked files status indicator - disabled > > by config' ' > > + printf " (master)" > expected && > > + echo "untracked" > file_untracked && > > + test_config bash.showUntrackedFiles false && > > + ( > > + GIT_PS1_SHOWUNTRACKEDFILES=y && > > + __git_ps1 > "$actual" > > + ) && > > + test_cmp expected "$actual" > > +' > > All six combinations need checking: > > * not having the configuration at all and not having the shell >variable should not show the untracked indicator (already tested). > > * not having the configuration at all and having the shell variable >should show the untracked indicator (already tested). > > * setting configuration to true without having the shell variable >should not show the untracked indicator. > > * setting configuration to true and having the shell variable >should show the unttracked indicator. > > * setting configuration to false and having the shell variable >should not show the untracked indicator (the above test checks >this). > > * setting configuration to false without having the shell variable >should not show the untracked indicator. > > to prevent others from breaking the code you wrote for [PATCH 1/2], > so you need three more tests, I guess? Ah, yes, I was mimicing what the test did for bash.showDirtyState, I've now added the three extra tests for bash.showUntrackedFiles, which should cover all of the above cases, hopefully? I've also added in the three extra tests for bash.showDirtyState, equivalently. These only cover the case of dirty files and not combinations with content in index, which I felt was a bit overkill, is that reasonable? Thanks for the review :) -- Martin Erik Werner -- 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: inotify to minimize stat() calls
On Tue, Feb 12, 2013 at 09:48:18PM +0100, Karsten Blees wrote: > However, the difference between git status -uall and -uno was always > about 1.3 s in all fscache versions, even though > opendir/readdir/closedir was served entirely from the cache. I added > a bit of performance tracing to find the cause, and I think most of > the time spent in wt_status_collect_untracked can be eliminated: > > 1.) 0.939 s is spent in dir.c/excluded (i.e. checking > .gitignore). This check is done for *every* file in the working > copy, including files in the index. Checking the index first could > eliminate most of that, i.e.: > > (Note: patches are for discussion only, I'm aware that they may have > unintended side effects...) > > @@ -1097,6 +1097,8 @@ static enum path_treatment treat_path(struct dir_struct > *dir, > return path_ignored; > strbuf_setlen(path, baselen); > strbuf_addstr(path, de->d_name); > + if (cache_name_exists(path->buf, path->len, ignore_case)) > + return path_ignored; > if (simplify_away(path->buf, path->len, simplify)) > return path_ignored; The below patch passes the test suite for me and still does the same thing. On my Linux box, running "git status" on gentoo-x86.git with this patch saves 0.05s (0.548s without the patch, 0.505s with the patch, best number of 20 runs). And I just realized gentoo-x86.git does not have any .gitignore. On webkit.git, it cuts "git status" time from 1.121s down to 0.762s. Unless I'm mistaken, "git add" should have the same benefit on normal case too. Good finding! -- 8< -- diff --git a/dir.c b/dir.c index 57394e4..4b4cf60 100644 --- a/dir.c +++ b/dir.c @@ -1244,7 +1244,19 @@ static enum path_treatment treat_one_path(struct dir_struct *dir, const struct path_simplify *simplify, int dtype, struct dirent *de) { - int exclude = is_excluded(dir, path->buf, &dtype); + int exclude; + + if (dtype == DT_UNKNOWN) + dtype = get_dtype(de, path->buf, path->len); + + if (!(dir->flags & DIR_SHOW_IGNORED) && + !(dir->flags & DIR_COLLECT_IGNORED) && + dtype == DT_REG && + cache_name_exists(path->buf, path->len, ignore_case)) + return path_ignored; + + exclude = is_excluded(dir, path->buf, &dtype); + if (exclude && (dir->flags & DIR_COLLECT_IGNORED) && exclude_matches_pathspec(path->buf, path->len, simplify)) dir_add_ignored(dir, path->buf, path->len); @@ -1256,9 +1268,6 @@ static enum path_treatment treat_one_path(struct dir_struct *dir, if (exclude && !(dir->flags & DIR_SHOW_IGNORED)) return path_ignored; - if (dtype == DT_UNKNOWN) - dtype = get_dtype(de, path->buf, path->len); - switch (dtype) { default: return path_ignored; -- 8< -- -- 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 v4 4/4] count-objects: report how much disk space taken by garbage files
Also issue warnings on loose garbages instead of errors as a result of using report_garbage() function in count_objects() Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-count-objects.txt | 2 ++ builtin/count-objects.c | 18 -- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt index 1611d7c..da6e72e 100644 --- a/Documentation/git-count-objects.txt +++ b/Documentation/git-count-objects.txt @@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git prune-packed`. + garbage: the number of files in object database that are not valid loose objects nor valid packs ++ +size-garbage: disk space consumed by garbage files, in KiB GIT --- diff --git a/builtin/count-objects.c b/builtin/count-objects.c index 1706c8b..3a01a8d 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -10,9 +10,13 @@ #include "parse-options.h" static unsigned long garbage; +static off_t size_garbage; static void real_report_garbage(const char *desc, const char *path) { + struct stat st; + if (!stat(path, &st)) + size_garbage += st.st_size; warning("%s: %s", desc, path); garbage++; } @@ -20,8 +24,7 @@ static void real_report_garbage(const char *desc, const char *path) static void count_objects(DIR *d, char *path, int len, int verbose, unsigned long *loose, off_t *loose_size, - unsigned long *packed_loose, - unsigned long *garbage) + unsigned long *packed_loose) { struct dirent *ent; while ((ent = readdir(d)) != NULL) { @@ -54,9 +57,11 @@ static void count_objects(DIR *d, char *path, int len, int verbose, } if (bad) { if (verbose) { - error("garbage found: %.*s/%s", - len + 2, path, ent->d_name); - (*garbage)++; + struct strbuf sb = STRBUF_INIT; + strbuf_addf(&sb, "%.*s/%s", + len + 2, path, ent->d_name); + report_garbage("garbage found", sb.buf); + strbuf_release(&sb); } continue; } @@ -107,7 +112,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) if (!d) continue; count_objects(d, path, len, verbose, - &loose, &loose_size, &packed_loose, &garbage); + &loose, &loose_size, &packed_loose); closedir(d); } if (verbose) { @@ -132,6 +137,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) printf("size-pack: %lu\n", (unsigned long) (size_pack / 1024)); printf("prune-packable: %lu\n", packed_loose); printf("garbage: %lu\n", garbage); + printf("size-garbage: %lu\n", (unsigned long) (size_garbage / 1024)); } else printf("%lu objects, %lu kilobytes\n", -- 1.8.1.2.536.gf441e6d -- 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 v4 2/4] sha1_file: reorder code in prepare_packed_git_one()
The current loop does while (...) { if (!not .idx file) continue; process .idx file; } and is reordered to while (...) { if (!.idx file) { process .idx file; } } This makes it easier to add new extension file processing. Signed-off-by: Nguyễn Thái Ngọc Duy --- sha1_file.c | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 40b2329..239bee7 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1024,27 +1024,25 @@ static void prepare_packed_git_one(char *objdir, int local) int namelen = strlen(de->d_name); struct packed_git *p; - if (!has_extension(de->d_name, ".idx")) - continue; - if (len + namelen + 1 > sizeof(path)) continue; - /* Don't reopen a pack we already have. */ strcpy(path + len, de->d_name); - for (p = packed_git; p; p = p->next) { - if (!memcmp(path, p->pack_name, len + namelen - 4)) - break; + + if (has_extension(de->d_name, ".idx")) { + /* Don't reopen a pack we already have. */ + for (p = packed_git; p; p = p->next) { + if (!memcmp(path, p->pack_name, len + namelen - 4)) + break; + } + if (p == NULL && + /* +* See if it really is a valid .idx file with +* corresponding .pack file that we can map. +*/ + (p = add_packed_git(path, len + namelen, local)) != NULL) + install_packed_git(p); } - if (p) - continue; - /* See if it really is a valid .idx file with corresponding -* .pack file that we can map. -*/ - p = add_packed_git(path, len + namelen, local); - if (!p) - continue; - install_packed_git(p); } closedir(dir); } -- 1.8.1.2.536.gf441e6d -- 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 v4 3/4] count-objects: report garbage files in pack directory too
prepare_packed_git_one() is modified to allow count-objects to hook a report function to so we don't need to duplicate the pack searching logic in count-objects.c. When report_pack_garbage is NULL, the overhead is insignificant. The garbage is reported with warning() instead of error() in packed garbage case because it's not an error to have garbage. Loose garbage is still reported as errors and will be converted to warnings later. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-count-objects.txt | 4 +- builtin/count-objects.c | 12 +- cache.h | 3 ++ sha1_file.c | 77 - t/t5304-prune.sh| 26 + 5 files changed, 118 insertions(+), 4 deletions(-) diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt index e816823..1611d7c 100644 --- a/Documentation/git-count-objects.txt +++ b/Documentation/git-count-objects.txt @@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB prune-packable: the number of loose objects that are also present in the packs. These objects could be pruned using `git prune-packed`. + -garbage: the number of files in loose object database that are not -valid loose objects +garbage: the number of files in object database that are not valid +loose objects nor valid packs GIT --- diff --git a/builtin/count-objects.c b/builtin/count-objects.c index 9afaa88..1706c8b 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -9,6 +9,14 @@ #include "builtin.h" #include "parse-options.h" +static unsigned long garbage; + +static void real_report_garbage(const char *desc, const char *path) +{ + warning("%s: %s", desc, path); + garbage++; +} + static void count_objects(DIR *d, char *path, int len, int verbose, unsigned long *loose, off_t *loose_size, @@ -76,7 +84,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) const char *objdir = get_object_directory(); int len = strlen(objdir); char *path = xmalloc(len + 50); - unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0; + unsigned long loose = 0, packed = 0, packed_loose = 0; off_t loose_size = 0; struct option opts[] = { OPT__VERBOSE(&verbose, N_("be verbose")), @@ -87,6 +95,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) /* we do not take arguments other than flags for now */ if (argc) usage_with_options(count_objects_usage, opts); + if (verbose) + report_garbage = real_report_garbage; memcpy(path, objdir, len); if (len && objdir[len-1] != '/') path[len++] = '/'; diff --git a/cache.h b/cache.h index 7339f21..73de68c 100644 --- a/cache.h +++ b/cache.h @@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char *feature_list, const char *fea extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path); +/* A hook for count-objects to report invalid files in pack directory */ +extern void (*report_garbage)(const char *desc, const char *path); + extern void prepare_packed_git(void); extern void reprepare_packed_git(void); extern void install_packed_git(struct packed_git *pack); diff --git a/sha1_file.c b/sha1_file.c index 239bee7..5bedf78 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -21,6 +21,7 @@ #include "sha1-lookup.h" #include "bulk-checkin.h" #include "streaming.h" +#include "dir.h" #ifndef O_NOATIME #if defined(__linux__) && (defined(__i386__) || defined(__PPC__)) @@ -1000,6 +1001,57 @@ void install_packed_git(struct packed_git *pack) packed_git = pack; } +void (*report_garbage)(const char *desc, const char *path); + +static void report_helper(const struct string_list *list, + int seen_bits, int first, int last) +{ + const char *msg; + switch (seen_bits) { + case 0: msg = "no corresponding .idx nor .pack"; break; + case 1: msg = "no corresponding .idx"; break; + case 2: msg = "no corresponding .pack"; break; + default: + return; + } + for (; first <= last; first++) + report_garbage(msg, list->items[first].string); +} + +static void report_pack_garbage(struct string_list *list) +{ + int i, baselen = -1, first = 0, seen_bits = 0; + + if (!report_garbage) + return; + + sort_string_list(list); + + for (i = 0; i < list->nr; i++) { + const char *path = list->items[i].string; + if (baselen != -1 && + strncmp(path, list->items[first].string, baselen)) { + report_helper(list, seen_bits, first, i - 1); + baselen = -1; + seen_bits = 0; +
[PATCH v4 1/4] git-count-objects.txt: describe each line in -v output
The current description requires a bit of guessing (what clause corresponds to what printed line?) and lacks information, such as the unit of size and size-pack. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-count-objects.txt | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt index 23c80ce..e816823 100644 --- a/Documentation/git-count-objects.txt +++ b/Documentation/git-count-objects.txt @@ -20,11 +20,21 @@ OPTIONS --- -v:: --verbose:: - In addition to the number of loose objects and disk - space consumed, it reports the number of in-pack - objects, number of packs, disk space consumed by those packs, - and number of objects that can be removed by running - `git prune-packed`. + Report in more detail: ++ +count: the number of loose objects ++ +size: disk space consumed by loose objects, in KiB ++ +in-pack: the number of in-pack objects ++ +size-pack: disk space consumed by the packs, in KiB ++ +prune-packable: the number of loose objects that are also present in +the packs. These objects could be pruned using `git prune-packed`. ++ +garbage: the number of files in loose object database that are not +valid loose objects GIT --- -- 1.8.1.2.536.gf441e6d -- 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 v4 0/4] count-objects improvements
On Wed, Feb 13, 2013 at 12:23 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> +/* A hook for count-objects to report invalid files in pack directory */ >> +extern void (*report_garbage)(const char *desc, const char *path, int len, >> const char *name); > > We may want to document the strange way the last three parameters > are used somewhere. e.g. > > shows "path" (if "name" is NULL), or prepends "path" in > front of name (otherwise); only for the latter, "path" can > be a string that is not NUL-terminated but its length > specified with "len" and in that case a slash is inserted > between the path and the "name". > > When described clearly, it sounds somewhat ugly and incoherent API, > even though it covers the immediate need X-<. One of the reasons why I did not export it explicitly. Changed it to void (*report_garbage)(const char *desc, const char *path); and pushed the ugly part back to callers. > How about doing it something along this line, perhaps? > > int i; > int beginning_of_this_name = -1; > int seen_bits = 0; /* 01 for .idx, 02 for .pack */ > for (i = 0; i < list->nr; i++) { > if (beginning_of_this_name < 0) > beginning_of_this_name = i; > else if (list->items[i] and > list->items[beginning_of_this_name] > share the same basename) > ; /* keep scanning */ > else { > /* one name ended at (i-1) */ > if (seen_bits == 3) > ; /* both .idx and .pack exist; good */ > else > report_garbage_for_one_name(list, > beginning_of_this_name, i, > seen_bits); > seen_bits = 0; > beginning_of_this_name = i; > } > if (list->items[i] is ".idx") > seen_bits |= 1; > if (list->items[i] is ".pack") > seen_bits |= 2; > > } > if (0 <= beginning_of_this_name && seen_bits != 3) > report_garbages_for_one_name(list, beginning_of_this_name, > list->nr, seen_bits); > > with a helper function report_garbage_for_one_name() that would look like > this: > > report_garbage_for_one_name(...) { > int j; > const char *msg; > switch (seen_bits) { > case 0: msg = "no corresponding .idx nor .pack"; break; > case 1: msg = "no corresponding .pack"; break; > case 2: msg = "no corresponding .idx; break; > } > for (j = beginning_of_this_name; j < i; j++) > report_garbage(msg, list->items[j]); > } > > For the above to work, prepare_packed_git_one() needs to retain only the > paths with known extensions in garbage list. "pack-deadbeef.unk" can and > should be reported as a garbage immediately when it is seen without being > placed in the list. Yup. Looks good. >> + } else if (has_extension(de->d_name, ".idx")) { >> + struct string_list_item *item; >> + int n = strlen(path) - 4; >> + item = string_list_append_nodup(&garbage, >> + xstrndup(path, n)); >> + item->util = ".idx"; >> + continue; >> + } else >> + report_garbage("garbage found", path, 0, NULL); > > Hmm, where is a ".keep" file handled in this flow? Apparently I smoked/drank while coding or something. .idx is supposed to be .keep. This calls for a test to guard my code (part of this v4). > The structure of the if/else cascade is much nicer than the earlier > iterations, but wouldn't it be even more clear to do this? > > if (is .idx file) { > ... do that .idx thing ... > } > > if (!report_garbage) > continue; /* it does not matter what the file is */ > > if (is .pack) { > ... remember that we saw this .pack ... > } else if (is .idx) { > ... remember that we saw this .idx ... > } else if (is .keep) { > ... remember that we saw this .keep ... > } else { > ... all else --- report as garbage immediately ... > } Done. 2/4 is updated to make sure the "if (is .idx file)" block does not shortcut the loop with "continue;" so that we always get .idx file in the end of the loop. Nguyễn Thái Ngọc Duy (4): git-count-objects.txt: describe each line in -v output sha1_file: reorder code in prepare_packed_git_one() count-objects: report garbage files in pack directory too count-objects: report how much disk spa
Re: Pushing a git repository to a new server
Jeff King venit, vidit, dixit 12.02.2013 21:42: > On Tue, Feb 12, 2013 at 12:28:53PM +0100, Michael J Gruber wrote: > >> I'm not sure providers like GitHub would fancy an interface which allows >> the programmatic creation of repos (giving a new meaning to "fork >> bomb"). But I bet you know better ;-) > > You can already do that: > > http://developer.github.com/v3/repos/#create Nice. I knew you knew ;) > We rate-limit API requests, and I imagine we might do something similar > with create-over-git. But that is exactly the kind of implementation > detail that can go into a custom create-repo script. > >> An alternative would be to teach git (the client) about repo types and >> how to create them. After all, a repo URL "ssh://host/path" gives a >> clear indication that "ssh host git init path" will create a repo. > > But that's the point of a microformat. It _doesn't_ always work, because > the server may not allow arbitrary commands, or may have special > requirements on top of the "init". You can make the microformat be "git > init path", and servers can intercept calls to "git init" and translate > them into custom magic. But I think the world is a little simpler if we > define a new service type (alongside git-upload-pack, git-receive-pack, > etc), and let clients request it. Then it's clear what the client is > trying to do, it's easy for servers to hook into it, we can request it > over http, etc. And it can be extended over time to take more fields > (like repo description, etc). > > I'm really not suggesting anything drastic. The wrapper case for ssh > would be as simple as a 3-line shell script which calls "git init" under > the hood, but it provides one level of indirection that makes > replacing/hooking it much simpler for servers. So the parts that are in > stock git would not be much work (most of the work would be on _calling_ > it, but that is the same for adding a call to "git init"). > > I think the main reason the idea hasn't gone anywhere is that nobody > really cares _that_ much. People just don't create repositories that > often. I feel like this is one of those topics that comes up once a > year, and then nothing happens on it, because people just make their > repo manually and then stop caring about it. > > Just my two cents, of course. :) Most repos are probably created by a local "git init" or "git clone", or by clicking a button on a provider's web interface. The need for git-create-repo seems to be restricted to: - "command line folks" who use a provider for it's hosting service and don't fancy a web interface for repo creation - noobs who need to get their head wrapped around local, remote, push/pull 'n' stuff... For the server side git-create-repo to take off we would probably need two things (besides the client support): - Implement and ship a git-create-repo which makes this work for git over ssh seamlessly. (Will take some to trickle down to servers in the wild.) - Get a large provider to offer this. Gitosis/Gitolite are probably to follow easily. I'm beginning to like idea ;) Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html