Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Eric Sunshine sunshine at sunshineco.com writes:
On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe neminaa at
gmail.com wrote:
Nemina Amarasinghe neminaa at gmail.com writes:
Sorry for the first patch. Something went wrong. This is the correct one
In addition to the tautological issue pointed out by Matthieu, please
note that this version of the patch is not the correct one. It won't
apply to the git code. At a guess, it appears that this patch is
against some other modification you made to the git code before this
change, or perhaps you committed it incorrectly. In any event, when
you resubmit, please be sure that the new version can be applied to
commit.c as it exists in git.git itself.
Thank you very much for your comments Eric. I will resubmit the patch.
Just a quick question
in this code
if (flag BRANCH_CONFIG_VERBOSE) {
if (remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote branch %s from %s by
rebasing.) :
_(Branch %s set up to track remote branch %s from %s.),
local, shortname, origin);
else if (remote_is_branch !origin)
printf_ln(rebasing ?
_(Branch %s set up to track local branch %s by rebasing.) :
_(Branch %s set up to track local branch %s.),
local, shortname);
else if (!remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else if (!remote_is_branch !origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else
die(BUG: impossible combination of %d and %p,
remote_is_branch, origin);
}
These local and remote variables are independent from the origin right?
So, If that the case couldn't we just use the bellow function
else if (!remote_is_branch)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
instead of
else if (!remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else if (!remote_is_branch !origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
Thanks
Nemina
--
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][GSoC]simplified branch.c:install_branch_config() if() statement
I hope this is the correct format for patch. Please comment on this if something is wrong. Signed-off-by:Nemina Amarasinghe nemi...@gmail.com --- branch.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 0304a7a..fd93603 100644 --- a/branch.c +++ b/branch.c @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(Branch %s set up to track local branch %s by rebasing.) : _(Branch %s set up to track local branch %s.), local, shortname); - else if (!remote_is_branch origin) - printf_ln(rebasing ? - _(Branch %s set up to track remote ref %s by rebasing.) : - _(Branch %s set up to track remote ref %s.), - local, remote); - else if (!remote_is_branch !origin) + else if (!remote_is_branch) printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), -- 1.9.0.152.g6ab4ae2 -- 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 nd/gitignore-trailing-whitespace] t0008: skip trailing space test on Windows
From: Johannes Sixt j...@kdbg.org The Windows API does not preserve file names with trailing spaces (and dots), but rather strips them. Our tools (MSYS bash, git) base the POSIX emulation on the Windows API. As a consequence, it is impossible for bash on Windows to allocate a file whose name has trailing spaces, and for git to stat such a file. Both operate on a file whose name has the spaces stripped. Skip the test that needs such a file name. Note that we do not use (another incarnation of) prerequisite FUNNYNAMES. The reason is that FUNNYNAMES is intended to represent a property of the file system. But the inability to have trailing spaces in a file name is a property of the Windows API. The file system (NTFS) does not have this limitation. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t0008-ignores.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index bbaf6b5..63beb99 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -793,7 +793,7 @@ EOF test_cmp err.expect err ' -test_expect_success 'quoting allows trailing whitespace' ' +test_expect_success !MINGW 'quoting allows trailing whitespace' ' rm -rf whitespace mkdir whitespace whitespace/trailing -- 1.9.0.1534.ga05fa03 -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
On Tue, Mar 11, 2014 at 2:30 AM, Nemina Amarasinghe nemi...@gmail.com wrote:
Eric Sunshine sunshine at sunshineco.com writes:
On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe neminaa at
gmail.com wrote:
Nemina Amarasinghe neminaa at gmail.com writes:
Sorry for the first patch. Something went wrong. This is the correct one
In addition to the tautological issue pointed out by Matthieu, please
note that this version of the patch is not the correct one. It won't
apply to the git code. At a guess, it appears that this patch is
against some other modification you made to the git code before this
change, or perhaps you committed it incorrectly. In any event, when
you resubmit, please be sure that the new version can be applied to
commit.c as it exists in git.git itself.
Thank you very much for your comments Eric. I will resubmit the patch.
Just a quick question
in this code
if (flag BRANCH_CONFIG_VERBOSE) {
if (remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote branch %s from %s by
rebasing.) :
_(Branch %s set up to track remote branch %s from %s.),
local, shortname, origin);
else if (remote_is_branch !origin)
printf_ln(rebasing ?
_(Branch %s set up to track local branch %s by rebasing.)
:
_(Branch %s set up to track local branch %s.),
local, shortname);
else if (!remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else if (!remote_is_branch !origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else
die(BUG: impossible combination of %d and %p,
remote_is_branch, origin);
}
This code does not exist in git.git. Somehow, the code you are
consulting has incorrectly substituted 'remote' for 'local' in the
strings of the (!remote_is_branch !origin) case. Whether this is
due to a local modification you made or some some other reason, it is
leading you astray in your proposed changes and breaking your patches
so that they cannot be applied to git.git.
Probably easiest at this point would be for you to start from scratch
by making a new clone of git.git and examining branch.c as it actually
exists in the 'master' branch.
These local and remote variables are independent from the origin right?
So, If that the case couldn't we just use the bellow function
else if (!remote_is_branch)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
instead of
else if (!remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
else if (!remote_is_branch !origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote ref %s by rebasing.) :
_(Branch %s set up to track remote ref %s.),
local, remote);
Thanks
Nemina
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][GSoC]simplified branch.c:install_branch_config() if() statement
On Tue, Mar 11, 2014 at 3:16 AM, Nemina Amarasinghe nemi...@gmail.com wrote: Subject: simplified branch.c:install_branch_config() if() statement Use imperative tone: simplify ... I hope this is the correct format for patch. Please comment on this if something is wrong. This commentary is relevant to the email discussion but not to the commit description, so it should be placed below the --- line just under your sign-0ff. Signed-off-by:Nemina Amarasinghe nemi...@gmail.com --- branch.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 0304a7a..fd93603 100644 --- a/branch.c +++ b/branch.c @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(Branch %s set up to track local branch %s by rebasing.) : _(Branch %s set up to track local branch %s.), local, shortname); - else if (!remote_is_branch origin) - printf_ln(rebasing ? - _(Branch %s set up to track remote ref %s by rebasing.) : - _(Branch %s set up to track remote ref %s.), - local, remote); - else if (!remote_is_branch !origin) + else if (!remote_is_branch) printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), The patch itself is broken in a couple ways. First, it is whitespace-damaged, possibly due to being pasted into your email client. Using git send-email can help avoid this problem. Second, the code against which this patch was made does not exist in git.git. You are likely making this change atop some other local modifications which you already made. Simplest at this point would probably be for you to make a fresh clone of git.git and start from scratch by editing branch.c in the 'master' branch. -- 1.9.0.152.g6ab4ae2 -- 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
[PATHv2] branch.c:install_branch_config():Simplify code generating verbose message.
Simplify the long if chain in install_branch_config().
There is a long chain of if statements. The code can be more clear.
Replace the chain with table of strings. New approach is more
compact.
Signed-off-by: Paweł Wawruch pa...@aleg.pl
---
branch.c | 40 ++--
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..8d3b219 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,18 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *messages[] = {
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ };
+ const char *name = remote_is_branch ? remote : shortname;
+ int message_number;
if (remote_is_branch
!strcmp(local, shortname)
@@ -77,29 +89,13 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.) :
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
+ message_number = (!!rebasing) + 2 * (!!origin) + 4 *
(!!remote_is_branch);
+ assert(message_number ARRAY_SIZE(messages));
+
+ if (message_number 2)
+ printf_ln(messages[message_number], local, name,
origin);
else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+ printf_ln(messages[message_number], local, name);
}
}
--
1.8.3.2
--
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] submodule : Add --no-separate-git-dir option to add and update command.
Le lundi 10 mars 2014 à 21:32 +0100, Heiko Voigt a écrit : On Mon, Mar 10, 2014 at 10:08:06AM +0100, Henri GEIST wrote: Le samedi 08 mars 2014 à 00:00 +0100, Jens Lehmann a écrit : Am 06.03.2014 23:20, schrieb Henri GEIST: What is the use case you are trying to solve and why can that not be handled by adding subsubmodule inside submodule? The problem is access rights. Imagine you have 2 people Pierre and Paul. Each with different access write on the server. Pierre has full access on every things. Paul has full access on superproject and subsubmodule but no read/write access to submodule only execution on the directory. Ok, I think I'm slowly beginning to understand your setup. I want all user to get every things they are allowed to have with the command 'git submodule update --init --recursive'. Then as Paul can not clone submodule he can not get subsubmodule recursively through it. Sure, that's how it should work. Paul could only work on a branch where submodule is an empty directory containing subsubmodule, as he doesn't have the rights to clone submodule. I will not redundantly create a branch for each user on the server. When users clone the server it already create a special branch for them 'master' which track 'origin/master'. And if each user have its own branch on the server it will completely defeat the goal of the server collaboration. And transform the git server in simple rsync server. I do not think that is what Jens was suggesting. It does not matter in which branch they work, they can directly use master if you like. What he was suggesting is that they create their repository structure like this: git clone g...@somewhere.net:superproject.git cd superproject/submodule git clone g...@somehwere.net:subsubmodule.git cd subsubmodule ... work, commit, work, commit ... The same applies for the superproject. Now only someone with access to the submodule has to update the registered sha1 once the work is pushed to submodule. I am not sure to understand everything. But if you suggest to clone manually subsubmodule because it could not be clone recursively by submodule due to the lake of access write to get submodule. It is not practical in my use cases. Two of the superprojects I have in charge contains hundreds of submodules or subsubmodules and I have too much users with disparate computer skills. Getting all what a user has access on should be just a recursive clone. And I need superproject to add also submodule/subsubmodule. No. Never let the same file/directory be tracked by two git repositories at the same time. Give Paul a branch to work on where submodule is just an empty directory, and everything will be fine. Or move subsubmodule outside of submodule (and let a symbolic link point to the new location if the path cannot be easily changed). Would that work for you? If I use symbolic links it will just as gitlink enable to use the same subsubmodule clone by more than one superproject but with two major problems : - symbolic links do not work under Windows and some of my users do not even know something else could exist. - symbolic links will not store the SHA-1 of the subsubmodule. And a 'git status' in the repository containing the symbolic link will say nothing about subsubmodule state. Here you are also missing something. What Jens was suggesting was that you move your subsubmodule directly underneath the superproject and from the old location you create a link to the new location for a quick transition. But you can also change all paths in your project to point to the new location. But in the new location you will have subsubmodule registered as a submodule only that it is now directly linked (as submodule) from the superproject instead of the submodule. Ok but in this case what happen to someone cloning only submodule but not superproject ? He will not get subsubmodule which is part of it. Just a dead symbolic link with no hint on what is missing behind. Each of my submodules (at any level) should be usable superprojects by them self having a gitlink to each subsubmodules they needs. I think where we diverge is in the way we are looking gitlinks. Where you see a hierarchic tree, I see a web. And I use gitlinks just like multiplatform symbolic links storing the SHA-1 of there destination and pointing exclusively on git repositories. Well but the problem with a web is that it will introduce a lot of problems that need to be solved. Some repository has to have the authority about a file (or link). If you have a file in multiple repositories overlayed how do you know who is in charge and when? It will not introduce a lot of problems. Me and my teams are using gitlinks this way every days for 2 years know. With a web far more complex than the example I give above. And the
Re: [PATCH] rebase: new option to post edit a squashed or fixed up commit
On Tue, Mar 11, 2014 at 2:47 AM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
After squashing or fixing up, you may want to have a final look at the
commit, edit some more if needed or even do some testing. --postedit
enables that. This is (to me) a paranoid mode so either I enable it
for all squashes and fixups, or none. Hence a new option, not new todo
commands that give finer selection.
If we were to adopt Michael's (?) idea of allowing flags to each
insn in the insn sheet, would this restriction be easily lifted?
That is, instead of saying squash, you say squash --stop or
something.
I think I still need something similar, otherwise I would need to
s/squash/squash --stop/ after rebase -i --autosquash. --postedit
code could be simplified by generating squash --stop though.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a1adae8..42061fc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -571,6 +571,11 @@ do_next () {
;;
esac
record_in_rewritten $sha1
+ if test -n $postedit
+ then
+ warn Stopped at $sha1... $rest
+ exit_with_patch $sha1 0
+ fi
;;
I would have expected that any new code would stop only at the last
squash (or fixup) in a series of squashes, but this appears to stop
even at an intermediate squashed result, which will not appear in
the final history. Am I misreading the patch (or misunderstanding
the intent of the patch)?
Never thought of that case. Yes it should only stop at the last squash/fixup.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATHv2] branch.c:install_branch_config():Simplify code generating verbose message.
Thanks for taking review comments from your previous attempt into
account. This is a more well-crafted submission. Additional comments
below.
On Tue, Mar 11, 2014 at 4:40 AM, Paweł Wawruch pa...@aleg.pl wrote:
Subject: [PATHv2] branch.c:install_branch_config():Simplify code generating
verbose message.
PATCH is misspelled as PATH. Adding the v2 is indeed good etiquette.
It's typical to have a space between PATCH and vN.
Insert a space before Simplify.
SubmittingPatches suggests omitting the final period in the subject.
Also downcase simplify.
Simplify the long if chain in install_branch_config().
A bit misleading. You're not actually simplifying the 'if' chain, but
rather replacing it.
There is a long chain of if statements. The code can be more clear.
Replace the chain with table of strings. New approach is more
compact.
The first sentence merely repeats what was said earlier. It can be
dropped. The second sentence is self-evident since you already talk
about simplification and also can be dropped, as can the fourth
sentence. The third sentence is a good explanation of how the code is
being simplified and should be kept. You could, therefore, collapse
the commit message to something along these lines:
Subject: install_branch_config: simplify verbose diagnostic logic
Replace the 'if' chain with a table of strings.
Signed-off-by: Paweł Wawruch pa...@aleg.pl
---
It is considerate to reviewers to provide a link to the previous
version of the patch, like this [1], and to explain what changed in
this version relative to the last. This area just below the --- line
after your sign off is where you would write such commentary.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/243502
branch.c | 40 ++--
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..8d3b219 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,18 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *messages[] = {
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ };
On this project, arrays are usually (though not consistently) named in
singular form (for instance message[]) so that a reference to a single
item, such as message[42], reads more grammatically correct.
+ const char *name = remote_is_branch ? remote : shortname;
+ int message_number;
if (remote_is_branch
!strcmp(local, shortname)
@@ -77,29 +89,13 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch
%s from %s by rebasing.) :
- _(Branch %s set up to track remote branch
%s from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch
%s by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s
by rebasing.) :
- _(Branch %s set up to track remote ref
%s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s
by rebasing.) :
- _(Branch %s set up to track local ref
%s.),
- local, remote);
+ message_number = (!!rebasing) + 2 * (!!origin) + 4 *
(!!remote_is_branch);
Unnecessary parentheses make the code more cumbersome to read and
should be dropped.
+ assert(message_number ARRAY_SIZE(messages));
+
+ if
[RFC memory leak?] Minor memory leak fix
Strbuf needs to be released even if it's locally declared.
Signed-off-by: Fredrik Gustafsson iv...@iveqy.com
---
archive.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/archive.c b/archive.c
index 346f3b2..d6d56e6 100644
--- a/archive.c
+++ b/archive.c
@@ -113,6 +113,7 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
struct git_attr_check check[2];
const char *path_without_prefix;
int err;
+ int to_ret = 0;
args-convert = 0;
strbuf_reset(path);
@@ -126,8 +127,10 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
setup_archive_check(check);
if (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {
- if (ATTR_TRUE(check[0].value))
- return 0;
+ if (ATTR_TRUE(check[0].value)) {
+ to_ret = 0;
+ goto cleanup;
+ }
args-convert = ATTR_TRUE(check[1].value);
}
@@ -135,14 +138,20 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
err = write_entry(args, sha1, path.buf, path.len, mode);
- if (err)
- return err;
- return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ if (err) {
+ to_ret = err;
+ goto cleanup;
+ }
+ to_ret = (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ goto cleanup;
}
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
- return write_entry(args, sha1, path.buf, path.len, mode);
+ to_ret = write_entry(args, sha1, path.buf, path.len, mode);
+cleanup:
+ strbuf_release(path);
+ return to_ret;
}
int write_archive_entries(struct archiver_args *args,
--
1.8.3.1.490.g39d9b24.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][GSoC] parse-options: Add OPT_SET_INT_NONEG.
Reference: http://git.github.io/SoC-2014-Microprojects.html
Signed-off-by: Yuxuan Shui yshu...@gmail.com
---
builtin/fetch.c| 5 ++---
builtin/merge.c| 5 ++---
builtin/notes.c| 10 --
builtin/pack-objects.c | 15 ++-
builtin/update-index.c | 20
parse-options.h| 4
6 files changed, 26 insertions(+), 33 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..37c2a9d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -97,9 +97,8 @@ static struct option builtin_fetch_options[] = {
OPT_BOOL(0, progress, progress, N_(force progress reporting)),
OPT_STRING(0, depth, depth, N_(depth),
N_(deepen history of shallow clone)),
- { OPTION_SET_INT, 0, unshallow, unshallow, NULL,
- N_(convert to a complete repository),
- PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },
+ OPT_SET_INT_NONEG(0, unshallow, unshallow,
+ N_(convert to a complete repository), 1),
{ OPTION_STRING, 0, submodule-prefix, submodule_prefix, N_(dir),
N_(prepend this to submodule path output),
PARSE_OPT_HIDDEN },
{ OPTION_STRING, 0, recurse-submodules-default,
diff --git a/builtin/merge.c b/builtin/merge.c
index f0cf120..cf9a157 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -203,9 +203,8 @@ static struct option builtin_merge_options[] = {
OPT_BOOL('e', edit, option_edit,
N_(edit message before committing)),
OPT_SET_INT(0, ff, fast_forward, N_(allow fast-forward (default)),
FF_ALLOW),
- { OPTION_SET_INT, 0, ff-only, fast_forward, NULL,
- N_(abort if fast-forward is not possible),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
+ OPT_SET_INT_NONEG(0, ff-only, fast_forward,
+ N_(abort if fast-forward is not possible), FF_ONLY),
OPT_RERERE_AUTOUPDATE(allow_rerere_auto),
OPT_BOOL(0, verify-signatures, verify_signatures,
N_(Verify that the named commit has a valid GPG signature)),
diff --git a/builtin/notes.c b/builtin/notes.c
index 2b24d05..02a554d 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -739,13 +739,11 @@ static int merge(int argc, const char **argv, const char
*prefix)
N_(resolve notes conflicts using the given strategy
(manual/ours/theirs/union/cat_sort_uniq))),
OPT_GROUP(N_(Committing unmerged notes)),
- { OPTION_SET_INT, 0, commit, do_commit, NULL,
- N_(finalize notes merge by committing unmerged notes),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+ OPT_SET_INT_NONEG(0, commit, do_commit,
+ N_(finalize notes merge by committing unmerged
notes), 1),
OPT_GROUP(N_(Aborting notes merge resolution)),
- { OPTION_SET_INT, 0, abort, do_abort, NULL,
- N_(abort notes merge),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+ OPT_SET_INT_NONEG(0, abort, do_abort,
+ N_(abort notes merge), 1),
OPT_END()
};
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..2e2b12a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2579,15 +2579,12 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_(do not create an empty pack output)),
OPT_BOOL(0, revs, use_internal_rev_list,
N_(read revision arguments from standard input)),
- { OPTION_SET_INT, 0, unpacked, rev_list_unpacked, NULL,
- N_(limit the objects to those that are not yet packed),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
- { OPTION_SET_INT, 0, all, rev_list_all, NULL,
- N_(include objects reachable from any reference),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
- { OPTION_SET_INT, 0, reflog, rev_list_reflog, NULL,
- N_(include objects referred by reflog entries),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+ OPT_SET_INT_NONEG(0, unpacked, rev_list_unpacked,
+ N_(limit the objects to those that are not yet packed), 1),
+ OPT_SET_INT_NONEG(0, all, rev_list_all,
+ N_(include objects reachable from any reference), 1),
+ OPT_SET_INT_NONEG(0, reflog, rev_list_reflog,
+ N_(include objects referred by reflog entries), 1),
OPT_BOOL(0, stdout, pack_to_stdout,
N_(output pack to stdout)),
OPT_BOOL(0, include-tag, include_tag,
diff --git a/builtin/update-index.c b/builtin/update-index.c
index d12ad95..807e853 100644
---
Re: [RFC/WIP] Pluggable reference backends
Am 10.03.2014 12:00, schrieb Michael Haggerty: Reference transactions -- Very cool ideas indeed. However, I'm concerned a bit that transactions are conceptual overkill. How many concurrent updates do you expect in a repository? Wouldn't a single repo-wide lock suffice (and be _much_ simpler to implement with any backend, esp. file-based)? The API you posted in [1] doesn't look very much like a transaction API either (rather like batch-updates). E.g. there's no rollback, the queue* methods cannot report failure, and there's no way to read a ref as part of the transaction. So I'm afraid that backends that support transactions out of the box (e.g. RDBMSs) will be hard to adapt to this. Just my 2cents, Karsten [1] http://article.gmane.org/gmane.comp.version-control.git/243748 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.
Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit : Am 05.03.2014 00:01, schrieb Henri GEIST: Permit to do a 'git clone --recursive' through git-gui. I really like where this is heading! Some minor issues: - I think we should be more verbose in the commit message, including that and why the default should be on. Maybe like this? Permit to do a 'git clone --recursive' through git-gui. Add a 'recursive' checkbox in the clone menu which allows users to clone a repository and all its submodules in one go (unless the 'update' flag is set to none in the .gitmodules file for a submodule, in that case that specific submodule is not cloned automatically). Enable this new option per default, as most users want to clone all submodules too when cloning the superproject (This is currently not possible without leaving git gui or adding a custom tool entry for that). - I'd rather change the button text from Recursive (For submodules) to something like Recursively clone submodules too or such. Perfect. Would you like me to send the new version of the patch in this thread Or to make a new thread [patch v2] ? signature.asc Description: This is a digitally signed message part
Re: [PATCH][GSOC2014] changed logical chain in branch.c to lookup tables
Eric Sunshine wrote
On Mon, Mar 10, 2014 at 5:47 PM, Tamer TAS lt;
tamertas@
gt; wrote:
Section 4.3 of the GNU gettext manual [1] explains the issues in more
detail. I urge you to read it. The upshot is that translators fare
best when handed full sentences.
Note also that your change effectively reverts d53a35032a67 [2], which
did away with the sort of string composition used in your patch.
Eric thank you for your constructive feedbacks.
I read the section 4.3 of GNU gettext manual and also checked the commit you
mentioned.
It seems like that my previous changes were not internationalization
compatible.
In order for a table-driven change to be compatible, the sentences has to be
meaningful and not tokenized.
I made the following change to the branch.c in order for the function to be
both table-driven and
internationalization compatible. Let me know if there are any oversights on
my part.
Signed-off-by: TamerTas tamer...@outlook.com
---
branch.c | 39 ---
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..4c04638 100644
--- a/branch.c
+++ b/branch.c
@@ -50,10 +50,25 @@ static int should_setup_rebase(const char *origin)
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
{
const char *shortname = remote + 11;
+const char *setup_messages[] = {
+ _(Branch %s set up to track remote branch %s from %s.),
+ _(Branch %s set up to track local branch %s.),
+ _(Branch %s set up to track remote ref %s.),
+ _(Branch %s set up to track local ref %s.),
+ _(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ _(Branch %s set up to track local branch %s by rebasing.),
+ _(Branch %s set up to track remote ref %s by rebasing.),
+ _(Branch %s set up to track local ref %s by rebasing.)
+ };
+
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+int msg_index = (!!origin0) +
+ (!!remote_is_branch 1) +
+ (!!rebasing 2);
+
if (remote_is_branch
!strcmp(local, shortname)
!origin) {
@@ -77,29 +92,7 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.)
:
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
- else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+ printf_ln(setup_messages[msg_index], local, remote);
}
}
--
1.7.9.5
--
View this message in context:
http://git.661346.n2.nabble.com/PATCH-GSOC2014-changed-logical-chain-in-branch-c-to-lookup-tables-tp7605343p7605407.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] i18n: assure command not corrupted by _() process
On Mon, Mar 10, 2014 at 7:51 PM, Sandy Carter
sandy.car...@savoirfairelinux.com wrote:
Is there any update on this patch?
The patch looks good. Maybe Junio missed it.
Le 2014-03-03 09:55, Sandy Carter a écrit :
Separate message from command examples to avoid translation issues
such as a dash being omitted in a translation.
Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
builtin/branch.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index b4d7716..b304da8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -1022,11 +1022,13 @@ int cmd_branch(int argc, const char **argv, const
char *prefix)
*/
if (argc == 1 track == BRANCH_TRACK_OVERRIDE
!branch_existed remote_tracking) {
- fprintf(stderr, _(\nIf you wanted to make '%s'
track '%s', do this:\n\n), head, branch-name);
- fprintf(stderr, _(git branch -d %s\n),
branch-name);
- fprintf(stderr, _(git branch
--set-upstream-to %s\n), branch-name);
+ fprintf(stderr, \n);
+ fprintf(stderr, _(If you wanted to make '%s'
track '%s', do this:), head, branch-name);
+ fprintf(stderr, \n\n);
+ fprintf(stderr, git branch -d %s\n,
branch-name);
+ fprintf(stderr, git branch --set-upstream-to
%s\n, branch-name);
+ fprintf(stderr, \n);
}
-
} else
usage_with_options(builtin_branch_usage, options);
--
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
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC memory leak?] Minor memory leak fix
On Tue, Mar 11, 2014 at 5:45 PM, Fredrik Gustafsson iv...@iveqy.com wrote:
Strbuf needs to be released even if it's locally declared.
path is declared static. So yes it's a leak but the leak is minimum.
Your patch would make more sense if static is gone and it's leaked
after every write_archive_entry call.
Signed-off-by: Fredrik Gustafsson iv...@iveqy.com
---
archive.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/archive.c b/archive.c
index 346f3b2..d6d56e6 100644
--- a/archive.c
+++ b/archive.c
@@ -113,6 +113,7 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
struct git_attr_check check[2];
const char *path_without_prefix;
int err;
+ int to_ret = 0;
args-convert = 0;
strbuf_reset(path);
@@ -126,8 +127,10 @@ static int write_archive_entry(const unsigned char
*sha1, const char *base,
setup_archive_check(check);
if (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {
- if (ATTR_TRUE(check[0].value))
- return 0;
+ if (ATTR_TRUE(check[0].value)) {
+ to_ret = 0;
+ goto cleanup;
+ }
to_ret is already 0 so I think goto cleanup; is enough.
args-convert = ATTR_TRUE(check[1].value);
}
@@ -135,14 +138,20 @@ static int write_archive_entry(const unsigned char
*sha1, const char *base,
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
err = write_entry(args, sha1, path.buf, path.len, mode);
- if (err)
- return err;
- return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ if (err) {
+ to_ret = err;
+ goto cleanup;
+ }
+ to_ret = (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ goto cleanup;
Maybe if (err) to_ret = ...; else to_ret = ...; so we only need one
goto cleanup statement. Going even further:
to_ret = write_entry(...);
if (!to_ret) to_ret = (S_ISDIR(...));
goto cleanup;
}
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
- return write_entry(args, sha1, path.buf, path.len, mode);
+ to_ret = write_entry(args, sha1, path.buf, path.len, mode);
+cleanup:
+ strbuf_release(path);
+ return to_ret;
}
int write_archive_entries(struct archiver_args *args,
--
1.8.3.1.490.g39d9b24.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[[RFC memory leak, v.2]] Minor memory leak fix
Strbuf needs to be released even if it's locally declared.
Signed-off-by: Fredrik Gustafsson iv...@iveqy.com
---
archive.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/archive.c b/archive.c
index 346f3b2..dfc557d 100644
--- a/archive.c
+++ b/archive.c
@@ -113,6 +113,7 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
struct git_attr_check check[2];
const char *path_without_prefix;
int err;
+ int to_ret = 0;
args-convert = 0;
strbuf_reset(path);
@@ -127,22 +128,25 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
setup_archive_check(check);
if (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {
if (ATTR_TRUE(check[0].value))
- return 0;
+ goto cleanup;
args-convert = ATTR_TRUE(check[1].value);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
- err = write_entry(args, sha1, path.buf, path.len, mode);
- if (err)
- return err;
- return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ to_ret = write_entry(args, sha1, path.buf, path.len, mode);
+ if (!to_ret)
+ to_ret = (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ goto cleanup;
}
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
- return write_entry(args, sha1, path.buf, path.len, mode);
+ to_ret = write_entry(args, sha1, path.buf, path.len, mode);
+cleanup:
+ strbuf_release(path);
+ return to_ret;
}
int write_archive_entries(struct archiver_args *args,
--
1.8.3.1.490.g39d9b24.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC memory leak?] Minor memory leak fix
On Tue, Mar 11, 2014 at 06:58:11PM +0700, Duy Nguyen wrote:
On Tue, Mar 11, 2014 at 5:45 PM, Fredrik Gustafsson iv...@iveqy.com wrote:
Strbuf needs to be released even if it's locally declared.
path is declared static. So yes it's a leak but the leak is minimum.
Your patch would make more sense if static is gone and it's leaked
after every write_archive_entry call.
That's one of the reasons of the RFC. I know Junio thinks that minor
things shouldn't be fixed by themselfes because it takes up review
bandwidth, so it's better to fix them once you touch that part of the
code anyway. (At least that's how I've understood him).
This leak is at about 4.1 kB so it's not huge.
+ if (ATTR_TRUE(check[0].value)) {
+ to_ret = 0;
+ goto cleanup;
+ }
to_ret is already 0 so I think goto cleanup; is enough.
Agree, fixed in next iteration.
err = write_entry(args, sha1, path.buf, path.len, mode);
- if (err)
- return err;
- return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ if (err) {
+ to_ret = err;
+ goto cleanup;
+ }
+ to_ret = (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ goto cleanup;
Maybe if (err) to_ret = ...; else to_ret = ...; so we only need one
goto cleanup statement. Going even further:
to_ret = write_entry(...);
if (!to_ret) to_ret = (S_ISDIR(...));
goto cleanup;
Agree, fixed in next iteration.
--
Med vänlig hälsning
Fredrik Gustafsson
tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/26] t1400: Pass a legitimate newvalue to update command
On 03/10/2014 05:38 PM, Michael Haggerty wrote: It seems to me that -z input will nearly always be machine-generated, so there is not much reason to accept the empty string as shorthand for zeros. So I think that my version of the rules, being simpler to explain, is a slight improvement. I agree. But your version is already out in the wild, so backwards-compatibility is also a consideration, even though it is rather a fine point in a rather unlikely usage (why use update rather than delete to delete a reference?). I'm not using empty==zero with -z in any deployment. Since the feature is quite new, the behavior change is not silent, and it is easy to construct input that works with both versions, I do not think we need to worry about compatibility. or rewrite the documentation to describe my rules. I prefer this approach. Thanks, -Brad -- 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] upload-pack: send shallow info over stdin to pack-objects
Before cdab485 (upload-pack: delegate rev walking in shallow fetch to
pack-objects - 2013-08-16) upload-pack does not write to the source
repository. cdab485 starts to write $GIT_DIR/shallow_XX if it's a
shallow fetch, so the source repo must be writable.
git:// servers do not need write access to repos and usually don't
have it, which means cdab485 breaks shallow clone over git://
Instead of using a temporary file as the media for shallow points, we
can send them over stdin to pack-objects as well. Prepend shallow
SHA-1 with --shallow so pack-objects knows what is what.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation update and a minor tweak.
Documentation/git-pack-objects.txt | 2 ++
builtin/pack-objects.c | 10 ++
t/t5537-fetch-shallow.sh | 13 +
upload-pack.c | 21 -
4 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-pack-objects.txt
b/Documentation/git-pack-objects.txt
index cdab9ed..d2d8f47 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -64,6 +64,8 @@ base-name::
the same way as 'git rev-list' with the `--objects` flag
uses its `commit` arguments to build the list of objects it
outputs. The objects on the resulting list are packed.
+ Besides revisions, `--not` or `--shallow SHA-1` lines are
+ also accepted.
--unpacked::
This implies `--revs`. When processing the list of
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..358f9a3 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2455,6 +2455,9 @@ static void get_object_list(int ac, const char **av)
save_commit_buffer = 0;
setup_revisions(ac, av, revs, NULL);
+ /* make sure shallows are read */
+ is_repository_shallow();
+
while (fgets(line, sizeof(line), stdin) != NULL) {
int len = strlen(line);
if (len line[len - 1] == '\n')
@@ -2467,6 +2470,13 @@ static void get_object_list(int ac, const char **av)
write_bitmap_index = 0;
continue;
}
+ if (starts_with(line, --shallow )) {
+ unsigned char sha1[20];
+ if (get_sha1_hex(line + 10, sha1))
+ die(not an SHA-1 '%s', line + 10);
+ register_shallow(sha1);
+ continue;
+ }
die(not a rev '%s', line);
}
if (handle_revision_arg(line, revs, flags,
REVARG_CANNOT_BE_FILENAME))
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 3ae9092..a980574 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -173,4 +173,17 @@ EOF
)
'
+test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
+ cp -R .git read-only.git
+ find read-only.git -print | xargs chmod -w
+ test_when_finished find read-only.git -type d -print | xargs chmod +w
+ git clone --no-local --depth=2 read-only.git from-read-only
+ git --git-dir=from-read-only/.git log --format=%s actual
+ cat expect EOF
+add-1-back
+4
+EOF
+ test_cmp expect actual
+'
+
test_done
diff --git a/upload-pack.c b/upload-pack.c
index 0c44f6b..a5c50e4 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -70,6 +70,14 @@ static ssize_t send_client_data(int fd, const char *data,
ssize_t sz)
return sz;
}
+static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
+{
+ FILE *fp = cb_data;
+ if (graft-nr_parent == -1)
+ fprintf(fp, --shallow %s\n, sha1_to_hex(graft-sha1));
+ return 0;
+}
+
static void create_pack_file(void)
{
struct child_process pack_objects;
@@ -81,12 +89,10 @@ static void create_pack_file(void)
const char *argv[12];
int i, arg = 0;
FILE *pipe_fd;
- char *shallow_file = NULL;
if (shallow_nr) {
- shallow_file = setup_temporary_shallow(NULL);
argv[arg++] = --shallow-file;
- argv[arg++] = shallow_file;
+ argv[arg++] = ;
}
argv[arg++] = pack-objects;
argv[arg++] = --revs;
@@ -114,6 +120,9 @@ static void create_pack_file(void)
pipe_fd = xfdopen(pack_objects.in, w);
+ if (shallow_nr)
+ for_each_commit_graft(write_one_shallow, pipe_fd);
+
for (i = 0; i want_obj.nr; i++)
fprintf(pipe_fd, %s\n,
sha1_to_hex(want_obj.objects[i].item-sha1));
@@ -242,12 +251,6 @@ static void create_pack_file(void)
error(git upload-pack: git-pack-objects died with error.);
goto fail;
}
-
[GSOC] Git Configuration API improvements
Hello to all, I'm Karthik Nayak, a Computer Science student from Bangalore India. I will be applying for GSOC 2014. This is my first time applying for GSOC. I have been using Git for a long time now, so it would be an ideal organisation for me to contribute to. As suggested I completed the Micro Project under the guidance of Eric Sunshine.[1] I went through the Idea's page[2] and the mails corresponding to the topic[3]. Currently we have multiple invocation of git_config() in an individual invocation of git() which is not efficient. Also, it is hard to implement new features, as mentioned -- such as allowing a configuration to be unset. The basic idea is to use a data structure to store the config, the first time the git_config() is called. And refer to this each and every time we invoke git_config() further. Jeff suggested [4] to use a tree or a mapping of keys to values which are stored in a string. I think that the idea of storing using the config as a tree data structure would be really advantageous. This would allow us to easily implement modifications later, and can help to easily take care of duplicates being created on multiple usage of config set and unset. As, whenever a node's children have been deleted the node can also be automatically deleted. The tree can be a struct with values, header link and link to other configs. This way we can also create functions to work on the tree easily. Would be great to hear your thoughts on this topic and also I plan to start creating a proposal. Nice to have any suggestions or feedback of any kind. Thank you, Karthik [1] : http://article.gmane.org/gmane.comp.version-control.git/243695/match=karthik+188+gmail+com [2] : http://git.github.io/SoC-2014-Ideas.html [3] : http://article.gmane.org/gmane.comp.version-control.git/243500/match=git+configuration+caching [4] : http://article.gmane.org/gmane.comp.version-control.git/243542 -- 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] install_branch_config: simplify verbose diagnostic logic
Replace the chain of if statements with table of strings.
Signed-off-by: Paweł Wawruch pa...@aleg.pl
---
I changed the commit message. Logic of table has changed. To make it more
clear I added three dimensions of the table.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/243502
[2]: http://thread.gmane.org/gmane.comp.version-control.git/243849
branch.c | 42 --
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..741551a 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,21 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *message[][2][2] = {{{
+ N_(Branch %s set up to track remote branch %s from %s
by rebasing.),
+ N_(Branch %s set up to track remote branch %s from
%s.),
+ },{
+ N_(Branch %s set up to track local branch %s by
rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ }},{{
+ N_(Branch %s set up to track remote ref %s by
rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ },{
+ N_(Branch %s set up to track local ref %s by
rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ }}};
+ const char *name = remote_is_branch ? remote : shortname;
+ int message_number;
if (remote_is_branch
!strcmp(local, shortname)
@@ -77,29 +92,12 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.) :
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
+ if (origin remote_is_branch)
+
printf_ln(_(messages[!remote_is_branch][!origin][!rebasing]),
+ local, name, origin);
else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+
printf_ln(_(messages[!remote_is_branch][!origin][!rebasing]),
+ local, name);
}
}
--
1.8.3.2
--
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: question about: Facebook makes Mercurial faster than Git
On Mon, Mar 10, 2014 at 10:56:51AM -0700, David Lang wrote: On Mon, 10 Mar 2014, Ondřej Bílka wrote: On Mon, Mar 10, 2014 at 03:13:45AM -0700, David Lang wrote: On Mon, 10 Mar 2014, Dennis Luehring wrote: according to these blog posts http://www.infoq.com/news/2014/01/facebook-scaling-hg https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/ mercurial can be faster then git but i don't found any reply from the git community if it is a real problem or if there a ongoing (maybe git 2.0) changes to compete better in this case As I understand this, the biggest part of what happened is that Facebook made a tweak to mercurial so that when it needs to know what files have changed in their massive tree, their version asks their special storage array, while git would have to look at it through the filesystem interface (by doing stat calls on the directories and files to see if anything has changed) That is mostly a kernel problem. Long ago there was proposed patch to add a recursive mtime so you could check what subtrees changed. If somebody ressurected that patch it would gave similar boost. btrfs could actually implement this efficiently, but for a lot of other filesysems this could be very expensive. The question is if it could be enough of a win to make it a good choice for people who are doing a heavy git workload as opposed to more generic uses. Read next paragraph how do that efficiently, a directory update needs to be done only between application runs. Also there is no overhead when not used (except if that makes headers bigger.) there's also the issue of managed vs generated files, if you update the mtime all the way up the tree because a source file was compiled and a binary created, that will quickly defeat the value of the recursive mtime. You could do marking on per-file basis. I am not sure if that is needed as larger projects use makefiles to not recompile everything so its probably recompiled because source at same directory changed. Also if your compile time is five minutes a half second status would not make much difference. There are two issues that need to be handled, first if you are concerned about one mtime change doing lot of updates a application needs to mark all directories it is interested on, when we do update we unmark directory and by that we update each directory at most once per application run. Second problem were hard links where probably a best course is keep list of these and stat them separately. -- 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: [GSOC] Git Configuration API improvements
karthik nayak karthik@gmail.com writes: Currently we have multiple invocation of git_config() in an individual invocation of git() which is not efficient. Also, it is hard to implement new features, I think efficiency is not a real concern here. Config files are small and easy to parse, so parsing them multiple times isn't really noticeable from the performance point of view. OTOH, the extensibility is a real concern, and that should be the main motivation for the project. -- 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: [GSOC] Git Configuration API improvements
On Tue, Mar 11, 2014 at 8:21 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: karthik nayak karthik@gmail.com writes: Currently we have multiple invocation of git_config() in an individual invocation of git() which is not efficient. Also, it is hard to implement new features, I think efficiency is not a real concern here. Config files are small and easy to parse, so parsing them multiple times isn't really noticeable from the performance point of view. OTOH, the extensibility is a real concern, and that should be the main motivation for the project. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ Hello Matthieu, Thanks. I understand what you mean. extensibility is the main motivation of the project, i think that by implementing the cache system we can fix the small problems (reappearing of headers while setting and unsetting configs) and also implement new features like to unset a config easily. -- 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: [GSoC14][RFC] Proposal Draft: Refactor tempfile handling
On 03/01/2014 10:04 PM, Brian Gesiak wrote: Hello all, My name is Brian Gesiak. I'm a research student at the University of Tokyo, and I'm hoping to participate in this year's Google Summer of Code by contributing to Git. I'm a longtime user, first-time contributor--some of you may have noticed my microproject patches.[1][2] I'd like to gather some information on one of the GSoC ideas posted on the ideas page. Namely, I'm interested in refactoring the way tempfiles are cleaned up. The ideas page points out that while lock files are closed and unlinked[3] when the program exits[4], object pack files implement their own brand of temp file creation and deletion. This implementation doesn't share the same guarantees as lock files--it is possible that the program terminates before the temp file is unlinked.[5] Lock file references are stored in a linked list. When the program exits, this list is traversed and each file is closed and unlinked. It seems to me that this mechanism is appropriate for temp files in general, not just lock files. Thus, my proposal would be to extract this logic into a separate module--tempfile.h, perhaps. Lock and object files would share the tempfile implementation. That is, both object and lock temp files would be stored in a linked list, and all of these would be deleted at program exit. I'm very enthused about this project--I think it has it all: - Tangible benefits for the end-user - Reduced complexity in the codebase - Ambitious enough to be interesting - Small enough to realistically be completed in a summer Please let me know if this seems like it would make for an interesting proposal, or if perhaps there is something I am overlooking. Any feedback at all would be appreciated. Thank you! Hi Brian, Thanks for your proposal. I have a technical point that I think your proposal should address: Currently the linked list of lockfiles only grows, never shrinks. Once an object has been linked into the list, there is no way to remove it again even after the lock has been released. So if a lock needs to be created dynamically at a random place in the code, its memory is unavoidably leaked. This hasn't been much of a problem in the past because (1) the number of locks acquired/released during a Git invocation is reasonable, and (2) a lock object (even if it is already in the list) can be reused after the lock has been released. So there are many lock callsites that define one static lock instance and use it over and over again. But I have a feeling that if we want to use a similar mechanism to handle all temporary files (of which there can be more), then it would be a good idea to lift this limitation. It will require some care, though, to make sure that record removal is done in a way that is threadsafe and safe in the event of all expected kinds of process death. 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
[PATCH] status merge: guarentee space between msg and path
Add space between how and one when printing status of unmerged data. This fixes an appending of the how message when it is longer than 20. This is the case in some translations such as the french one where the colon gets appended to the file: supprimé par nous :wt-status.c modifié des deux côtés :wt-status.h Additionally, having a space makes the file in question easier to select in console to quickly address the problem. Without the space, the colon (and, sometimes the last word) of the message is selected along with the file. The previous french example should now print as the following, which is more proper: supprimé par nous : wt-status.c modifié des deux côtés : wt-status.h Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com --- wt-status.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index a452407..69e0dfc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -264,7 +264,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s, case 6: how = _(both added:); break; case 7: how = _(both modified:); break; } - status_printf_more(s, c, %-20s%s\n, how, one); + status_printf_more(s, c, %-19s %s\n, how, one); strbuf_release(onebuf); } -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.
Am 11.03.2014 12:07, schrieb Henri GEIST: Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit : Am 05.03.2014 00:01, schrieb Henri GEIST: Permit to do a 'git clone --recursive' through git-gui. I really like where this is heading! Some minor issues: - I think we should be more verbose in the commit message, including that and why the default should be on. Maybe like this? Permit to do a 'git clone --recursive' through git-gui. Add a 'recursive' checkbox in the clone menu which allows users to clone a repository and all its submodules in one go (unless the 'update' flag is set to none in the .gitmodules file for a submodule, in that case that specific submodule is not cloned automatically). Enable this new option per default, as most users want to clone all submodules too when cloning the superproject (This is currently not possible without leaving git gui or adding a custom tool entry for that). - I'd rather change the button text from Recursive (For submodules) to something like Recursively clone submodules too or such. Perfect. Would you like me to send the new version of the patch in this thread Or to make a new thread [patch v2] ? It doesn't matter that much as long as you start the subject with [PATCH v2]. But I believe you should send it to Pat and create the diff from inside the git-gui directory so he can simply apply it to his repository. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] run-command: mark run_hook_with_custom_index as deprecated
On Tue, Mar 11, 2014 at 2:00 AM, brian m. carlson sand...@crustytoothpaste.net wrote: On Mon, Mar 10, 2014 at 07:49:37PM +0100, Benoit Pierre wrote: --- run-command.h | 1 + 1 file changed, 1 insertion(+) diff --git a/run-command.h b/run-command.h index 88460f9..3653bfa 100644 --- a/run-command.h +++ b/run-command.h @@ -51,6 +51,7 @@ extern int run_hook_le(const char *const *env, const char *name, ...); extern int run_hook_ve(const char *const *env, const char *name, va_list args); LAST_ARG_MUST_BE_NULL +__attribute__((deprecated)) It doesn't appear that we use the deprecated attribute anywhere else in the code. Wouldn't it just be better to change the places that use this and then remove the function altogether? I imagine your current patch might introduce a number of warnings that some people would rather avoid. This last patch is optional. There are no callers to run_hook_with_custom_index, so no warnings for now. See discussion on first draft as to why I added it: http://article.gmane.org/gmane.comp.version-control.git/243561 -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with commit -p -m
On Mon, Mar 10, 2014 at 9:07 PM, Jeff King p...@peff.net wrote: On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote: Don't change git environment: move the GIT_EDITOR=: override to the hook command subprocess, like it's already done for GIT_INDEX_FILE. [...] This is a lot of change, and in some ways I think it is making things better overall. However, the simplest fix for this is basically to move the setting of GIT_EDITOR down to after we prepare the index. Jun Hao (cc'd) has been preparing a series for this based on the Bloomberg git hackday a few weeks ago, but it hasn't been sent to the list yet. Commits are here: https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing if you care to look. I'm not sure which solution is technically superior, but it's worth considering both. I regret not encouraging Jun to post to the list sooner, as we might have avoided some duplicated effort. But that's a sunk cost, and we should pick up whichever is the best for the project. Replying here instead of to Jun Hao message (I'm not subscribed to the mailing list, so I did not received it): Jun Hao wrote: I like the idea that the environment setting should be done in one place. Just not sure run_hook is the right place tho. If user doesn't have any hook setup, will this kick in? One more question, will this work for dry run? Or dry run doesn't matter in this case? According to the original commit, the change to GIT_EDITOR is only here for hooks: commit 406400ce4f69e79b544dd3539a71b85d99331820 Author: Paolo Bonzini bonz...@gnu.org Date: Tue Feb 5 11:01:45 2008 +0100 git-commit: set GIT_EDITOR=: if editor will not be launched This is a preparatory patch that provides a simple way for the future prepare-commit-msg hook to discover if the editor will be launched. Signed-off-by: Junio C Hamano gits...@pobox.com So there is really no reason to set it earlier, and not just in the hook subprocess environment. Regarding dry run: the bug is present, and my patch fix it too (but is missing a test for this). As to which patch is better: it's really not for me to decide! It's a question for the maintainer(s), Jun Hao patch is sure much smaller and simpler, mine is more involved and I believe cleaner in the long term: there is no risk of another part of the commit process to be impacted by the change of environment. Also note that my patch changes the merge builtin too: GIT_EDITOR will not be overriden if an editor will be launched (when used with --edit). -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with commit -p -m
On Tue, Mar 11, 2014 at 06:56:02PM +0100, Benoit Pierre wrote: According to the original commit, the change to GIT_EDITOR is only here for hooks: commit 406400ce4f69e79b544dd3539a71b85d99331820 Author: Paolo Bonzini bonz...@gnu.org Date: Tue Feb 5 11:01:45 2008 +0100 git-commit: set GIT_EDITOR=: if editor will not be launched This is a preparatory patch that provides a simple way for the future prepare-commit-msg hook to discover if the editor will be launched. Signed-off-by: Junio C Hamano gits...@pobox.com So there is really no reason to set it earlier, and not just in the hook subprocess environment. Ah, you're right. I was thinking that our invocation of launch_editor also respected it. It does, but we never get there at all because use_editor is set to 0. So yeah, it really is just needed for the hook. Your patch, even though it is a bigger change, keeps the setting to the minimal area, which is cleaner and more maintainable in the long run. -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: [GSoC14][RFC] Proposal Draft: Refactor tempfile handling
On Tue, Mar 11, 2014 at 05:27:05PM +0100, Michael Haggerty wrote: Thanks for your proposal. I have a technical point that I think your proposal should address: Currently the linked list of lockfiles only grows, never shrinks. Once an object has been linked into the list, there is no way to remove it again even after the lock has been released. So if a lock needs to be created dynamically at a random place in the code, its memory is unavoidably leaked. Thanks, I remember thinking about this when I originally conceived of the idea, but I forgot to mention it in the idea writeup. In most cases the potential leaks are finite and small, but object creation and diff tempfiles could both be unbounded. So this is definitely something to consider. In both cases we have a bounded number of _simultaneous_ tempfiles, so one strategy could be to continue using static objects. But it should not be hard to do it dynamically, and I suspect the resulting API will be a lot easier to comprehend. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Eric Sunshine sunsh...@sunshineco.com writes: On Mon, Mar 10, 2014 at 2:49 PM, Benoit Pierre benoit.pie...@gmail.com wrote: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh Is it possible to give this file a name less unusual and more consistent with other test scripts? Perhaps choose a more generic name which may allow other similar tests to be added to the file in the future (if needed)? Surely. There are reset-patch and checkout-patch tests, and if we were to add something like this, I'd imagine commit-patch would be a logical name for the new test. diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh new file mode 100755 index 000..994939a --- /dev/null +++ b/t/t7513-commit_-p_-m_hunk_edit.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 + echo line3 file + cat expect -\EOF + diff --git a/file b/file + index a29bdeb..c0d0fb4 100644 + --- a/file + +++ b/file + @@ -1 +1,2 @@ +line1 + +line2 + EOF In the previous review, the suggestion was that creation of 'expect' should be moved to the test below where it is actually used rather than into the 'setup' phase above. +' + +test_expect_failure 'edit hunk commit -p -m message' ' + echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 file + git diff HEAD^ HEAD actual + test_cmp expect actual +' + +test_done -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-gui: Add a 'recursive' checkbox in the clone menu.
Permit to do a 'git clone --recursive' through git-gui.
Add a 'recursive' checkbox in the clone menu which allows
users to clone a repository and all its submodules in one
go (unless the 'update' flag is set to none in the
.gitmodules file for a submodule, in that case that
specific submodule is not cloned automatically).
Enable this new option per default, as most users want to
clone all submodules too when cloning the superproject
(This is currently not possible without leaving git gui
or adding a custom tool entry for that).
Signed-off-by: Henri GEIST geist.he...@laposte.net
Thanks-to: Jens Lehmann jens.lehm...@web.de
---
lib/choose_repository.tcl | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 3c10bc6..1209fa6 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -18,6 +18,7 @@ field local_path {} ; # Where this repository is locally
field origin_url {} ; # Where we are cloning from
field origin_name origin ; # What we shall call 'origin'
field clone_type hardlink ; # Type of clone to construct
+field recursive true ; # Recursive cloning flag
field readtree_err; # Error output from read-tree (if any)
field sorted_recent ; # recent repositories (sorted)
@@ -525,6 +526,11 @@ method _do_clone {} {
foreach r $w_types {
pack $r -anchor w
}
+ ${NS}::checkbutton $args.type_f.recursive \
+ -text [mc Recursively clone submodules too] \
+ -variable @recursive \
+ -onvalue true -offvalue false
+ pack $args.type_f.recursive
grid $args.type_l $args.type_f -sticky new
grid columnconfigure $args 1 -weight 1
@@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
fileevent $fd readable [cb _readtree_wait $fd]
}
+method _do_validate_submodule_cloning {ok} {
+ if {$ok} {
+ $o_cons done $ok
+ set done 1
+ } else {
+ _clone_failed $this [mc Cannot clone submodules.]
+ }
+}
+
+method _do_clone_submodules {} {
+ if {$recursive eq {true}} {
+ destroy $w_body
+ set o_cons [console::embed \
+ $w_body \
+ [mc Cloning submodules]]
+ pack $w_body -fill both -expand 1 -padx 10
+ $o_cons exec \
+ [list git submodule update --init --recursive] \
+ [cb _do_validate_submodule_cloning]
+ } else {
+ set done 1
+ }
+}
+
method _readtree_wait {fd} {
set buf [read $fd]
$o_cons update_meter $buf
@@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
} else {
- set done 1
+ _do_clone_submodules $this
}
}
@@ -996,7 +1026,7 @@ method _postcheckout_wait {fd_ph} {
hook_failed_popup post-checkout $pch_error 0
}
unset pch_error
- set done 1
+ _do_clone_submodules $this
return
}
fconfigure $fd_ph -blocking 0
--
1.7.9.3.369.gd715.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with
Jeff King peff at peff.net writes: Ah, you're right. I was thinking that our invocation of launch_editor also respected it. It does, but we never get there at all because use_editor is set to 0. So yeah, it really is just needed for the hook. Your patch, even though it is a bigger change, keeps the setting to the minimal area, which is cleaner and more maintainable in the long run. -Peff Oh, didn't realize GIT_EDITOR change is only for hooks. Good catch. I agree Benoit's patch is better for the long term. Thanks - Jun -- 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] configure.ac: link with -liconv for locale_charset()
On e.g. FreeBSD 10.x, the following situation is common: - there's iconv implementation in libc, which has no locale_charset() function - there's GNU libiconv installed from Ports Collection Git build process - detects that iconv is in libc and thus -liconv is not needed for it - detects locale_charset in -liconv, but for some reason doesn't add it to CHARSET_LIB (as it would do with -lcharset if locale_charset() was found there instead of -liconv) - git doesn't build due to unresolved external locale_charset() Fix this by adding -liconv to CHARSET_LIB if locale_charset() is detected in this library. Signed-off-by: Dmitry Marakasov amd...@amdmi3.ru --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git configure.ac configure.ac index 2f43393..3f5c644 100644 --- configure.ac +++ configure.ac @@ -890,7 +890,7 @@ GIT_CONF_SUBST([HAVE_STRINGS_H]) # and libcharset does CHARSET_LIB= AC_CHECK_LIB([iconv], [locale_charset], - [], + [CHARSET_LIB=-liconv], [AC_CHECK_LIB([charset], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
Junio C Hamano gits...@pobox.com writes: Documentation on the whole argument parsing is quite short, so, I though, adding an example just to show how usage is generated would look like I am trying to make this feature look important than it is :) You already are by saying the Angle brackets are automatic, aren't you? That is, among the things --parseopt mode does, the above stresses what happens _only_ when it emits help text for items that use this feature. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] fix status_printf_ln calls zero-length format warnings
Jeff King p...@peff.net writes: Most of us who compile with -Wall decided a while ago to use -Wno-format-zero-length, because it really is a silly complaint (it assumes there are no side effects of the function besides printing the format string, which is obviously not true in this case). It would be nice to compile out of the box with -Wall -Werror, and I think your solution looks relatively clean. But I am slightly concerned about the assumption that it is OK to pass a NULL fmt parameter to a function marked with __attribute__((format)). It certainly seems to be the case now, and I do not know of any plans for that to change, but it seems like a potentially obvious thing for the compiler to check. Yes, exactly. -- 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: GSoC idea: allow git rebase --interactive todo lines to take options
Jeff King p...@peff.net writes: I'd say keep it at this point. I think there _are_ some good ideas here, and part of a project is figuring out what is good. And part of the role of the mentor is applying some taste. Amen to that. I hope we have enough mentor-candidates with good taste, though ;-) There are probably students who would be a good fit, and students who would not. That is true for just about every project, of course, but I think this one is just a little trickier than some. Perhaps. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
Jeff King p...@peff.net writes: I think the main flag of interest is giving an fnmatch pattern to limit the advertised refs. There could potentially be others, but I do not know of any offhand. One thing that comes to mind is where symrefs point at, which we failed to add the last time around because we ran out of the hidden-space behind NUL. -- 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 memory leak, v.2]] Minor memory leak fix
Am 11.03.2014 13:36, schrieb Fredrik Gustafsson:
Strbuf needs to be released even if it's locally declared.
Signed-off-by: Fredrik Gustafsson iv...@iveqy.com
---
archive.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/archive.c b/archive.c
index 346f3b2..dfc557d 100644
--- a/archive.c
+++ b/archive.c
@@ -113,6 +113,7 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
struct git_attr_check check[2];
const char *path_without_prefix;
int err;
+ int to_ret = 0;
args-convert = 0;
strbuf_reset(path);
@@ -127,22 +128,25 @@ static int write_archive_entry(const unsigned char *sha1,
const char *base,
setup_archive_check(check);
if (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {
if (ATTR_TRUE(check[0].value))
- return 0;
+ goto cleanup;
args-convert = ATTR_TRUE(check[1].value);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
- err = write_entry(args, sha1, path.buf, path.len, mode);
- if (err)
- return err;
- return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ to_ret = write_entry(args, sha1, path.buf, path.len, mode);
+ if (!to_ret)
+ to_ret = (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
+ goto cleanup;
Why add to_ret when you can use the existing variable err directly? Or
at least remove it when it's not used anymore.
}
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
- return write_entry(args, sha1, path.buf, path.len, mode);
+ to_ret = write_entry(args, sha1, path.buf, path.len, mode);
+cleanup:
+ strbuf_release(path);
+ return to_ret;
If you free the memory of the strbuf at the end of the function then
there is no point in keeping it static anymore. Growing it to PATH_MAX
also doesn't make as much sense as before then.
The patch makes git archive allocate and free the path strbuf once per
archive entry, while before it allocated only once per run and left
freeing to the OS. What's the performance impact of this change?
}
int write_archive_entries(struct archiver_args *args,
--
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 memory leak?] Minor memory leak fix
Fredrik Gustafsson iv...@iveqy.com writes: On Tue, Mar 11, 2014 at 06:58:11PM +0700, Duy Nguyen wrote: On Tue, Mar 11, 2014 at 5:45 PM, Fredrik Gustafsson iv...@iveqy.com wrote: Strbuf needs to be released even if it's locally declared. path is declared static. So yes it's a leak but the leak is minimum. Your patch would make more sense if static is gone and it's leaked after every write_archive_entry call. That's one of the reasons of the RFC. I know Junio thinks that minor things shouldn't be fixed by themselfes because it takes up review bandwidth, so it's better to fix them once you touch that part of the code anyway. (At least that's how I've understood him). Yes, but I at the same time think this static struct strbuf is a clear statement by the original author that this is not a leak per-se. The trade-off, if I am reading the code right, is between keeping a piece of memory that is large enough to hold the longest pathname until exit() vs saving repeated allocations and frees for each of the thousands of paths in the resulting archive. I tend to think the original strikes a better balance between the two. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] install_branch_config: simplify verbose diagnostic logic
Paweł Wawruch pa...@aleg.pl writes:
Replace the chain of if statements with table of strings.
Signed-off-by: Paweł Wawruch pa...@aleg.pl
---
I changed the commit message. Logic of table has changed. To make it more
clear I added three dimensions of the table.
I am not sure if the message is diagnostic; it looks more like
reminder text to me.
diff --git a/branch.c b/branch.c
index 723a36b..741551a 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,21 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *message[][2][2] = {{{
+ N_(Branch %s set up to track remote branch %s from %s
by rebasing.),
+ N_(Branch %s set up to track remote branch %s from
%s.),
+ },{
+ N_(Branch %s set up to track local branch %s by
rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ }},{{
+ N_(Branch %s set up to track remote ref %s by
rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ },{
+ N_(Branch %s set up to track local ref %s by
rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ }}};
I almost agree with the above use of a strange brace opening/closing
convention in order to reduce the indentation levels [*1*]
but then perhaps the above can be dedented even further?
+ const char *message[][2][2] = {{{
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ }, {
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ }}, {{
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ }, {
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ }}};
+ const char *name = remote_is_branch ? remote : shortname;
+ int message_number;
Do you still need this variable after making it a multi-dimentional
array?
[Footnote]
*1* i.e. otherwise we would need something like
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] status merge: guarentee space between msg and path
Sandy Carter sandy.car...@savoirfairelinux.com writes: diff --git a/wt-status.c b/wt-status.c index a452407..69e0dfc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -264,7 +264,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s, case 6: how = _(both added:); break; case 7: how = _(both modified:); break; } - status_printf_more(s, c, %-20s%s\n, how, one); + status_printf_more(s, c, %-19s %s\n, how, one); strbuf_release(onebuf); } Thanks; I have to wonder if we would want to do something similar to what 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05) to the other parts of the output, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Tue, Mar 11, 2014 at 12:32:37PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: I think the main flag of interest is giving an fnmatch pattern to limit the advertised refs. There could potentially be others, but I do not know of any offhand. One thing that comes to mind is where symrefs point at, which we failed to add the last time around because we ran out of the hidden-space behind NUL. Yeah, good idea. I might be misremembering some complications, but we can probably do it with: 1. Teach the client to send an advertise-symrefs flag before the ref advertisement. 2. Teach the server to include symrefs in the ref advertisement; we can invent a new syntax because we know the client has asked for it. That does not have to come immediately, though. Done correctly, upload-pack2 is not about implementing the fnmatch feature, but allowing arbitrary capability strings from the client before the ref advertisement starts. So this just becomes an extension that we can add and advertise during that new phase. -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 03/26] t1400: Pass a legitimate newvalue to update command
Michael Haggerty mhag...@alum.mit.edu writes: It seems to me that -z input will nearly always be machine-generated, so there is not much reason to accept the empty string as shorthand for zeros. So I think that my version of the rules, being simpler to explain, is a slight improvement. But your version is already out in the wild, so backwards-compatibility is also a consideration, even though it is rather a fine point in a rather unlikely usage (why use update rather than delete to delete a reference?). I don't know. I'm willing to rewrite the code to go back to your rules, or rewrite the documentation to describe my rules. Neutral bystanders *cough*Junio*cough*, what do you prefer? I may be misremembering things, but your first sentence quoted above was exactly my reaction while reviewing the original change, and I might have even raised that as an issue myself, saying something like consistency across values is more important than type-saving in a machine format. Since nobody else were raising the issue back then, however, we are stuck with the interface. I am not against deprecating and removing the support for it in the longer term, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH] submodule : Add --no-separate-git-dir option to add and update command.
On Tue, Mar 11, 2014 at 10:55:03AM +0100, Henri GEIST wrote: Le lundi 10 mars 2014 à 21:32 +0100, Heiko Voigt a écrit : On Mon, Mar 10, 2014 at 10:08:06AM +0100, Henri GEIST wrote: Le samedi 08 mars 2014 à 00:00 +0100, Jens Lehmann a écrit : Am 06.03.2014 23:20, schrieb Henri GEIST: What is the use case you are trying to solve and why can that not be handled by adding subsubmodule inside submodule? The problem is access rights. Imagine you have 2 people Pierre and Paul. Each with different access write on the server. Pierre has full access on every things. Paul has full access on superproject and subsubmodule but no read/write access to submodule only execution on the directory. Ok, I think I'm slowly beginning to understand your setup. I want all user to get every things they are allowed to have with the command 'git submodule update --init --recursive'. Then as Paul can not clone submodule he can not get subsubmodule recursively through it. Sure, that's how it should work. Paul could only work on a branch where submodule is an empty directory containing subsubmodule, as he doesn't have the rights to clone submodule. I will not redundantly create a branch for each user on the server. When users clone the server it already create a special branch for them 'master' which track 'origin/master'. And if each user have its own branch on the server it will completely defeat the goal of the server collaboration. And transform the git server in simple rsync server. I do not think that is what Jens was suggesting. It does not matter in which branch they work, they can directly use master if you like. What he was suggesting is that they create their repository structure like this: git clone g...@somewhere.net:superproject.git cd superproject/submodule git clone g...@somehwere.net:subsubmodule.git cd subsubmodule ... work, commit, work, commit ... The same applies for the superproject. Now only someone with access to the submodule has to update the registered sha1 once the work is pushed to submodule. I am not sure to understand everything. But if you suggest to clone manually subsubmodule because it could not be clone recursively by submodule due to the lake of access write to get submodule. It is not practical in my use cases. Two of the superprojects I have in charge contains hundreds of submodules or subsubmodules and I have too much users with disparate computer skills. Getting all what a user has access on should be just a recursive clone. Then I would think about getting rid of the recursion part as it seems you have interdependencies which can only be solved by a package management system. I would see the superproject as this package management system, but it requires you to have all the submodules next to each other instead of contained in each other. I think in terms of combining libraries that is actually the correct solution because there can be modules that need each other. Some submodule A might evolve and add a dependency to a subsubmodule B that is itself contained in another submodule C. Then it just does not feel correct anymore that B is contained in C. You want to have one instance that is in charge of all the dependencies, that is IMO directly the superproject and not something that reaches through another submodule to record a dependency to a subsubmodule. And I need superproject to add also submodule/subsubmodule. No. Never let the same file/directory be tracked by two git repositories at the same time. Give Paul a branch to work on where submodule is just an empty directory, and everything will be fine. Or move subsubmodule outside of submodule (and let a symbolic link point to the new location if the path cannot be easily changed). Would that work for you? If I use symbolic links it will just as gitlink enable to use the same subsubmodule clone by more than one superproject but with two major problems : - symbolic links do not work under Windows and some of my users do not even know something else could exist. - symbolic links will not store the SHA-1 of the subsubmodule. And a 'git status' in the repository containing the symbolic link will say nothing about subsubmodule state. Here you are also missing something. What Jens was suggesting was that you move your subsubmodule directly underneath the superproject and from the old location you create a link to the new location for a quick transition. But you can also change all paths in your project to point to the new location. But in the new location you will have subsubmodule registered as a submodule only that it is now directly linked (as submodule) from the superproject instead of the submodule. Ok but in this case what happen to someone cloning only
Re: [PATCH] status merge: guarentee space between msg and path
Le 2014-03-11 15:59, Junio C Hamano a écrit : Sandy Carter sandy.car...@savoirfairelinux.com writes: diff --git a/wt-status.c b/wt-status.c index a452407..69e0dfc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -264,7 +264,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s, case 6: how = _(both added:); break; case 7: how = _(both modified:); break; } - status_printf_more(s, c, %-20s%s\n, how, one); + status_printf_more(s, c, %-19s %s\n, how, one); strbuf_release(onebuf); } Thanks; I have to wonder if we would want to do something similar to what 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05) to the other parts of the output, though. I could, do this. It would be cleaner, but there's just the issue of the colon (:) which requires a space before it in the french language[1]. As you can see in po/fr.po, the french translators have done a good job at including it [2]. 3651e45c takes the colon out of the control of the translators. + if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED) + status_printf_more(s, c, %s:%.*s%s - %s, + what, len, padding, one, two); + else + status_printf_more(s, c, %s:%.*s%s, + what, len, padding, one); [1] https://en.wikipedia.org/wiki/Colon_%28punctuation%29#Spacing [2] https://github.com/git/git/blob/master/po/fr.po#L585 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
Jeff King p...@peff.net writes: On Tue, Mar 11, 2014 at 12:32:37PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: I think the main flag of interest is giving an fnmatch pattern to limit the advertised refs. There could potentially be others, but I do not know of any offhand. One thing that comes to mind is where symrefs point at, which we failed to add the last time around because we ran out of the hidden-space behind NUL. Yeah, good idea. I might be misremembering some complications, but we can probably do it with: 1. Teach the client to send an advertise-symrefs flag before the ref advertisement. 2. Teach the server to include symrefs in the ref advertisement; we can invent a new syntax because we know the client has asked for it. I was thinking more about the underlying protocol, not advertisement in particular, and I think we came to the same conclusion. The capability advertisement deserves to have its own separate packet message type, when both sides say that they understand it, so that we do not have to be limited by the pkt-line length limit. We could do one message per capability, and at the same time can lift the traditional capability hidden after the NUL is purged every time, so we need to repeat them if we want to later change it, because that is how older clients and servers use that information insanity, for example. -- 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] status merge: guarentee space between msg and path
Sandy Carter sandy.car...@savoirfairelinux.com writes: Le 2014-03-11 15:59, Junio C Hamano a écrit : Sandy Carter sandy.car...@savoirfairelinux.com writes: diff --git a/wt-status.c b/wt-status.c index a452407..69e0dfc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -264,7 +264,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s, case 6: how = _(both added:); break; case 7: how = _(both modified:); break; } - status_printf_more(s, c, %-20s%s\n, how, one); + status_printf_more(s, c, %-19s %s\n, how, one); strbuf_release(onebuf); } Thanks; I have to wonder if we would want to do something similar to what 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05) to the other parts of the output, though. I could, do this. It would be cleaner, but there's just the issue of the colon (:) which requires a space before it in the french language[1]. As you can see in po/fr.po, the french translators have done a good job at including it [2]. 3651e45c takes the colon out of the control of the translators. That is a separate bug we would need to address, then. Duy Cc'ed. + if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED) + status_printf_more(s, c, %s:%.*s%s - %s, + what, len, padding, one, two); + else + status_printf_more(s, c, %s:%.*s%s, + what, len, padding, one); [1] https://en.wikipedia.org/wiki/Colon_%28punctuation%29#Spacing [2] https://github.com/git/git/blob/master/po/fr.po#L585 -- 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][GSOC2014] changed logical chain in branch.c to lookup tables
Thanks for the resubmission. Comments below.
On Tue, Mar 11, 2014 at 7:33 AM, Tamer TAS tamer...@outlook.com wrote:
Subject: changed logical chain in branch.c to lookup tables
Use imperative tone: change rather than changed
Prefix the message with the part of the project you are touching. So,
for instance, you might write:
Subject: install_branch_config: change logical chain to lookup table
Eric Sunshine wrote
On Mon, Mar 10, 2014 at 5:47 PM, Tamer TAS lt;
tamertas@
gt; wrote:
Section 4.3 of the GNU gettext manual [1] explains the issues in more
detail. I urge you to read it. The upshot is that translators fare
best when handed full sentences.
Note also that your change effectively reverts d53a35032a67 [2], which
did away with the sort of string composition used in your patch.
Eric thank you for your constructive feedbacks.
I read the section 4.3 of GNU gettext manual and also checked the commit you
mentioned.
It seems like that my previous changes were not internationalization
compatible.
In order for a table-driven change to be compatible, the sentences has to be
meaningful and not tokenized.
I made the following change to the branch.c in order for the function to be
both table-driven and
internationalization compatible. Let me know if there are any oversights on
my part.
This commentary is relevant to the ongoing email thread but not
suitable for the permanent commit message. Place it below the ---
line just under your sign-off.
Signed-off-by: TamerTas tamer...@outlook.com
---
It's considerate to reviewers to provide a link to the previous
attempt, like this [1]. This area below the --- line is the
appropriate place to do so. Likewise, explain how this version differs
from the previous one.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/243793
branch.c | 39 ---
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..4c04638 100644
--- a/branch.c
+++ b/branch.c
@@ -50,10 +50,25 @@ static int should_setup_rebase(const char *origin)
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
Your patch is whitespace damaged, probably due to being pasted into
your email. git send-email avoids such problems.
Indentation is also broken. Use tabs for indentation, and set tab
width to 8 in your editor, as explained in
Documentation/CodingGuidelines.
{
const char *shortname = remote + 11;
+const char *setup_messages[] = {
+ _(Branch %s set up to track remote branch %s from %s.),
+ _(Branch %s set up to track local branch %s.),
+ _(Branch %s set up to track remote ref %s.),
+ _(Branch %s set up to track local ref %s.),
+ _(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ _(Branch %s set up to track local branch %s by rebasing.),
+ _(Branch %s set up to track remote ref %s by rebasing.),
+ _(Branch %s set up to track local ref %s by rebasing.)
+ };
On this project, arrays are usually (though not consistently) named in
singular form (for instance setup_message[]) so that a reference to a
single item, such as setup_message[42], reads more grammatically
correct.
It's not correct to use _() inside the initializer list. Instead use
N_(). Read section 4.7 of the GNU gettext manual [2] for an
explanation. You will still need to use _() at the point where you
extract the message from the array.
[2]: http://www.gnu.org/software/gettext/manual/gettext.html#Special-cases
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+int msg_index = (!!origin0) +
+ (!!remote_is_branch 1) +
+ (!!rebasing 2);
Are you sure this is correct? As I read it, msg_index will only ever
have a value of 0 or 1, which is unlikely what you intended.
if (remote_is_branch
!strcmp(local, shortname)
!origin) {
@@ -77,29 +92,7 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch
%s from %s by rebasing.)
:
- _(Branch %s set up to track remote branch
%s from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch
%s by rebasing.) :
-
Re: [PATCH] configure.ac: link with -liconv for locale_charset()
Dmitry Marakasov amd...@amdmi3.ru writes: On e.g. FreeBSD 10.x, the following situation is common: - there's iconv implementation in libc, which has no locale_charset() function - there's GNU libiconv installed from Ports Collection Git build process - detects that iconv is in libc and thus -liconv is not needed for it - detects locale_charset in -liconv, but for some reason doesn't add it to CHARSET_LIB (as it would do with -lcharset if locale_charset() was found there instead of -liconv) - git doesn't build due to unresolved external locale_charset() Fix this by adding -liconv to CHARSET_LIB if locale_charset() is detected in this library. Signed-off-by: Dmitry Marakasov amd...@amdmi3.ru --- Looks sensible; Dilyan, any comments? configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git configure.ac configure.ac index 2f43393..3f5c644 100644 --- configure.ac +++ configure.ac @@ -890,7 +890,7 @@ GIT_CONF_SUBST([HAVE_STRINGS_H]) # and libcharset does CHARSET_LIB= AC_CHECK_LIB([iconv], [locale_charset], - [], + [CHARSET_LIB=-liconv], [AC_CHECK_LIB([charset], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Tue, Mar 11, 2014 at 01:25:23PM -0700, Junio C Hamano wrote: Yeah, good idea. I might be misremembering some complications, but we can probably do it with: 1. Teach the client to send an advertise-symrefs flag before the ref advertisement. 2. Teach the server to include symrefs in the ref advertisement; we can invent a new syntax because we know the client has asked for it. I was thinking more about the underlying protocol, not advertisement in particular, and I think we came to the same conclusion. The capability advertisement deserves to have its own separate packet message type, when both sides say that they understand it, so that we do not have to be limited by the pkt-line length limit. We could do one message per capability, and at the same time can lift the traditional capability hidden after the NUL is purged every time, so we need to repeat them if we want to later change it, because that is how older clients and servers use that information insanity, for example. So this may be entering the more radical changes realm I mentioned earlier. If the client is limited to setting a few flags, then something like http can get away with: GET foo.git/info/refs?service=git-upload-packadvertise-symrefsrefspec=refs/heads/* And it does not need to worry about upload-pack2 at all. Either the server recognizes and acts on them, or it ignores them. But given that we do not have such a magic out-of-band method for passing values over ssh and git, maybe it is not worth worrying about. Http can move to upload-pack2 along with the rest. One thing that _is_ worth considering for http is how the protocol starts. We do not want to introduce an extra http round-trip to the protocol if we can help it. If the initial GET becomes a POST, then it could pass along the pkt-line of client capabilities with the initial request, and the server would respond with the ref advertisement as usual. -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 4/7] commit: fix patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837..da423b2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -53,10 +53,10 @@ struct checkout_opts {
static int post_checkout_hook(struct commit *old, struct commit *new,
int changed)
{
- return run_hook(NULL, post-checkout,
- sha1_to_hex(old ? old-object.sha1 : null_sha1),
- sha1_to_hex(new ? new-object.sha1 : null_sha1),
- changed ? 1 : 0, NULL);
+return run_hook_le(NULL, post-checkout,
+sha1_to_hex(old ? old-object.sha1 : null_sha1),
+sha1_to_hex(new ? new-object.sha1 : null_sha1),
+changed ? 1 : 0, NULL);
Funny indentation.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh I'll move this to t/t7514-commit-patch.sh for now while queuing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t7810: add missing variables to tests in loop
Some tests in t7810-grep.sh are in a loop that runs them against HEAD and
the work tree. In order for that to work the test code should use the
variables $L (display name), $H (HEAD or empty string) and $HC (revision
prefix for result lines); otherwise tests are just repeated with the same
target. Add the variables where they're missing and make sure the test
description is wrapped in double quotes (instead of single quotes) to
allow variables to be expanded.
Signed-off-by: Rene Scharfe l@web.de
---
t/t7810-grep.sh | 58 -
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f698001..46aaebc 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -105,7 +105,7 @@ do
test_expect_success grep -w $L (w) '
: expected
- test_must_fail git grep -n -w -e ^w actual
+ test_must_fail git grep -n -w -e ^w $H actual
test_cmp expected actual
'
@@ -240,92 +240,92 @@ do
test_cmp expected actual
'
test_expect_success grep $L with grep.extendedRegexp=false '
- echo ab:a+bc expected
- git -c grep.extendedRegexp=false grep a+b*c ab actual
+ echo ${HC}ab:a+bc expected
+ git -c grep.extendedRegexp=false grep a+b*c $H ab actual
test_cmp expected actual
'
test_expect_success grep $L with grep.extendedRegexp=true '
- echo ab:abc expected
- git -c grep.extendedRegexp=true grep a+b*c ab actual
+ echo ${HC}ab:abc expected
+ git -c grep.extendedRegexp=true grep a+b*c $H ab actual
test_cmp expected actual
'
test_expect_success grep $L with grep.patterntype=basic '
- echo ab:a+bc expected
- git -c grep.patterntype=basic grep a+b*c ab actual
+ echo ${HC}ab:a+bc expected
+ git -c grep.patterntype=basic grep a+b*c $H ab actual
test_cmp expected actual
'
test_expect_success grep $L with grep.patterntype=extended '
- echo ab:abc expected
- git -c grep.patterntype=extended grep a+b*c ab actual
+ echo ${HC}ab:abc expected
+ git -c grep.patterntype=extended grep a+b*c $H ab actual
test_cmp expected actual
'
test_expect_success grep $L with grep.patterntype=fixed '
- echo ab:a+b*c expected
- git -c grep.patterntype=fixed grep a+b*c ab actual
+ echo ${HC}ab:a+b*c expected
+ git -c grep.patterntype=fixed grep a+b*c $H ab actual
test_cmp expected actual
'
test_expect_success LIBPCRE grep $L with grep.patterntype=perl '
- echo ab:a+b*c expected
- git -c grep.patterntype=perl grep a\x{2b}b\x{2a}c ab actual
+ echo ${HC}ab:a+b*c expected
+ git -c grep.patterntype=perl grep a\x{2b}b\x{2a}c $H ab
actual
test_cmp expected actual
'
test_expect_success grep $L with grep.patternType=default and
grep.extendedRegexp=true '
- echo ab:abc expected
+ echo ${HC}ab:abc expected
git \
-c grep.patternType=default \
-c grep.extendedRegexp=true \
- grep a+b*c ab actual
+ grep a+b*c $H ab actual
test_cmp expected actual
'
test_expect_success grep $L with grep.extendedRegexp=true and
grep.patternType=default '
- echo ab:abc expected
+ echo ${HC}ab:abc expected
git \
-c grep.extendedRegexp=true \
-c grep.patternType=default \
- grep a+b*c ab actual
+ grep a+b*c $H ab actual
test_cmp expected actual
'
- test_expect_success 'grep $L with grep.patternType=extended and
grep.extendedRegexp=false' '
- echo ab:abc expected
+ test_expect_success grep $L with grep.patternType=extended and
grep.extendedRegexp=false '
+ echo ${HC}ab:abc expected
git \
-c grep.patternType=extended \
-c grep.extendedRegexp=false \
- grep a+b*c ab actual
+ grep a+b*c $H ab actual
test_cmp expected actual
'
- test_expect_success 'grep $L with grep.patternType=basic and
grep.extendedRegexp=true' '
- echo ab:a+bc expected
+ test_expect_success grep $L with grep.patternType=basic and
grep.extendedRegexp=true '
+ echo ${HC}ab:a+bc expected
git \
[PATCH 2/2] grep: support -h (no header) with --count
Suppress printing the header (filename) with -h even if in -c/--count
mode. GNU grep and OpenBSD's grep do the same.
Signed-off-by: Rene Scharfe l@web.de
---
grep.c | 7 +--
t/t7810-grep.sh | 12
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index c668034..94f7290 100644
--- a/grep.c
+++ b/grep.c
@@ -1562,8 +1562,11 @@ static int grep_source_1(struct grep_opt *opt, struct
grep_source *gs, int colle
*/
if (opt-count count) {
char buf[32];
- output_color(opt, gs-name, strlen(gs-name),
opt-color_filename);
- output_sep(opt, ':');
+ if (opt-pathname) {
+ output_color(opt, gs-name, strlen(gs-name),
+opt-color_filename);
+ output_sep(opt, ':');
+ }
snprintf(buf, sizeof(buf), %u\n, count);
opt-output(opt, buf, strlen(buf));
return 1;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 46aaebc..63b3039 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -328,6 +328,18 @@ do
grep a+b*c $H ab actual
test_cmp expected actual
'
+
+ test_expect_success grep --count $L '
+ echo ${HC}ab:3 expected
+ git grep --count -e b $H -- ab actual
+ test_cmp expected actual
+ '
+
+ test_expect_success grep --count -h $L '
+ echo 3 expected
+ git grep --count -h -e b $H -- ab actual
+ test_cmp expected actual
+ '
done
cat expected EOF
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git $Id$ smudge filter
Currently, I've got a perl script that modifies the Id line in a smudge filter: [filter ident-line] smudge = /usr/local/bin/githook_ident-filter.pl %f The problem I've noticed with smudge filters is that it leaves the repo dirty. How do I fix this? I am basically trying to replicate the behavior of CVS or SVN $Id$ line here. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/26] t1400: Pass a legitimate newvalue to update command
On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano gits...@pobox.com wrote: I may be misremembering things, but your first sentence quoted above was exactly my reaction while reviewing the original change, and I might have even raised that as an issue myself, saying something like consistency across values is more important than type-saving in a machine format. For reference, the original design discussion of the format was here: http://thread.gmane.org/gmane.comp.version-control.git/233842 I do not recall this issue being raised before, but now that it has been raised I fully agree: http://thread.gmane.org/gmane.comp.version-control.git/243754/focus=243862 In -z mode an empty newvalue should be treated as missing just as it is for oldvalue. This is obvious now in hindsight and I wish I had realized this at the time. Back then I went through a lot of iterations on the format and missed this simplification in the final version :( Moving forward: The create command rejects a zero newvalue so the change in question for that command is merely the wording of the error message and there is no compatibility issue. The update command supports a zero newvalue so that it can be used for all operations (create, update, delete, verify) with the proper combination of old and new values. The change in question makes an empty newvalue an error where it was previously treated as zero. (BTW, Michael, I do not see a test case for the new error in your series. Something like the patch below should work.) I am not against deprecating and removing the support for it in the longer term, though. As I reported in my above-linked response, I'm not depending on the old behavior myself. Also if one were to start seeing this error then generated input needs only trivial changes to avoid it. If we do want to preserve compatibility for others then perhaps an empty newvalue with -z should produce: warning: update $ref: missing newvalue, treating as zero Then after a few releases it can be switched to an error. Thanks, -Brad diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 3cc5c66..1e9fe7c 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -730,6 +730,12 @@ test_expect_success 'stdin -z fails update with bad ref name' ' grep fatal: invalid ref format: ~a err ' +test_expect_success 'stdin -z fails update with empty new value' ' + printf $F update $a stdin + test_must_fail git update-ref -z --stdin stdin 2err + grep fatal: update $a: missing newvalue err +' + test_expect_success 'stdin -z fails update with no new value' ' printf $F update $a stdin test_must_fail git update-ref -z --stdin stdin 2err -- 1.8.5.2 -- 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] mv: prevent mismatched data when ignoring errors.
brian m. carlson sand...@crustytoothpaste.net writes: We shrink the source and destination arrays, but not the modes or submodule_gitfile arrays, resulting in potentially mismatched data. Shrink all the arrays at the same time to prevent this. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- builtin/mv.c | 5 + 1 file changed, 5 insertions(+) diff --git a/builtin/mv.c b/builtin/mv.c index f99c91e..b20cd95 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -230,6 +230,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix) memmove(destination + i, destination + i + 1, (argc - i) * sizeof(char *)); + memmove(modes + i, modes + i + 1, + (argc - i) * sizeof(char *)); + memmove(submodule_gitfile + i, + submodule_gitfile + i + 1, + (argc - i) * sizeof(char *)); i--; } } else Thanks. Neither this nor John's seems to describe the user-visible way to trigger the symptom. Can we have tests for them? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git $Id$ smudge filter
shawn wilson ag4ve...@gmail.com writes: Currently, I've got a perl script that modifies the Id line in a smudge filter: [filter ident-line] smudge = /usr/local/bin/githook_ident-filter.pl %f The problem I've noticed with smudge filters is that it leaves the repo dirty. How do I fix this? I am basically trying to replicate the behavior of CVS or SVN $Id$ line here. It somewhat smells fishy to have only smudge filter defined without a corresponding clean filter, doesn't it? -- 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] implement submodule config cache for lookup of submodule names
On Mon, Mar 10, 2014 at 10:24:12PM +0100, Heiko Voigt wrote:
I have also moved all functions into the new submodule-config-cache
module. I am not completely satisfied with the naming since it is quite
long. If someone comes up with some different naming I am open for it.
Maybe simply submodule-config (submodule_config prefix for functions)?
Since the cache is totally internal to the submodule-config code, I do
not know that you even need to have a nice submodule-config-cache API.
Can it just be a singleton?
That is bad design in a sense (it becomes harder later if you ever want
to pull submodule config from two sources simultaneously), but it
matches many other subsystems in git which cache behind the scenes.
I also suspect you could call submodule_config simply submodule, and
let it be a stand-in for the submodule (for now, only data from the
config, but potentially you could keep other data on it, too).
So with all that, the entry point into your code is just:
const struct submodule *submodule_from_path(const char *path);
and the caching magically happens behind-the-scenes.
But take all of this with a giant grain of salt, as I am not too
familiar with the needs of the callers.
+/* one submodule_config_cache entry */
+struct submodule_config {
+ struct strbuf path;
+ struct strbuf name;
+ unsigned char gitmodule_sha1[20];
+};
Do these strings need changed after they are written once? If not, you
may want to just make these bare pointers (you can still use strbufs to
create them, and then strbuf_detach at the end). That may just be a matter of
taste, though.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH] submodule : Add --no-separate-git-dir option to add and update command.
Le mardi 11 mars 2014 à 21:11 +0100, Heiko Voigt a écrit : On Tue, Mar 11, 2014 at 10:55:03AM +0100, Henri GEIST wrote: Le lundi 10 mars 2014 à 21:32 +0100, Heiko Voigt a écrit : On Mon, Mar 10, 2014 at 10:08:06AM +0100, Henri GEIST wrote: Le samedi 08 mars 2014 à 00:00 +0100, Jens Lehmann a écrit : Am 06.03.2014 23:20, schrieb Henri GEIST: What is the use case you are trying to solve and why can that not be handled by adding subsubmodule inside submodule? The problem is access rights. Imagine you have 2 people Pierre and Paul. Each with different access write on the server. Pierre has full access on every things. Paul has full access on superproject and subsubmodule but no read/write access to submodule only execution on the directory. Ok, I think I'm slowly beginning to understand your setup. I want all user to get every things they are allowed to have with the command 'git submodule update --init --recursive'. Then as Paul can not clone submodule he can not get subsubmodule recursively through it. Sure, that's how it should work. Paul could only work on a branch where submodule is an empty directory containing subsubmodule, as he doesn't have the rights to clone submodule. I will not redundantly create a branch for each user on the server. When users clone the server it already create a special branch for them 'master' which track 'origin/master'. And if each user have its own branch on the server it will completely defeat the goal of the server collaboration. And transform the git server in simple rsync server. I do not think that is what Jens was suggesting. It does not matter in which branch they work, they can directly use master if you like. What he was suggesting is that they create their repository structure like this: git clone g...@somewhere.net:superproject.git cd superproject/submodule git clone g...@somehwere.net:subsubmodule.git cd subsubmodule ... work, commit, work, commit ... The same applies for the superproject. Now only someone with access to the submodule has to update the registered sha1 once the work is pushed to submodule. I am not sure to understand everything. But if you suggest to clone manually subsubmodule because it could not be clone recursively by submodule due to the lake of access write to get submodule. It is not practical in my use cases. Two of the superprojects I have in charge contains hundreds of submodules or subsubmodules and I have too much users with disparate computer skills. Getting all what a user has access on should be just a recursive clone. Then I would think about getting rid of the recursion part as it seems you have interdependencies which can only be solved by a package management system. I would see the superproject as this package management system, but it requires you to have all the submodules next to each other instead of contained in each other. You put the finger on a key point. I use the submodule system exactly as a package management system. It is even the only use I have of it. I am not able to imagine another use. (My imagination is limited). I really use 'git clone --recursive' as 'apt-get install'. And I am pretty sure you also. And in fact for the case where the submodules/packages should be side by side, I have a third patch witch enable just this by enabling '../' to be part of a gitlink. Much of my submodules/packages make use of this feature but I also have the case where the dependency make them contained in each others. I think in terms of combining libraries that is actually the correct solution because there can be modules that need each other. Some submodule A might evolve and add a dependency to a subsubmodule B that is itself contained in another submodule C. Then it just does not feel correct anymore that B is contained in C. You want to have one instance that is in charge of all the dependencies, that is IMO directly the superproject and not something that reaches through another submodule to record a dependency to a subsubmodule. Right. But each module need to know by its own gitlinks which are its dependency to be able to track version compatibility and not rely on an hypothetic superproject which may or may not do it as a submodule do not even know if it is part of a superproject. And could be include in totally different superprojects. And I need superproject to add also submodule/subsubmodule. No. Never let the same file/directory be tracked by two git repositories at the same time. Give Paul a branch to work on where submodule is just an empty directory, and everything will be fine. Or move subsubmodule outside of submodule (and let a symbolic link point to the new location if the path cannot
What's cooking in git.git (Mar 2014, #02; Tue, 11)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Topics that have been cooking in 'next' for 2.0 have been merged to 'master', which means we are committed to make the next one a big release. Kind of scary, isn't it? You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * cc/starts-n-ends-with-endgame (2013-12-05) 1 commit (merged to 'next' on 2014-02-25 at 473e143) + strbuf: remove prefixcmp() and suffixcmp() Originally merged to 'next' on 2014-01-07 Endgame for the cc/starts-n-ends-with topic; this needs to be evil-merged with other topics that introduce new uses of prefix/suffix-cmp functions. * gj/push-more-verbose-advice (2013-11-13) 1 commit (merged to 'next' on 2014-02-25 at 1cd10b0) + push: switch default from matching to simple Originally merged to 'next' on 2013-11-21 Explain 'simple' and 'matching' in git push advice message; the topmost patch is a rebase of jc/push-2.0-default-to-simple on top of it. * jc/add-2.0-ignore-removal (2013-04-22) 1 commit (merged to 'next' on 2014-02-25 at a0d018a) + git add pathspec... defaults to -A Originally merged to 'next' on 2013-12-06 Updated endgame for git add pathspec that defaults to --all aka --no-ignore-removal. * jc/core-checkstat-2.0 (2013-05-06) 1 commit (merged to 'next' on 2014-02-25 at 62f6aeb) + core.statinfo: remove as promised in Git 2.0 Originally merged to 'next' on 2013-12-06 * jc/hold-diff-remove-q-synonym-for-no-deletion (2013-07-19) 1 commit (merged to 'next' on 2014-02-25 at ccfff88) + diff: remove diff-files -q in a version of Git in a distant future Originally merged to 'next' on 2013-12-06 Remove deprecated -q option git diff-files. * jc/push-2.0-default-to-simple (2013-06-18) 1 commit (merged to 'next' on 2014-02-25 at 1f0e178) + push: switch default from matching to simple Originally merged to 'next' on 2013-12-06 * jk/run-network-tests-by-default (2014-02-14) 1 commit (merged to 'next' on 2014-02-25 at 62a8ad0) + tests: turn on network daemon tests by default Originally merged to 'next' on 2014-02-20 Teach make test to run networking tests when possible by default. * jn/add-2.0-u-A-sans-pathspec (2013-04-26) 1 commit (merged to 'next' on 2014-02-25 at 9e5c0d2) + git add: -u/-A now affects the entire working tree Originally merged to 'next' on 2013-12-06 * ks/combine-diff (2014-02-24) 6 commits (merged to 'next' on 2014-02-25 at 69e5a87) + tests: add checking that combine-diff emits only correct paths + combine-diff: simplify intersect_paths() further + combine-diff: combine_diff_path.len is not needed anymore + combine-diff: optimize combine_diff_path sets intersection + diff test: add tests for combine-diff with orderfile + diffcore-order: export generic ordering interface (this branch is used by ks/tree-diff-nway.) Originally merged to 'next' on 2014-02-20 Teach combine-diff to honour the path-output-order imposed by diffcore-order, and optimize how matching paths are found in the N-way diffs made with parents. * nd/daemonize-gc (2014-02-10) 2 commits (merged to 'next' on 2014-02-25 at f592335) + gc: config option for running --auto in background + daemon: move daemonize() to libgit.a Originally merged to 'next' on 2014-02-20 Allow running gc --auto in the background. -- [New Topics] * jk/detect-push-typo-early (2014-03-05) 3 commits - push: detect local refspec errors early - match_explicit_lhs: allow a verify only mode - match_explicit: hoist refspec lhs checks into their own function Catch git push $there no-such-branch early. Will merge to 'next'. * jk/diff-funcname-cpp-regex (2014-03-05) 1 commit - diff: simplify cpp funcname regex Has the discussion settled on this? * jk/doc-deprecate-grafts (2014-03-05) 1 commit - docs: mark info/grafts as outdated Will merge to 'next'. * rm/strchrnul-not-strlen (2014-03-10) 1 commit - use strchrnul() in place of strchr() and strlen() Will merge to 'next'. * sh/use-hashcpy (2014-03-06) 1 commit - Use hashcpy() when copying object names Will merge to 'next'. * jc/no-need-for-env-in-sh-scripts (2014-03-06) 1 commit - *.sh: drop useless use of env Will merge to 'next'. * jc/tag-contains-with (2014-03-07) 1 commit - tag: grok --with as synonym to --contains Will merge to 'next'. * bp/commit-p-editor (2014-03-11) 8 commits - run-command: mark run_hook_with_custom_index as deprecated - merge hook tests: fix and update tests - merge: fix GIT_EDITOR override for commit hook - commit: fix patch hunk editing with commit -p -m - SQUASH??? - test patch hunk editing with commit -p -m - merge hook tests: use 'test_must_fail'
Re: [PATCH] configure.ac: link with -liconv for locale_charset()
* Junio C Hamano (gits...@pobox.com) wrote: On e.g. FreeBSD 10.x, the following situation is common: - there's iconv implementation in libc, which has no locale_charset() function - there's GNU libiconv installed from Ports Collection Git build process - detects that iconv is in libc and thus -liconv is not needed for it - detects locale_charset in -liconv, but for some reason doesn't add it to CHARSET_LIB (as it would do with -lcharset if locale_charset() was found there instead of -liconv) - git doesn't build due to unresolved external locale_charset() Fix this by adding -liconv to CHARSET_LIB if locale_charset() is detected in this library. Signed-off-by: Dmitry Marakasov amd...@amdmi3.ru --- Looks sensible; Dilyan, any comments? Addendum: build logs before and after the fix: http://people.freebsd.org/~amdmi3/git-iconv-fail.log http://people.freebsd.org/~amdmi3/git-iconv-fixed.log -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amd...@amdmi3.ru ..: jabber: amd...@jabber.ruhttp://www.amdmi3.ru -- 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] sh-i18n--envsubst: retire unused string_list_member()
This static function has no callers, nor has it had any since its
introduction in ba67aaf2d05d (git-sh-i18n--envsubst: our own envsubst(1)
for eval_gettext(), 2011-05-14). Remove it.
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
The latest Apple developer tools (just released) has -Wunused-function
enabled by default, thus it complains about this anomaly.
sh-i18n--envsubst.c | 12
1 file changed, 12 deletions(-)
diff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
index 5ddd688..855d28c 100644
--- a/sh-i18n--envsubst.c
+++ b/sh-i18n--envsubst.c
@@ -237,18 +237,6 @@ string_list_sort (string_list_ty *slp)
qsort (slp-item, slp-nitems, sizeof (slp-item[0]), cmp_string);
}
-/* Test whether a string list contains a given string. */
-static inline int
-string_list_member (const string_list_ty *slp, const char *s)
-{
- size_t j;
-
- for (j = 0; j slp-nitems; ++j)
-if (strcmp (slp-item[j], s) == 0)
- return 1;
- return 0;
-}
-
/* Test whether a sorted string list contains a given string. */
static int
sorted_string_list_member (const string_list_ty *slp, const char *s)
--
1.9.0.282.g01e9d92
--
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] configure.ac: link with -liconv for locale_charset()
Hello, this changes effectively the meaning of CHARSET_LIB to always/unconditionally contain the library with the charset_locale () function. The snippet at the end of the email updates the description in /Makefile . However, I checked now how gnulib deals with locale_charset (). Contary to my expectation, where gnulib builds only functions if they are not found in libraries on the target system, gnulib module localcharset builds unconditionally from source the function locale_charset (). I guess they do this in a portable way, but still. My preference is to agree on universal approach for finding this function, something like: OK, if found in libiconv, else OK, if found in libcharset, else OK, here are the sources, build the function from them, and don't look for it in (shared) libaries I asked at bug-gnulib@ why they build locale_charset() before checking for it in libiconv / libcharset. My posting shall appear at http://lists.gnu.org/archive/html/bug-gnulib/2014-03/index.html . Let's see what the answer will be. I don't know if in the git codebase generally is refused to use gnulib code, but if you agree on the above procedure with 3xOK, then locale_charset() will be available even on systems, that don't have this function in libcharset or libiconv Maybe /compat/poll/poll.[ch] in git-core from gnulib had similar history. Kind regards Дилян On 11.03.2014 21:35, Junio C Hamano wrote: Dmitry Marakasov amd...@amdmi3.ru writes: On e.g. FreeBSD 10.x, the following situation is common: - there's iconv implementation in libc, which has no locale_charset() function - there's GNU libiconv installed from Ports Collection Git build process - detects that iconv is in libc and thus -liconv is not needed for it - detects locale_charset in -liconv, but for some reason doesn't add it to CHARSET_LIB (as it would do with -lcharset if locale_charset() was found there instead of -liconv) - git doesn't build due to unresolved external locale_charset() Fix this by adding -liconv to CHARSET_LIB if locale_charset() is detected in this library. Signed-off-by: Dmitry Marakasov amd...@amdmi3.ru --- Looks sensible; Dilyan, any comments? configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git configure.ac configure.ac index 2f43393..3f5c644 100644 --- configure.ac +++ configure.ac @@ -890,7 +890,7 @@ GIT_CONF_SUBST([HAVE_STRINGS_H]) # and libcharset does CHARSET_LIB= AC_CHECK_LIB([iconv], [locale_charset], - [], + [CHARSET_LIB=-liconv], [AC_CHECK_LIB([charset], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) -- diff --git a/Makefile b/Makefile index dddaf4f..dce4694 100644 --- a/Makefile +++ b/Makefile @@ -59,9 +59,9 @@ all:: # FreeBSD can use either, but MinGW and some others need to use # libcharset.h's locale_charset() instead. # -# Define CHARSET_LIB to you need to link with library other than -liconv to +# Define CHARSET_LIB to the library you need to link with in order to # use locale_charset() function. On some platforms this needs to set to -# -lcharset +# -lcharset, on others to -liconv . # # Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't # need -lintl when linking. -- 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: An idea for git bisect and a GSoC enquiry
On Mon, Mar 3, 2014 at 7:34 PM, Junio C Hamano gits...@pobox.com wrote: I think you fundamentally cannot use two labels that are merely distinct and bisect correctly. You need to know which ones (i.e. good) are to be excluded and the other (i.e. bad) are to be included when computing the remaining to be tested set of commits. Good point. Yes, this isn't viable. -- 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] install_branch_config: simplify verbose messages logic
Replace the chain of if statements with table of strings.
Signed-off-by: Paweł Wawruch pa...@aleg.pl
---
The changes proposed by Junio C Hamano:
Improvement of indentations. Removed an unused variable.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/243502
[2]: http://thread.gmane.org/gmane.comp.version-control.git/243849
[3]: http://thread.gmane.org/gmane.comp.version-control.git/243865
branch.c | 41 +++--
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..2a4b911 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,20 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *message[][2][2] = {{{
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ },{
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ }},{{
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ },{
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ }}};
+ const char *name = remote_is_branch ? remote : shortname;
if (remote_is_branch
!strcmp(local, shortname)
@@ -77,29 +91,12 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.) :
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
+ if (origin remote_is_branch)
+
printf_ln(_(message[!remote_is_branch][!origin][!rebasing]),
+ local, name, origin);
else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+
printf_ln(_(message[!remote_is_branch][!origin][!rebasing]),
+ local, name);
}
}
--
1.8.3.2
--
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] status merge: guarentee space between msg and path
On Wed, Mar 12, 2014 at 3:26 AM, Junio C Hamano gits...@pobox.com wrote: Sandy Carter sandy.car...@savoirfairelinux.com writes: Le 2014-03-11 15:59, Junio C Hamano a écrit : Sandy Carter sandy.car...@savoirfairelinux.com writes: diff --git a/wt-status.c b/wt-status.c index a452407..69e0dfc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -264,7 +264,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s, case 6: how = _(both added:); break; case 7: how = _(both modified:); break; } - status_printf_more(s, c, %-20s%s\n, how, one); + status_printf_more(s, c, %-19s %s\n, how, one); strbuf_release(onebuf); } Thanks; I have to wonder if we would want to do something similar to what 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05) to the other parts of the output, though. I could, do this. It would be cleaner, but there's just the issue of the colon (:) which requires a space before it in the french language[1]. As you can see in po/fr.po, the french translators have done a good job at including it [2]. 3651e45c takes the colon out of the control of the translators. That is a separate bug we would need to address, then. Duy Cc'ed. We went through this before http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/1109239/focus=239560 If the colon needs language specific treatment then it should be part of the translatable strings. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] configure.ac: link with -liconv for locale_charset()
* Junio C Hamano (gits...@pobox.com) wrote: Looks sensible; Dilyan, any comments? Another addendum, comment from Tijl Coosemans t...@freebsd.org who just fixed this problem in FreeBSD ports (differently): --- Please let upstream know they should either use iconv from libc + nl_langinfo from libc, or iconv from libiconv + locale_charset from libiconv, but not mix the two. Actually they could just always use nl_langinfo when it's available because locale_charset is not much more than an alias for it. --- The fix used in ports was to just disable check for libcharset.h: http://www.freebsd.org/cgi/query-pr.cgi?pr=187326#reply3 -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amd...@amdmi3.ru ..: jabber: amd...@jabber.ruhttp://www.amdmi3.ru -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] status merge: guarentee space between msg and path
Add space between how and one when printing status of unmerged data. This fixes an appending of the how message when it is longer than 20, such is the case in some translations such as the french one where the colon gets appended to the file: supprimé par nous :wt-status.c modifié des deux côtés :wt-status.h Additionally, having a space makes the file in question easier to select in console to quickly address the problem. Without the space, the colon (and, sometimes the last word) of the message is selected along with the file. The previous french example should now print as, which is more proper: supprimé par nous : wt-status.c modifié des deux côtés : wt-status.h try 2: Add function so wt_status_print_unmerged_data() and wt_status_print_change_data() make use of the same padding technique defined as wt_status_status_padding_string() This has the additionnal advantage of aligning unmerged paths with paths of regular statuses. Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com --- t/t7060-wtstatus.sh | 16 +++ t/t7506-status-submodule.sh | 18 t/t7508-status.sh | 94 +++ t/t7512-status-help.sh | 30 ++--- wt-status.c | 104 +--- 5 files changed, 149 insertions(+), 113 deletions(-) diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh index 7d467c0..f49f3b3 100755 --- a/t/t7060-wtstatus.sh +++ b/t/t7060-wtstatus.sh @@ -38,7 +38,7 @@ You have unmerged paths. Unmerged paths: (use git add/rm file... as appropriate to mark resolution) - deleted by us: foo + deleted by us: foo no changes added to commit (use git add and/or git commit -a) EOF @@ -142,8 +142,8 @@ You have unmerged paths. Unmerged paths: (use git add/rm file... as appropriate to mark resolution) - both added: conflict.txt - deleted by them:main.txt + both added: conflict.txt + deleted by them: main.txt no changes added to commit (use git add and/or git commit -a) EOF @@ -175,9 +175,9 @@ You have unmerged paths. Unmerged paths: (use git add/rm file... as appropriate to mark resolution) - both deleted: main.txt - added by them: sub_master.txt - added by us:sub_second.txt + both deleted:main.txt + added by them: sub_master.txt + added by us: sub_second.txt no changes added to commit (use git add and/or git commit -a) EOF @@ -198,12 +198,12 @@ You have unmerged paths. Changes to be committed: - new file: sub_master.txt + new file:sub_master.txt Unmerged paths: (use git rm file... to mark resolution) - both deleted: main.txt + both deleted:main.txt Untracked files not listed (use -u option to show untracked files) EOF diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index d31b34d..745d88b 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -38,7 +38,7 @@ test_expect_success 'status with modified file in submodule' ' (cd sub git reset --hard) echo changed sub/foo git status output - test_i18ngrep modified: sub (modified content) output + test_i18ngrep modified:sub (modified content) output ' test_expect_success 'status with modified file in submodule (porcelain)' ' @@ -53,7 +53,7 @@ test_expect_success 'status with modified file in submodule (porcelain)' ' test_expect_success 'status with added file in submodule' ' (cd sub git reset --hard echo foo git add foo) git status output - test_i18ngrep modified: sub (modified content) output + test_i18ngrep modified:sub (modified content) output ' test_expect_success 'status with added file in submodule (porcelain)' ' @@ -68,7 +68,7 @@ test_expect_success 'status with untracked file in submodule' ' (cd sub git reset --hard) echo content sub/new-file git status output - test_i18ngrep modified: sub (untracked content) output + test_i18ngrep modified:sub (untracked content) output ' test_expect_success 'status -uno with untracked file in submodule' ' @@ -87,7 +87,7 @@ test_expect_success 'status with added and untracked file in submodule' ' (cd sub git reset --hard echo foo git add foo) echo content sub/new-file git status output - test_i18ngrep modified: sub (modified content, untracked content) output + test_i18ngrep modified:sub (modified content, untracked content) output ' test_expect_success 'status with added and untracked file in submodule (porcelain)' ' @@ -105,7 +105,7 @@ test_expect_success 'status with modified file in modified submodule' ' (cd sub echo next change foo git commit -m next change foo) echo changed sub/foo git
Re: What's cooking in git.git (Mar 2014, #02; Tue, 11)
On Wed, Mar 12, 2014 at 5:12 AM, Junio C Hamano gits...@pobox.com wrote: * nd/log-show-linear-break (2014-02-10) 1 commit - log: add --show-linear-break to help see non-linear history Attempts to show where a single-strand-of-pearls break in git log output. Will merge to 'next'. Hold this one. I haven't found time to check again if any rev-list combincation may break it. The option name is coule use some improvement too. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] implement submodule config cache for lookup of submodule names
Hi,
Some quick thoughts.
Heiko Voigt wrote:
This submodule configuration cache allows us to lazily read .gitmodules
configurations by commit into a runtime cache which can then be used to
easily lookup values from it. Currently only the values for path or name
are stored but it can be extended for any value needed.
Makes sense.
[...]
Next I am planning to extend this so configuration cache so it will
return the local configuration (with the usual .gitmodules,
/etc/gitconfig, .git/config, ... overlay of variables) when you pass it
null_sha1 for gitmodule or commit sha1. That way we can give this
configuration cache some use and implement its usage for the current
configuration variables in submodule.c.
Yay!
I wonder whether local configuration should also override information
from commits at times.
[...]
--- /dev/null
+++ b/Documentation/technical/api-submodule-config-cache.txt
@@ -0,0 +1,39 @@
+submodule config cache API
+==
Most API documents in Documentation/technical have a section explaining
general usage --- e.g. (from api-revision-walking),
Calling sequence
The walking API has a given calling sequence: first you need to
initialize a rev_info structure, then add revisions to control what kind
of revision list do you want to get, finally you can iterate over the
revision list.
Or (from api-string-list):
The caller:
. Allocates and clears a `struct string_list` variable.
. Initializes the members. You might want to set the flag
`strdup_strings`
if the strings should be strdup()ed. For example, this is necessary
[...]
. Can remove items not matching a criterion from a sorted or unsorted
list using `filter_string_list`, or remove empty strings using
`string_list_remove_empty_items`.
. Finally it should free the list using `string_list_clear`.
This makes it easier to get started by looking at the documentation alone
without having to mimic an example.
Another thought: it's tempting to call this the 'submodule config API' and
treat the (optional?) memoization as an implementation detail instead of
reminding the caller of it too much. But I haven't thought that through
completely.
[...]
+`struct submodule_config *get_submodule_config_from_path(
+ struct submodule_config_cache *cache,
+ const unsigned char *commit_sha1,
+ const char *path)::
+
+ Lookup a submodules configuration by its commit_sha1 and path.
The cache just always grows until it's freed, right? Would it make
sense to allow cached configurations to be explicitly evicted when the
caller is done with them some day? (Just curious, not a complaint.
Might be worth mentioning in the overview how the cache is expected to
be used, though.)
[...]
--- /dev/null
+++ b/submodule-config-cache.h
@@ -0,0 +1,26 @@
+#ifndef SUBMODULE_CONFIG_CACHE_H
+#define SUBMODULE_CONFIG_CACHE_H
+
+#include hashmap.h
+#include strbuf.h
+
+struct submodule_config_cache {
+ struct hashmap for_path;
+ struct hashmap for_name;
+};
Could use comments (e.g., saying what keys each maps to what values).
Would callers ever look at the hashmaps directly or are they only
supposed to be accessed using helper functions that take a whole
struct?
[...]
+
+/* one submodule_config_cache entry */
+struct submodule_config {
+ struct strbuf path;
+ struct strbuf name;
+ unsigned char gitmodule_sha1[20];
Could also use comments. What does the gitmodule_sha1 field represent?
[...]
--- /dev/null
+++ b/submodule-config-cache.c
@@ -0,0 +1,256 @@
+#include cache.h
+#include submodule-config-cache.h
+#include strbuf.h
+
+struct submodule_config_entry {
+ struct hashmap_entry ent;
+ struct submodule_config *config;
+};
+
+static int submodule_config_path_cmp(const struct submodule_config_entry *a,
+ const struct submodule_config_entry *b,
+ const void *unused)
+{
+ int ret;
+ if ((ret = strcmp(a-config-path.buf, b-config-path.buf)))
+ return ret;
Style: avoid assignments in conditionals:
ret = strcmp(...);
if (ret)
return ret;
return hashcmp(...);
The actual return value from strcmp isn't important since
hashmap_cmp_fn is only used to check for equality. Perhaps as a
separate change it would make sense to make it a hashmap_equality_fn
some day:
return !strcmp(...) !hashcmp(...);
This checks both the path and gitmodule_sha1, so I guess it's for
looking up submodule names.
[...]
+static int submodule_config_name_cmp(const struct submodule_config_entry *a,
+ const struct submodule_config_entry *b,
+ const void *unused)
Likewise.
[...]
+void
Borrowing objects from nearby repositories
Hi all, I am considering developing a new feature, and I'd like to poll the group for opinions. Background: A couple years ago, I wrote a set of scripts that speed up cloning of frequently used repositories. The scripts utilize a bare Git repository located at a known location, and automate providing a --reference parameter to `git clone` and `git submodule update`. Recently, some coworkers of mine expressed an interest in using the scripts, so I published the current version of my scripts, called `git repocache`, described at the bottom of https://github.com/andrewkeller/ak-git-tools. Slowly, it has occurred to me that this feature, or something similar to it, may be worth adding to Git, so I've been thinking about the best approach. Here's my best idea so far: 1) Introduce '--borrow' to `git-fetch`. This would behave similarly to '--reference', except that it operates on a temporary basis, and does not assume that the reference repository will exist after the operation completes, so any used objects are copied into the local objects database. In theory, this mechanism would be distinct from '--reference', so if both are used, some objects would be copied, and some objects would be accessible via a reference repository referenced by the alternates file. 2) Teach `git fetch` to read 'repocache.path' (or a better-named configuration), and use it to automatically activate borrowing. 3) For consistency, `git clone`, `git pull`, and `git submodule update` should probably all learn '--borrow', and forward it to `git fetch`. 4) In some scenarios, it may be necessary to temporarily not automatically borrow, so `git fetch`, and everything that calls it may need an argument to do that. Intended outcome: With 'repocache.path' set, and the cached repository properly updated, one could run `git clone url`, and the operation would complete much faster than it does now due to less load on the network. Things I haven't figured out yet: * What's the best approach to copying the needed objects? It's probably inefficient to copy individual objects out of pack files one at a time, but it could be wasteful to copy entire pack files just because you need one object. Hard-linking could help, but that won't always be available. One of my previous ideas was to add a '--auto-repack' option to `git-clone`, which solves this problem better, but introduces some other front-end usability problems. * To maintain optimal effectiveness, users would have to regularly run a fetch in the cache repository. Not all users know how to set up a scheduled task on their computer, so this might become a maintenance problem for the user. This kind of problem I think brings into question the viability of the underlying design here, assuming that the ultimate goal is to clone faster, with very little or no change in the use of git. Thoughts? Thanks, Andrew Keller -- 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] SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach
Signed-off-by: Yao Zhao zhaox...@umn.edu
---
branch.c | 55 +--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..6432e27 100644
--- a/branch.c
+++ b/branch.c
@@ -53,7 +53,20 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
-
+ char** print_list = malloc(8 * sizeof(char*));
+ char* arg1=NULL;
+ char* arg2=NULL;
+ char* arg3=NULL;
+ int index=0;
+
+ print_list[7] = _(Branch %s set up to track remote branch %s from %s
by rebasing.);
+ print_list[6] = _(Branch %s set up to track remote branch %s from
%s.);
+ print_list[5] = _(Branch %s set up to track local branch %s by
rebasing.);
+ print_list[4] = _(Branch %s set up to track local branch %s.);
+ print_list[3] = _(Branch %s set up to track remote ref %s by
rebasing.);
+ print_list[2] = _(Branch %s set up to track remote ref %s.);
+ print_list[1] = _(Branch %s set up to track local ref %s by
rebasing.);
+ print_list[0] = _(Branch %s set up to track local ref %s.);
if (remote_is_branch
!strcmp(local, shortname)
!origin) {
@@ -77,7 +90,44 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
+ if(remote_is_branch)
+ index += 4;
+ if(origin)
+ index += 2;
+ if(rebasing)
+ index += 1;
+
+ if(index 0 || index 7)
+ {
+ die(BUG: impossible combination of %d and %p,
+ remote_is_branch, origin);
+ }
+
+ if(index = 4) {
+ arg1 = local;
+ arg2 = remote;
+ }
+ else if(index 6) {
+ arg1 = local;
+ arg2 = shortname;
+ arg3 = origin;
+ }
+ else {
+ arg1 = local;
+ arg2 = shortname;
+ }
+
+ if(!arg3) {
+ printf_ln(print_list[index],arg1,arg2);
+ }
+ else {
+ printf_ln(print_list[index],arg1,arg2,arg3);
+ }
+
+ free(print_list);
+
+
+/* if (remote_is_branch origin)
printf_ln(rebasing ?
_(Branch %s set up to track remote branch %s
from %s by rebasing.) :
_(Branch %s set up to track remote branch %s
from %s.),
@@ -100,6 +150,7 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
else
die(BUG: impossible combination of %d and %p,
remote_is_branch, origin);
+*/
}
}
--
1.8.3.2
--
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] install_branch_config: simplify verbose messages logic
On Tue, Mar 11, 2014 at 8:33 PM, Paweł Wawruch pa...@aleg.pl wrote:
Replace the chain of if statements with table of strings.
Signed-off-by: Paweł Wawruch pa...@aleg.pl
---
The changes proposed by Junio C Hamano:
Improvement of indentations. Removed an unused variable.
Better, thanks. More below.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/243502
[2]: http://thread.gmane.org/gmane.comp.version-control.git/243849
[3]: http://thread.gmane.org/gmane.comp.version-control.git/243865
The [n]: notation typically is used for footnotes which you reference
in the running text as [n] (or sometimes [*n*], depending upon
preference). If you're simply listing links to previous attempts, it
would be less confusing to say:
v3: http://...
v2: http://...
v1: http://...
branch.c | 41 +++--
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..2a4b911 100644
--- a/branch.c
+++ b/branch.c
@@ -53,6 +53,20 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+ const char *message[][2][2] = {{{
+ N_(Branch %s set up to track remote branch %s from %s by
rebasing.),
+ N_(Branch %s set up to track remote branch %s from %s.),
+ },{
+ N_(Branch %s set up to track local branch %s by rebasing.),
+ N_(Branch %s set up to track local branch %s.),
+ }},{{
+ N_(Branch %s set up to track remote ref %s by rebasing.),
+ N_(Branch %s set up to track remote ref %s.),
+ },{
+ N_(Branch %s set up to track local ref %s by rebasing.),
+ N_(Branch %s set up to track local ref %s.)
+ }}};
+ const char *name = remote_is_branch ? remote : shortname;
if (remote_is_branch
!strcmp(local, shortname)
@@ -77,29 +91,12 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch
%s from %s by rebasing.) :
- _(Branch %s set up to track remote branch
%s from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch
%s by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s
by rebasing.) :
- _(Branch %s set up to track remote ref
%s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s
by rebasing.) :
- _(Branch %s set up to track local ref
%s.),
- local, remote);
+ if (origin remote_is_branch)
+
printf_ln(_(message[!remote_is_branch][!origin][!rebasing]),
+ local, name, origin);
else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+
printf_ln(_(message[!remote_is_branch][!origin][!rebasing]),
+ local, name);
Shouldn't this logic also be encoded in the table? After all, the
point of making the code table-driven is so that such hard-coded logic
can be avoided. It shouldn't be difficult to do.
The same argument also applies to computation of the 'name' variable
above. It too can be pushed into the the table.
}
}
--
1.8.3.2
--
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
