Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)
On 2013-08-01 22.51, Ramsay Jones wrote: > Junio C Hamano wrote: >> Ramsay Jones writes: >> I am personally in favor of this simpler solution. Comments? >>> >>> I had expected this to me marked for 'master'. >>> >>> Has this simply been overlooked, or do you have reservations about >>> applying this patch? >> >> I am just being careful and do want to keep it cooking in 'next' >> during the feature freeze. The more users work with 'next' (not >> "work *on* 'next'"), the more confidence we would be with, and >> hopefully this can be one of the topis that graduate early after >> the 1.8.4 release. > > Hmm, this patch is a bug-fix for a bug that (currently) will be > _introduced_ by v1.8.4. > > Do you want me to try and find a different bug-fix for v1.8.4? > (Although that would most likely be more risky than simply taking > this patch! ;-) ). > > ATB, > Ramsay Jones I just managed to run v1.8.4-rc1 under cygwin 1.7, and it all passed. Good work, thanks. I realized that core.filemode is true by default, which by default switches of the stat()/lstat() code in cygwin.c Which bug fix are we missing for v1.8.4 ? /Torsten -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Please pull l10n updates for 1.8.4 round 1
Hi, Junio Please pull these updates for git l10n. BTW, Ralf's updates for de.po are still in the review process in this list, but I want to send this pull request earlier, because I find there are some new l10n changes (5 new/modified messages) in v1.8.4-rc1. I will start git 1.8,4 l10n rnd 2 right after gitster closes this pull request. The following changes since commit c490a6079021a3343ca5b042b37258858fdefbfc: Git 1.8.4-rc0 (2013-07-24 19:29:07 -0700) are available in the git repository at: git://github.com/gotgit/git-po for you to fetch changes up to 2e8451e8602380a8a129f21da6364f3ea879e6a9: l10n: zh_CN.po: translate 99 messages (2133t0f0u) (2013-08-03 14:14:07 +0800) Jiang Xin (2): l10n: git.pot: v1.8.4 round 1 (99 new, 46 removed) l10n: zh_CN.po: translate 99 messages (2133t0f0u) Tran Ngoc Quan (1): l10n: vi.po (2133t) po/git.pot | 3072 + po/vi.po| 3383 +-- po/zh_CN.po | 3300 - 3 files changed, 5403 insertions(+), 4352 deletions(-) -- Jiang Xin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Rewriting git-repack.sh in C
On Fri, Aug 02, 2013 at 09:10:59PM +0700, Duy Nguyen wrote: > > So my question is, how you'd generally approach rewriting a > > shell script in C. > > Start a new process via start_command/run_command interface. It's > safer to retain the process boundary at this stage. You can try to > integrate further later. Is it really the right approach to just replace sh with C? IMHO forking and wait for the result should not be done if it can be avoided. It just adds overhead. Of course you can argue that just replace sh with C is a good first step towards to actually do the command in "full C". Altough I'm afraid that that will get such low priority that it won't be done. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.com -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] gc: reject if another gc is running, unless --force is given
This may happen when `git gc --auto` is run automatically, then the
user, to avoid wait time, switches to a new terminal, keeps working
and `git gc --auto` is started again because the first gc instance has
not clean up the repository.
This patch tries to avoid multiple gc running, especially in --auto
mode. In the worst case, gc may be delayed 12 hours if a daemon reuses
the pid stored in gc-%s.pid.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
v2 deals with the stored pid taken by another process and takes
shared repository into account (sort of, it only makes sure gc is not
run multiple times on the same machine, not only one gc instance across
multiple machines)
I changed mingw.h to add a stub uname() because I don't think MinGW
port has that function, but that's totally untested.
Documentation/git-gc.txt | 6 +-
builtin/gc.c | 35 +++
compat/mingw.h | 6 ++
git-compat-util.h| 1 +
4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2402ed6..e158a3b 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local
repository
SYNOPSIS
[verse]
-'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
+'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
[--force]
DESCRIPTION
---
@@ -72,6 +72,10 @@ automatic consolidation of packs.
--quiet::
Suppress all progress reports.
+--force::
+ Force `git gc` to run even if there may be another `git gc`
+ instance running on this repository.
+
Configuration
-
diff --git a/builtin/gc.c b/builtin/gc.c
index 6be6c8d..8a7f24a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -172,6 +172,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
int aggressive = 0;
int auto_gc = 0;
int quiet = 0;
+ int force = 0;
+ FILE *fp;
+ struct utsname utsname;
+ struct stat st;
struct option builtin_gc_options[] = {
OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -180,6 +184,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOLEAN(0, "aggressive", &aggressive, N_("be more thorough
(increased runtime)")),
OPT_BOOLEAN(0, "auto", &auto_gc, N_("enable auto-gc mode")),
+ OPT_BOOL(0, "force", &force, N_("force running gc even if there
may be another gc running")),
OPT_END()
};
@@ -225,6 +230,34 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
} else
add_repack_all_option();
+ if (uname(&utsname))
+ strcpy(utsname.nodename, "unknown");
+
+ if (!force &&
+ (fp = fopen(git_path("gc-%s.pid", utsname.nodename), "r")) != NULL
&&
+ !fstat(fileno(fp), &st) &&
+ /*
+* 12 hour limit is very generous as gc should never take
+* that long. On the other hand we don't really need a
+* strict limit here, running gc --auto one day late is
+* not a big problem. --force can be used in manual gc
+* after the user verifies that no gc is running.
+*/
+ time(NULL) - st.st_mtime <= 12 * 3600) {
+ uintmax_t pid;
+ if (fscanf(fp, "%"PRIuMAX, &pid) == 1 && !kill(pid, 0)) {
+ if (auto_gc)
+ return 0; /* be quiet on --auto */
+ die(_("gc is already running"));
+ }
+ fclose(fp);
+ }
+
+ if ((fp = fopen(git_path("gc-%s.pid", utsname.nodename), "w")) != NULL)
{
+ fprintf(fp, "%"PRIuMAX"\n", (uintmax_t) getpid());
+ fclose(fp);
+ }
+
if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
return error(FAILED_RUN, pack_refs_cmd.argv[0]);
@@ -249,5 +282,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
warning(_("There are too many unreachable loose objects; "
"run 'git prune' to remove them."));
+ unlink(git_path("gc-%s.pid", utsname.nodename));
+
return 0;
}
diff --git a/compat/mingw.h b/compat/mingw.h
index bd0a88b..2c25d2d 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -68,6 +68,10 @@ struct itimerval {
};
#define ITIMER_REAL 0
+struct utsname {
+ char nodename[128];
+};
+
/*
* sanitize preprocessor namespace polluted by Windows headers defining
* macros which collide with git local versions
@@ -86,6 +90,8 @@ static inline int fchmod(int fildes, mode_t mode)
{ errno = ENOSYS; return -1; }
static inline pid_t fork(void)
{ errno = ENOSYS; return -1; }
+static inline int uname(s
Re: [BUG?] gc and impatience
On Sat, Aug 3, 2013 at 11:44 AM, Junio C Hamano wrote: > On Fri, Aug 2, 2013 at 8:53 PM, Duy Nguyen wrote: >> Good point. I think that is because gc does not check if gc is already >> running. Adding such a check should not be too hard. I think gc could >> save its pid in $GIT_DIR/auto-gc.pid. The next auto-gc checks this, if >> the pid is valid, skip auto-gc. > > Defining "valid" is a tricky business, though, as pid can and will > wrap around, Yes there is a chance that the old pid is not used for another process and it could get worse when that process is a daemon and runs forever. If we go the optimistic way, we could check mtime of auto-gc.pid. If it's older than a couple hours, ignore it and run gc anyway, assuming gc can't last longer than an hour or so. A more reliable way is save a unix socket instead of auto-gc.pid and send something over the socket to verify it's gc, but I think it's overkill. > and the directory could be shared on multiple machines. A > pid written by a process on one machine has no relation to any pid on > another machine. I worry less about this. It's not the right model to have two machines modify the same shared repository (gc --auto is only triggered when we think there are new objects) even though I think we support it. If it's two _scripts_ modifying the same repo, I don't care as this is more about user interaction. If it's two people modifying the same repo, it sounds like an insane setup and there may be more issues to worry about than gc --auto. -- 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
[PATCH] cherry-pick: allow "-" as abbreviation of '@{-1}'
As "git cherry-pick -" or "git merge -" is convenient to
switch back to or merge the previous branch,
"git cherry-pick -" is abbreviation of "git cherry-pick @{-1}"
to pick up a commit from the previous branch conveniently.
Signed-off-by: Hiroshige Umino
---
builtin/revert.c| 2 ++
t/t3512-cherry-pick-last.sh | 28
2 files changed, 30 insertions(+)
create mode 100755 t/t3512-cherry-pick-last.sh
diff --git a/builtin/revert.c b/builtin/revert.c
index 1d2648b..cb403f8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -234,6 +234,8 @@ int cmd_cherry_pick(int argc, const char **argv, const char
*prefix)
memset(&opts, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
+ if (!strcmp(argv[1], "-"))
+ argv[1] = "@{-1}";
parse_args(argc, argv, &opts);
res = sequencer_pick_revisions(&opts);
if (res < 0)
diff --git a/t/t3512-cherry-pick-last.sh b/t/t3512-cherry-pick-last.sh
new file mode 100755
index 000..8b05f81
--- /dev/null
+++ b/t/t3512-cherry-pick-last.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo hello >world &&
+ git add world &&
+ git commit -m initial &&
+ git branch other &&
+ echo "hello again" >>world &&
+ git add world &&
+ git commit -m second
+'
+
+test_expect_success '"cherry-pick -" does not work initially' '
+ test_must_fail git cherry-pick -
+'
+
+test_expect_success 'cherry-pick the commit in the previous branch' '
+ prev=$(git rev-parse HEAD) &&
+ git checkout other &&
+ git cherry-pick - &&
+ test "z$(git rev-parse HEAD)" = "z$prev"
+'
+
+test_done
--
1.8.3.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] gc and impatience
On Fri, Aug 2, 2013 at 8:53 PM, Duy Nguyen wrote: > Good point. I think that is because gc does not check if gc is already > running. Adding such a check should not be too hard. I think gc could > save its pid in $GIT_DIR/auto-gc.pid. The next auto-gc checks this, if > the pid is valid, skip auto-gc. Defining "valid" is a tricky business, though, as pid can and will wrap around, and the directory could be shared on multiple machines. A pid written by a process on one machine has no relation to any pid on another machine. -- To unsubscribe from this list: send the line "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] gc: reject if another gc is running, unless --force is given
This may happen when `git gc --auto` is run automatically, then the
user, to avoid wait time, switches to a new terminal, keeps working
and `git gc --auto` is started again because the first gc instance has
not clean up the repository.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
I don't know if the kill(pid, 0) trick works on Windows..
Documentation/git-gc.txt | 6 +-
builtin/gc.c | 18 ++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2402ed6..e158a3b 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local
repository
SYNOPSIS
[verse]
-'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
+'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
[--force]
DESCRIPTION
---
@@ -72,6 +72,10 @@ automatic consolidation of packs.
--quiet::
Suppress all progress reports.
+--force::
+ Force `git gc` to run even if there may be another `git gc`
+ instance running on this repository.
+
Configuration
-
diff --git a/builtin/gc.c b/builtin/gc.c
index 6be6c8d..07c566c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -169,9 +169,11 @@ static int need_to_gc(void)
int cmd_gc(int argc, const char **argv, const char *prefix)
{
+ FILE *fp;
int aggressive = 0;
int auto_gc = 0;
int quiet = 0;
+ int force = 0;
struct option builtin_gc_options[] = {
OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -180,6 +182,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOLEAN(0, "aggressive", &aggressive, N_("be more thorough
(increased runtime)")),
OPT_BOOLEAN(0, "auto", &auto_gc, N_("enable auto-gc mode")),
+ OPT_BOOL(0, "force", &force, N_("force running gc even if there
may be another gc running")),
OPT_END()
};
@@ -225,6 +228,21 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
} else
add_repack_all_option();
+ if (!force && (fp = fopen(git_path("gc.pid"), "r")) != NULL) {
+ uintmax_t pid;
+ if (fscanf(fp, "%"PRIuMAX, &pid) == 1 && !kill(pid, 0)) {
+ if (auto_gc)
+ return 0; /* be quiet on --auto */
+ die(_("gc is already running"));
+ }
+ fclose(fp);
+ }
+
+ if ((fp = fopen(git_path("gc.pid"), "w")) != NULL) {
+ fprintf(fp, "%"PRIuMAX"\n", (uintmax_t) getpid());
+ fclose(fp);
+ }
+
if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
return error(FAILED_RUN, pack_refs_cmd.argv[0]);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] gc and impatience
On Sat, Aug 3, 2013 at 8:48 AM, Ramkumar Ramachandra wrote: > Auto packing the repository for optimum performance. You may also > run "git gc" manually. See "git help gc" for more information. > > Being my usual impatient self, I opened another prompt and started > merging changes. After the checkout, it started running another gc > (why!?), Good point. I think that is because gc does not check if gc is already running. Adding such a check should not be too hard. I think gc could save its pid in $GIT_DIR/auto-gc.pid. The next auto-gc checks this, if the pid is valid, skip auto-gc. > Why is gc not designed for impatient people, and what needs to be done > to change this? Some improvements could be made in gc, for example warn users about upcoming gc so they can run it in background (of course the above bug should be fixed) http://thread.gmane.org/gmane.comp.version-control.git/197716/focus=197877 or speed up repack by implementing pack-objects --merge-pack: http://thread.gmane.org/gmane.comp.version-control.git/57672/focus=57943 Or you could just make a cron job to gc all repos every week and the problem goes away ;-) -- 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
change remote to track new branch
Hi: Long ago I added a remote to my repo. It is set to track what was then WordPress' main release branch (3.4-branch) and created a local branch to use it. Well, time marches on. I want to update my remote and branch to track the new main release branch (3.6-branch). Here's how I set things up at the time: git remote add -t 3.4-branch -f wp https://github.com/WordPress/WordPress git checkout -b wp wp/3.4-branch I've tried various fetch, remote and branch commands to effectuate this change, to no avail. I get messages like "Not a valid ref." I know a major part of this is that my repo only knows of one remote branch (of the many that exist). What I don't know are the commands needed to fetch the right remote branch, set my remote and local branch to track it. Your help will be appreciated, please. Here's some information on the current state of things: git branch -av dev a18677e [ahead 153] backup of 3.4 db master 0d6f9b5 Merge branch 'dev' * wp 42abc67 [ahead 5] Merge branch '3.4-branch' of https://github.com/WordPress/WordPress into wp remotes/git_push_deployer/wordpress c8a5d69 Make my branch names generic. remotes/prod/HEAD -> prod/master remotes/prod/master 0d6f9b5 Merge branch 'dev' remotes/wp/3.4-branch b535358 POT, generated from r24100 remotes/wpconfig/master 3e45a81 LSS 0.27.0. git remote show wp * remote wp Fetch URL: https://github.com/WordPress/WordPress Push URL: https://github.com/WordPress/WordPress HEAD branch: master Remote branch: 3.4-branch tracked Local branch configured for 'git pull': wp merges with remote 3.4-branch Local ref configured for 'git push': master pushes to master (local out of date) Thanks, --Dan -- T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y data intensive web and database programming http://www.AnalysisAndSolutions.com/ 4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG?] gc and impatience
Hi, I was pulling in some changes in the morning to find: Auto packing the repository for optimum performance. You may also run "git gc" manually. See "git help gc" for more information. Being my usual impatient self, I opened another prompt and started merging changes. After the checkout, it started running another gc (why!?), which I attempted to kill using ^C. Counting objects: 449291 x$ It didn't just fail to stop, but it kept writing output making my prompt completely unusable. I finally just killed the pane. Now, it's struggling to update-index and update my cache (read: more waiting). Why is gc not designed for impatient people, and what needs to be done to change this? Ram -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git_mkstemps: improve test suite test
Hi, Dale R. Worley wrote: > Commit 52749 fixes a bug regarding testing the return of an open() $ git show 52749 fatal: ambiguous argument '52749': unknown revision or path not in the working tree. Could you mention its subject line or date so it's easier to find? > call for success/failure. Improve the testsuite test for that fix by > removing the helper program 'test-close-fd-0' and replacing it with > the shell redirection '<&-'. (The redirection is Posix, so it should > be portable.) > > Signed-off-by: Dale Worley [...] > Someone has gone ahead and made the code change, so all that remains > is to update the testsuite test by replacing the helper program > 'test-close-fd-0' with the Posix shell redirection '<&-'. The above paragraph should be part of the commit message, since otherwise the patch is hard to understand. The patch text looks good. Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line "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] git_mkstemps: improve test suite test
Commit 52749 fixes a bug regarding testing the return of an open()
call for success/failure. Improve the testsuite test for that fix by
removing the helper program 'test-close-fd-0' and replacing it with
the shell redirection '<&-'. (The redirection is Posix, so it should
be portable.)
Signed-off-by: Dale Worley
---
> From: Junio C Hamano
> Date: Fri, 19 Jul 2013 07:29:47 -0700
>
> The change itself looks good; care to write it up as a proper patch
> with a proposed log message?
My apologies for the delay; I've had to do some yak-shaving to learn
how to construct patches properly. (I've written some clarifications
for Document/SubmittingPatches, which I will submit separately.)
Someone has gone ahead and made the code change, so all that remains
is to update the testsuite test by replacing the helper program
'test-close-fd-0' with the Posix shell redirection '<&-'.
Dale
Makefile |1 -
test-close-fd-0.c | 14 --
2 files changed, 0 insertions(+), 15 deletions(-)
delete mode 100644 test-close-fd-0.c
diff --git a/Makefile b/Makefile
index 8ad40d4..3588ca1 100644
--- a/Makefile
+++ b/Makefile
@@ -557,7 +557,6 @@ X =
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
-TEST_PROGRAMS_NEED_X += test-close-fd-0
TEST_PROGRAMS_NEED_X += test-ctype
TEST_PROGRAMS_NEED_X += test-date
TEST_PROGRAMS_NEED_X += test-delta
diff --git a/test-close-fd-0.c b/test-close-fd-0.c
deleted file mode 100644
index 3745c34..000
--- a/test-close-fd-0.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include
-
-/* Close file descriptor 0 (which is standard-input), then execute the
- * remainder of the command line as a command. */
-
-int main(int argc, char **argv)
-{
- /* Close fd 0. */
- close(0);
- /* Execute the requested command. */
- execvp(argv[1], &argv[1]);
- /* If execve() failed, return an error. */
- return 1;
-}
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view
This commit introduces and uses the log_select function to find the
correct commit in the unsplit log view. In the log view, if one
scrolls down across a commit line, the current commit (as displayed in
the status bar) gets updated, but not so when scrolling upward across
a commit. The log_select function handles this scenario to to the
``right thing''. In addition, it introduces the log_state structure as
the private entry of the log view to hold a flag that decides whether
to re-evaluate the current commit based on scrolling.
Signed-off-by: Kumar Appaiah
---
tig.c | 50 --
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/tig.c b/tig.c
index 72f132a..dd4b0f4 100644
--- a/tig.c
+++ b/tig.c
@@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)
}
}
+struct log_state {
+ bool update_commit_ref;
+};
+
+static void
+log_select(struct view *view, struct line *line)
+{
+ struct log_state *state = (struct log_state *) view->private;
+
+ if (state->update_commit_ref && line->lineno > 1) {
+ /* We need to recalculate the previous commit,
+ since the user has likely scrolled up. */
+ const struct line *commit_line = find_prev_line_by_type(view,
line, LINE_COMMIT);
+
+ if (commit_line)
+ string_copy_rev(view->ref, (char *) (commit_line->data
+ STRING_SIZE("commit ")));
+ }
+ if (line->type == LINE_COMMIT) {
+ char *text = (char *)line->data + STRING_SIZE("commit ");
+
+ if (!view_has_flags(view, VIEW_NO_REF))
+ string_copy_rev(view->ref, text);
+ }
+ string_copy_rev(ref_commit, view->ref);
+ state->update_commit_ref = FALSE;
+}
+
static bool
pager_open(struct view *view, enum open_flags flags)
{
@@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)
static enum request
log_request(struct view *view, enum request request, struct line *line)
{
+ struct log_state *state = (struct log_state *) view->private;
+
switch (request) {
case REQ_REFRESH:
load_refs();
refresh_view(view);
return REQ_NONE;
+
+ case REQ_MOVE_UP:
+ case REQ_PREVIOUS:
+ if (line->type == LINE_COMMIT && line->lineno > 1) {
+ /* We are at a commit, and heading upward. We
+ force log_select to find the previous
+ commit above, from the context. */
+ state->update_commit_ref = TRUE;
+ }
+ return pager_request(view, request, line);
+
+ case REQ_MOVE_PAGE_UP:
+ case REQ_MOVE_PAGE_DOWN:
+ /* We need to figure out the right commit again. */
+ state->update_commit_ref = TRUE;
+ return pager_request(view, request, line);
+
default:
return pager_request(view, request, line);
}
@@ -4441,13 +4487,13 @@ static struct view_ops log_ops = {
"line",
{ "log" },
VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER |
VIEW_NO_PARENT_NAV,
- 0,
+ sizeof(struct log_state),
log_open,
pager_read,
pager_draw,
log_request,
pager_grep,
- pager_select,
+ log_select,
};
struct diff_state {
--
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
[[TIG][PATCH] 2/3] Display correct diff the context in split log view
In the log view, when scrolling across a commit, the diff view should automatically switch to the commit whose context the cursor is on in the log view. This commit changes things to catch the REQ_ENTER in the log view and handle recalculation of the commit and diff display from log_request, rather than delegating it to pager_request. In addition, it also gets rid of unexpected upward scrolling of the log view. Fixes GH #155 Signed-Off-By: Kumar Appaiah --- NEWS | 1 + tig.c | 12 2 files changed, 13 insertions(+) diff --git a/NEWS b/NEWS index 0394407..f59e517 100644 --- a/NEWS +++ b/NEWS @@ -46,6 +46,7 @@ Bug fixes: - Fix rendering glitch for branch names. - Do not apply diff styling to untracked files in the stage view. (GH #153) - Fix tree indentation for entries containing combining characters. (GH #170) + - Introduce a more natural context-sensitive log display. (GH #155) tig-1.1 --- diff --git a/tig.c b/tig.c index dd4b0f4..53947b7 100644 --- a/tig.c +++ b/tig.c @@ -4478,6 +4478,18 @@ log_request(struct view *view, enum request request, struct line *line) state->update_commit_ref = TRUE; return pager_request(view, request, line); + case REQ_ENTER: + /* Recalculate the correct commit for the context. */ + state->update_commit_ref = TRUE; + + open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT); + update_view_title(view); + + /* We don't want to delegate this to pager_request, + since we don't want the extra scrolling of the log + view. */ + return REQ_NONE; + default: return pager_request(view, request, line); } -- 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
[[TIG][PATCH] 3/3] Revert "Scroll diff with arrow keys in log view"
This reverts commit 888611dd5d407775245d574a3dc5c01b5963a5ba. This is
because, in the re-engineered log view, scrolling the log with the
arrows now updates the diff in the diff view when the screen is
split. This resembles the earlier behaviour, and is also what users of
software like Mutt (which uses the pager view concept) would expect.
Signed-Off-By: Kumar Appaiah
Conflicts:
tig.c
---
tig.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tig.c b/tig.c
index 53947b7..65c91a0 100644
--- a/tig.c
+++ b/tig.c
@@ -1901,7 +1901,6 @@ enum view_flag {
VIEW_STDIN = 1 << 8,
VIEW_SEND_CHILD_ENTER = 1 << 9,
VIEW_FILE_FILTER= 1 << 10,
- VIEW_NO_PARENT_NAV = 1 << 11,
};
#define view_has_flags(view, flag) ((view)->ops->flags & (flag))
@@ -3774,7 +3773,7 @@ view_driver(struct view *view, enum request request)
case REQ_NEXT:
case REQ_PREVIOUS:
- if (view->parent && !view_has_flags(view->parent,
VIEW_NO_PARENT_NAV)) {
+ if (view->parent) {
int line;
view = view->parent;
@@ -4498,7 +4497,7 @@ log_request(struct view *view, enum request request,
struct line *line)
static struct view_ops log_ops = {
"line",
{ "log" },
- VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER |
VIEW_NO_PARENT_NAV,
+ VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER,
sizeof(struct log_state),
log_open,
pager_read,
--
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
[TIG][PATCH 0/3] Refactoring of the log view
Hi. These set of patches refactor the log view to provide a behaviour that is quite similar to, say, e-mail with Mutt. The key improvements are: - The current commit is inferred based on the context. For example, if you focus on the commit message of a particular commit, the correct commit is inferred automagically. - Scrolling the log view when the diff is open shows the correct commit on the screen, rather than have to scroll up and cross the commit line to display the screen. I have decided to revert 888611dd5d407775245d574a3dc5c01b5963a5ba, since the behaviour with the updated scrolling pattern is much more consistent. As always, I will gladly alter the patch based on comments on coding style and all other aspects. Thanks! Kumar Kumar Appaiah (3): Add log_select function to find commit from context in log view Display correct diff the context in split log view Revert "Scroll diff with arrow keys in log view" NEWS | 1 + tig.c | 67 ++- 2 files changed, 63 insertions(+), 5 deletions(-) -- 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: Making a patch: "git format-patch" does not produce the documented format
wor...@alum.mit.edu (Dale R. Worley) writes: > I'm preparing some clarifications of SubmittingPatches to explain > things that a new person (e.g., me) would not know. I am not sure if SubmittingPatches is a good place, though. The document is a guidance for people who contribute to _this_ project. But the specialness of the first paragraph applies to any project that uses Git, so people other than those who contribute to this project should be aware of it. Originally we literally used "first line", but that made many things like shortlog output and patch Subject: useless when people write a block of text starting from the first line without a title. Also after resurrecting such a text from e-mail, "am" couldn't tell if the "first line" on the "Subject:" is meant to be the first line of the same first paragraph (which is not what we encourage), or it is properly a single line title, and need a blank line before the first line of the body. So quite a while ago, we changed the rule to take "the first paragraph" and use that in these places where we want to give a title of a patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] hooks/post-receive-email: force log messages in UTF-8
Git commands write commit messages in UTF-8 by default, but that default can be overridden by the [i18n] commitEncoding and logOutputEncoding settings. With such a setting, the emails written by the post-receive-email hook use a mixture of encodings: 1. Log messages use the configured log output encoding, which is meant to be whatever encoding works best with local terminals (and does not have much to do with what encoding should be used for email) 2. Filenames are left as is: on Linux, usually UTF-8, and in the Mingw port (which uses Unicode filesystem APIs), always UTF-8 3. The "This is an automated email" preface uses a project description from .git/description, which is typically in UTF-8 to support gitweb. So (1) is configurable, and (2) and (3) are unconfigurable and typically UTF-8. Override the log output encoding to always use UTF-8 when writing the email to get the best chance of a comprehensible single-encoding email. Signed-off-by: Jonathan Nieder --- contrib/hooks/post-receive-email | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index 72084511..ba93a0d8 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -471,7 +471,7 @@ generate_delete_branch_email() echo " was $oldrev" echo "" echo $LOGBEGIN - git diff-tree -s --always --pretty=oneline $oldrev + git diff-tree -s --always --encoding=UTF-8 --pretty=oneline $oldrev echo $LOGEND } @@ -571,7 +571,7 @@ generate_delete_atag_email() echo " was $oldrev" echo "" echo $LOGBEGIN - git diff-tree -s --always --pretty=oneline $oldrev + git diff-tree -s --always --encoding=UTF-8 --pretty=oneline $oldrev echo $LOGEND } @@ -617,7 +617,7 @@ generate_general_email() echo "" if [ "$newrev_type" = "commit" ]; then echo $LOGBEGIN - git diff-tree -s --always --pretty=medium $newrev + git diff-tree -s --always --encoding=UTF-8 --pretty=medium $newrev echo $LOGEND else # What can we do here? The tag marks an object that is not @@ -636,7 +636,7 @@ generate_delete_general_email() echo " was $oldrev" echo "" echo $LOGBEGIN - git diff-tree -s --always --pretty=oneline $oldrev + git diff-tree -s --always --encoding=UTF-8 --pretty=oneline $oldrev echo $LOGEND } -- 1.8.4.rc1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] hooks/post-receive-email: set declared encoding to utf-8
From: Gerrit Pape
Date: Thu, 11 Dec 2008 20:27:21 +
Some email clients (e.g., claws-mail) display the message body
incorrectly when the charset is not defined explicitly in a
Content-Type header. "git log" generates logs in UTF-8 encoding by
default, so add a Content-Type header declaring that encoding to
the emails the post-receive-email example hook sends.
[jn: also setting the Content-Transfer-Encoding so MTAs know what
kind of mangling might be needed when sending to a non 8-bit clean
SMTP host]
Requested-by: Alexander Gerasiov
Signed-off-by: Jonathan Nieder
---
That's the end of the series. Thanks for reading.
contrib/hooks/post-receive-email | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index ba93a0d8..8ee410f8 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -242,6 +242,9 @@ generate_email_header()
cat <<-EOF
To: $recipients
Subject: ${emailprefix}$projectdesc $refname_type $short_refname
${change_type}d. $describe
+ MIME-Version: 1.0
+ Content-Type: text/plain; charset=utf-8
+ Content-Transfer-Encoding: 8bit
X-Git-Refname: $refname
X-Git-Reftype: $refname_type
X-Git-Oldrev: $oldrev
--
1.8.4.rc1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] hooks/post-receive-email: use plumbing instead of git log/show
This way the hook doesn't have to keep being tweaked as porcelain learns new features like color and pagination. While at it, replace the "git rev-list | git shortlog" idiom with plain "git shortlog" for simplicity. Except for depending less on the value of settings like '[log] abbrevCommit', no change in output intended. Signed-off-by: Jonathan Nieder --- contrib/hooks/post-receive-email | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index 15311502..72084511 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -471,7 +471,7 @@ generate_delete_branch_email() echo " was $oldrev" echo "" echo $LOGBEGIN - git show -s --pretty=oneline $oldrev + git diff-tree -s --always --pretty=oneline $oldrev echo $LOGEND } @@ -547,11 +547,11 @@ generate_atag_email() # performed on them if [ -n "$prevtag" ]; then # Show changes since the previous release - git rev-list --pretty=short "$prevtag..$newrev" | git shortlog + git shortlog "$prevtag..$newrev" else # No previous tag, show all the changes since time # began - git rev-list --pretty=short $newrev | git shortlog + git shortlog $newrev fi ;; *) @@ -571,7 +571,7 @@ generate_delete_atag_email() echo " was $oldrev" echo "" echo $LOGBEGIN - git show -s --pretty=oneline $oldrev + git diff-tree -s --always --pretty=oneline $oldrev echo $LOGEND } @@ -617,7 +617,7 @@ generate_general_email() echo "" if [ "$newrev_type" = "commit" ]; then echo $LOGBEGIN - git show --no-color --root -s --pretty=medium $newrev + git diff-tree -s --always --pretty=medium $newrev echo $LOGEND else # What can we do here? The tag marks an object that is not @@ -636,7 +636,7 @@ generate_delete_general_email() echo " was $oldrev" echo "" echo $LOGBEGIN - git show -s --pretty=oneline $oldrev + git diff-tree -s --always --pretty=oneline $oldrev echo $LOGEND } -- 1.8.4.rc1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/3] post-receive-email: explicitly set Content-Type header
Hi all, This is a revival of [1], which declares encoding in emails to make it more likely that they can be read. I like to think it avoids the mistakes of previous attempts, but I'll let you judge. :) Sorry for the long delay. Thoughts of all kinds welcome, as always. Thanks, Jonathan Gerrit Pape (1): hooks/post-receive-email: set declared encoding to utf-8 Jonathan Nieder (2): hooks/post-receive-email: use plumbing instead of 'git log' and 'git show' hooks/post-receive-email: force log messages in UTF-8 contrib/hooks/post-receive-email | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) [1] http://thread.gmane.org/gmane.comp.version-control.git/181737/focus=183070 -- To unsubscribe from this list: send the line "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 9/6] push: teach --force-with-lease to smart-http transport
We have been passing enough information to enable the
compare-and-swap logic down to the transport layer, but the
transport helper was not passing it to smart-http transport.
Signed-off-by: Junio C Hamano
---
* I didn't bother with the dumb commit walker push for obvious
reasons, but if somebody is inclined to add one, it shouldn't be
too hard to add.
remote-curl.c| 16 +++-
t/lib-httpd.sh | 3 ++-
t/t5541-http-push.sh | 2 +-
transport-helper.c | 24 ++--
4 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 60eda63..53c8a3d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -6,6 +6,7 @@
#include "exec_cmd.h"
#include "run-command.h"
#include "pkt-line.h"
+#include "string-list.h"
#include "sideband.h"
static struct remote *remote;
@@ -20,6 +21,7 @@ struct options {
thin : 1;
};
static struct options options;
+static struct string_list cas_options = STRING_LIST_INIT_DUP;
static int set_option(const char *name, const char *value)
{
@@ -66,6 +68,13 @@ static int set_option(const char *name, const char *value)
return -1;
return 0;
}
+ else if (!strcmp(name, "cas")) {
+ struct strbuf val = STRBUF_INIT;
+ strbuf_addf(&val, "--" CAS_OPT_NAME "=%s", value);
+ string_list_append(&cas_options, val.buf);
+ strbuf_release(&val);
+ return 0;
+ }
else {
return 1 /* unsupported */;
}
@@ -789,8 +798,9 @@ static int push_git(struct discovery *heads, int nr_spec,
char **specs)
struct rpc_state rpc;
const char **argv;
int argc = 0, i, err;
+ struct string_list_item *cas_option;
- argv = xmalloc((10 + nr_spec) * sizeof(char*));
+ argv = xmalloc((10 + nr_spec + cas_options.nr) * sizeof(char *));
argv[argc++] = "send-pack";
argv[argc++] = "--stateless-rpc";
argv[argc++] = "--helper-status";
@@ -803,6 +813,10 @@ static int push_git(struct discovery *heads, int nr_spec,
char **specs)
else if (options.verbosity > 1)
argv[argc++] = "--verbose";
argv[argc++] = options.progress ? "--progress" : "--no-progress";
+
+ for_each_string_list_item(cas_option, &cas_options)
+ argv[argc++] = cas_option->string;
+
argv[argc++] = url;
for (i = 0; i < nr_spec; i++)
argv[argc++] = specs[i];
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index e2eca1f..dab405d 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -141,10 +141,11 @@ stop_httpd() {
-f "$TEST_PATH/apache.conf" $HTTPD_PARA -k stop
}
-test_http_push_nonff() {
+test_http_push_nonff () {
REMOTE_REPO=$1
LOCAL_REPO=$2
BRANCH=$3
+ EXPECT_CAS_RESULT=${4-failure}
test_expect_success 'non-fast-forward push fails' '
cd "$REMOTE_REPO" &&
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index beb00be..470ac54 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -153,7 +153,7 @@ test_expect_success 'used receive-pack service' '
'
test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
- "$ROOT_PATH"/test_repo_clone master
+ "$ROOT_PATH"/test_repo_clone master success
test_expect_success 'push fails for non-fast-forward refs unmatched by remote
helper' '
# create a dissimilarly-named remote ref so that git is unable to match
the
diff --git a/transport-helper.c b/transport-helper.c
index 95d22f8..e3a60d7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -742,13 +742,15 @@ static void push_update_refs_status(struct helper_data
*data,
}
static int push_refs_with_push(struct transport *transport,
- struct ref *remote_refs, int flags)
+ struct ref *remote_refs, int flags)
{
int force_all = flags & TRANSPORT_PUSH_FORCE;
int mirror = flags & TRANSPORT_PUSH_MIRROR;
struct helper_data *data = transport->data;
struct strbuf buf = STRBUF_INIT;
struct ref *ref;
+ struct string_list cas_options = STRING_LIST_INIT_DUP;
+ struct string_list_item *cas_option;
get_helper(transport);
if (!data->push)
@@ -784,11 +786,29 @@ static int push_refs_with_push(struct transport
*transport,
strbuf_addch(&buf, ':');
strbuf_addstr(&buf, ref->name);
strbuf_addch(&buf, '\n');
+
+ /*
+* The "--force-with-lease" options without explicit
+* values to expect have already been expanded into
+* the ref->old_sha1_expect[] field; we can ignore
+* transport->smart_options->cas altogether and instead
+* can enumerate them from the refs.
+*/
+ if (ref->expect_ol
[PATCH v3 8/6] send-pack: fix parsing of --force-with-lease option
The last argument for parse_push_cas_option() is if it is "unset"
(i.e. --no-force-with-lease), and we are parsing the option with an
explicit value here, so it has to be 0.
Signed-off-by: Junio C Hamano
---
builtin/send-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 41dc512..4482f16 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -183,7 +183,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
}
if (!prefixcmp(arg, "--" CAS_OPT_NAME "=")) {
if (parse_push_cas_option(&cas,
- strchr(arg, '=') + 1,
1) < 0)
+ strchr(arg, '=') + 1,
0) < 0)
exit(1);
continue;
}
--
1.8.4-rc1-129-g1f3472b
--
To unsubscribe from this list: send the line "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/RFC] log doc: explain --encoding=none and default output encoding
Signed-off-by: Jonathan Nieder --- I'm not thrilled with the wording. This can probably be explained more simply. Ideas? Documentation/pretty-options.txt | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt index 5e499421..e31fd494 100644 --- a/Documentation/pretty-options.txt +++ b/Documentation/pretty-options.txt @@ -32,8 +32,14 @@ people using 80-column terminals. The commit objects record the encoding used for the log message in their encoding header; this option can be used to tell the command to re-code the commit log message in the encoding - preferred by the user. For non plumbing commands this - defaults to UTF-8. + preferred by the user. "--encoding=none" means to use the + raw log message without paying attention to its encoding header. ++ +For non plumbing commands, the output encoding defaults to the commit +encoding (as set using the `i18n.commitEncoding` variable, or UTF-8 +by default). This default can be overridden using the +`i18n.logOutputEncoding` configuration item. See linkgit:git-config[1] +for details. --notes[=]:: Show the notes (see linkgit:git-notes[1]) that annotate the -- 1.8.4.rc1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] log doc: the argument to --encoding is not optional
$ git log --encoding fatal: Option '--encoding' requires a value $ git rev-list --encoding fatal: Option '--encoding' requires a value The argument to --encoding has always been mandatory. Unfortunately manpages like git-rev-list(1), git-log(1), and git-show(1) have described the option's syntax as "--encoding[=]" since it was first documented. Clarify by removing the extra brackets. Signed-off-by: Jonathan Nieder --- Documentation/git-rev-list.txt | 2 +- Documentation/pretty-options.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index 65ac27e0..045b37b8 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -40,7 +40,7 @@ SYNOPSIS [ \--right-only ] [ \--cherry-mark ] [ \--cherry-pick ] -[ \--encoding[=] ] +[ \--encoding= ] [ \--(author|committer|grep)= ] [ \--regexp-ignore-case | -i ] [ \--extended-regexp | -E ] diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt index 5e499421..eea0e306 100644 --- a/Documentation/pretty-options.txt +++ b/Documentation/pretty-options.txt @@ -28,7 +28,7 @@ people using 80-column terminals. This is a shorthand for "--pretty=oneline --abbrev-commit" used together. ---encoding[=]:: +--encoding=:: The commit objects record the encoding used for the log message in their encoding header; this option can be used to tell the command to re-code the commit log message in the encoding -- 1.8.4.rc1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Making a patch: "git format-patch" does not produce the documented format
> From: John Keeping > > git-format-patch(1) says: > > By default, the subject of a single patch is "[PATCH] " followed > by the concatenation of lines from the commit message up to the > first blank line (see the DISCUSSION section of git-commit(1)). > > I think that accurately describes what you're seeing. The referenced > DISCUSSION section describes how to write a commit message that is > formatted in a suitable way, with a short first subject line and then a > blank line before the body of the message. Thanks for the confirmation. I've figured out what is going wrong: Documentation/SubmittingPatches says: The first line of the commit message should be a short description (50 characters is the soft limit, see DISCUSSION in git-commit(1)), and should skip the full stop. What it *doesn't* say is that the second line of the commit message should be empty -- precisely so that git format-patch turns the first line into the Subject: but does not merge the remainder of the commit message (the "body") into the Subject: line. Now that I know to look for this, I can see that the commit messages in the Git repository show this pattern. I'm preparing some clarifications of SubmittingPatches to explain things that a new person (e.g., me) would not know. To fix this issue, I am inserting: This first line should be a separate paragraph, that is, it should be followed by an empty line, which is then followed by the body of the commit message. Dale -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-p4: use "p4 fstat" to interpret View setting
ksaitoh...@gmail.com wrote on Fri, 02 Aug 2013 17:02 +0900: > I trying clone Perforce project and I found git-p4. It's a great tool! > > And I don't know how to exclude special extension file in a directory? > (Practically, I want to exclude Excel files at git p4 clone/sync.) > > In Perforce, View setting of p4 client can describe > -//depot/project/files/*.xls //client/project/files/*.xls > to exclude Excel files. > But "git p4 --use-client-spec" cannot support '*'. > > In git-p4.py, "map_in_client" method analyzes View setting and return > client file path. > So I modify the method to just use p4 command, "p4 fstat -T clientFile". > > I'd like to know your opinions about that and what you think about the > suggestion. This is either incredibly brilliant or fundamentally broken. I'm hoping for the former. :) Your theory is: there is a client spec, and p4 knows how to interpret these things, so instead of figuring out and implementing the algorithms for %% and * and ... in git-p4, just ask p4 directly. Let me play with this for a bit. I wonder about the performance aspects of doing a "p4 fstat" for every file. Would it be possible to do one or a few batch queries higher up somewhere? A few nit-picky questions below, just on the test bits, while I play with your code. > diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh > index 2098b9b..fbda1ad 100644 > --- a/t/lib-git-p4.sh > +++ b/t/lib-git-p4.sh > @@ -48,6 +48,9 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start))) > P4PORT=localhost:$P4DPORT > P4CLIENT=client > P4EDITOR=: > +P4USER="" > +P4PASS="" > +P4CHARSET="" Why do you need these? > diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh > index 2bf142d..b97bdda 100755 > --- a/t/t9801-git-p4-branch.sh > +++ b/t/t9801-git-p4-branch.sh > @@ -480,6 +480,7 @@ test_expect_success 'use-client-spec detect-branches > skips branches setup' ' > test_expect_success 'use-client-spec detect-branches skips branches' ' > client_view "//depot/usecs/... //client/..." \ > "-//depot/usecs/b3/... //client/b3/..." && > +( p4 sync ) && > test_when_finished cleanup_git && How does the p4 sync help here? > +test_expect_success 'view wildcard *' ' > + client_view "//depot/... //client/..." \ > + "-//depot/dir1/*.junk //client/dir1/*.junk" \ > + "-//depot/dir2/*.junk //client/dir2/*.junk" && > + (p4 sync ) && > + files="dir1/file11 dir1/file12 dir2/file21 dir2/file22" && > + client_verify $files && > + git p4 clone --use-client-spec --dest="$git" //depot && > + git_verify $files > +' It works! Cool. -- Pete -- To unsubscribe from this list: send the line "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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
Junio C Hamano wrote: > Here is what is on top of the revert that has been pushed out on > 'pu'. For what it's worth, Reviewed-by: Jonathan Nieder [...] > To remain backwards compatible, we cannot split on whitespace by > default, hence we will ship 1.8.4 with the commit reverted. [...] > It might be more robust to have something like "-z" to separate the > input elements. But this patch is still a reasonable step before > having that. It makes the easy cases easy; people who do not care > about %(rest) do not have to consider it, and the %(rest) code > handles the spaces and newlines of "rev-list --objects" correctly. Another idea for the future might be to start rejecting refnames starting with a double-quote '"', which would make it safe to treat a leading quote-mark as the start of a C-style quoted string. But currently that would technically be a breaking change, making "-z" more useful in the meantime. I think several commands already don't deal well with filenames with newlines. I hope POSIX forbids them (with some suitable migration plan) soonish and even wouldn't mind if git were taught to refuse to track them. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to hierarchically merge from the root to the leaf of a branch tree? (Patch stack management)
Am 02.08.2013 06:33, schrieb Jens Müller: > Am 01.08.2013 09:28, schrieb Jakub Narebski: >> > There is also TopGit, which is feature-branch management tools (which >> > seems like what you want, from what you written below). > Indeed, thank you very much for pointing me to it. I have not read the > whole documentation, but it sounds promising enough that I will try it > out with some real patches I have flying around and need to combine and > do further development on. Seems nice until now, but lacks some essential functionality ... For example, you can add a dependency to a parent branch, but not remove 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
[PATCH] Documentation/rev-list-options: add missing word in --*-parents
A commit has "parent commits" or "parents", not "commits". Signed-off-by: Torstein Hegge --- Documentation/rev-list-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 27f8de3..5bdfb42 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -119,7 +119,7 @@ if it is part of the log message. --no-min-parents:: --no-max-parents:: - Show only commits which have at least (or at most) that many + Show only commits which have at least (or at most) that many parent commits. In particular, `--max-parents=1` is the same as `--no-merges`, `--min-parents=2` is the same as `--merges`. `--max-parents=0` gives all root commits and `--min-parents=3` all octopus merges. -- 1.8.4.rc1.372.g12d6635 -- To unsubscribe from this list: send the line "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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
Jeff King writes:
> On Fri, Aug 02, 2013 at 09:41:52AM -0700, Junio C Hamano wrote:
>
>> > Of the two, I think the latter is more sensible; the former is
>> > unnecessarily placing the burden on the user to match "--split" with
>> > their use of "%(rest)". The second is pointless without the first.
>> >
>> > A patch to implement (2) is below.
>>
>> As I'd queue this on top of the revert, I had to wrangle it a bit to
>> make it relative, i.e. "this resurrects what the other reverted
>> patch did but in a weaker/safer form".
>
> Yeah, sorry. After doing the patch I had the thought that maybe the
> least invasive thing would be the fix rather than the straight revert
> (we are counting on my assertion that just reverting out part of the
> series will be OK; I'm pretty sure that is the case, but it is not
> risk-free, either).
>
> I didn't see the result of your wrangling in pu, but I will keep an eye
> out to double-check it (unless you did not finish, in which case I am
> happy to do the wrangling myself).
Here is what is on top of the revert that has been pushed out on
'pu'.
Thanks.
-- >8 --
From: Jeff King
Date: Fri, 2 Aug 2013 04:59:07 -0700
Subject: [PATCH] cat-file: only split on whitespace when %(rest) is used
Commit c334b87b (cat-file: split --batch input lines on whitespace,
2013-07-11) taught `cat-file --batch-check` to split input lines on
the first whitespace, and stash everything after the first token
into the %(rest) output format element. It claimed:
Object names cannot contain spaces, so any input with
spaces would have resulted in a "missing" line.
But that is not correct. Refs, object sha1s, and various peeling
suffixes cannot contain spaces, but some object names can. In
particular:
1. Tree paths like "[]:path with whitespace"
2. Reflog specifications like "@{2 days ago}"
3. Commit searches like "rev^{/grep me}" or ":/grep me"
To remain backwards compatible, we cannot split on whitespace by
default, hence we will ship 1.8.4 with the commit reverted.
Resurrect its attempt but in a weaker form; only do the splitting
when "%(rest)" is used in the output format. Since that element did
not exist at all before c334b87, old scripts cannot be affected.
The existence of object names with spaces does mean that you
cannot reliably do:
echo ":path with space and other data" |
git cat-file --batch-check="%(objectname) %(rest)"
as it would split the path and feed only ":path" to get_sha1. But
that command is nonsensical. If you wanted to see "and other data"
in "%(rest)", git cannot possibly know where the filename ends and
the "rest" begins.
It might be more robust to have something like "-z" to separate the
input elements. But this patch is still a reasonable step before
having that. It makes the easy cases easy; people who do not care
about %(rest) do not have to consider it, and the %(rest) code
handles the spaces and newlines of "rev-list --objects" correctly.
Hard cases remain hard but possible (if you might get whitespace in
your input, you do not get to use %(rest) and must split and join
the output yourself using more flexible tools). And most
importantly, it does not preclude us from having different splitting
rules later if a "-z" (or similar) option is added. So we can make
the hard cases easier later, if we choose to.
Signed-off-by: Jeff King
Signed-off-by: Junio C Hamano
---
Documentation/git-cat-file.txt | 14 ++
builtin/cat-file.c | 31 ++-
t/t1006-cat-file.sh| 15 +++
3 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 10fbc6a..21cffe2 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -86,10 +86,9 @@ BATCH OUTPUT
If `--batch` or `--batch-check` is given, `cat-file` will read objects
-from stdin, one per line, and print information about them.
-
-Each line is considered as a whole object name, and is parsed as if
-given to linkgit:git-rev-parse[1].
+from stdin, one per line, and print information about them. By default,
+the whole line is considered as an object, as if it were fed to
+linkgit:git-rev-parse[1].
You can specify the information shown for each object by using a custom
``. The `` is copied literally to stdout for each
@@ -110,6 +109,13 @@ newline. The available atoms are:
The size, in bytes, that the object takes up on disk. See the
note about on-disk sizes in the `CAVEATS` section below.
+`rest`::
+ If this atom is used in the output string, input lines are split
+ at the first whitespace boundary. All characters before that
+ whitespace are considered to be the object name; characters
+ after that first run of whitespace (i.e., the "rest" of the
+ line) are output in place of the `%(rest)` atom.
+
If no format is specified, the default format is `%(objectname)
%(objecttype)
Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
On Fri, Aug 02, 2013 at 09:41:52AM -0700, Junio C Hamano wrote: > > Of the two, I think the latter is more sensible; the former is > > unnecessarily placing the burden on the user to match "--split" with > > their use of "%(rest)". The second is pointless without the first. > > > > A patch to implement (2) is below. > > As I'd queue this on top of the revert, I had to wrangle it a bit to > make it relative, i.e. "this resurrects what the other reverted > patch did but in a weaker/safer form". Yeah, sorry. After doing the patch I had the thought that maybe the least invasive thing would be the fix rather than the straight revert (we are counting on my assertion that just reverting out part of the series will be OK; I'm pretty sure that is the case, but it is not risk-free, either). I didn't see the result of your wrangling in pu, but I will keep an eye out to double-check it (unless you did not finish, in which case I am happy to do the wrangling myself). -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 v3] sha1_file: introduce close_one_pack() to close packs on fd pressure
On Fri, Aug 2, 2013 at 9:26 AM, Junio C Hamano wrote:
> Brandon Casey writes:
>
>> +/*
>> + * The LRU pack is the one with the oldest MRU window, preferring packs
>> + * with no used windows, or the oldest mtime if it has no windows allocated.
>> + */
>> +static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p,
>> struct pack_window **mru_w, int *accept_windows_inuse)
>> +{
>> + struct pack_window *w, *this_mru_w;
>> + int has_windows_inuse = 0;
>> +
>> + /*
>> + * Reject this pack if it has windows and the previously selected
>> + * one does not. If this pack does not have windows, reject
>> + * it if the pack file is newer than the previously selected one.
>> + */
>> + if (*lru_p && !*mru_w && (p->windows || p->mtime > (*lru_p)->mtime))
>> + return;
>> +
>> + for (w = this_mru_w = p->windows; w; w = w->next) {
>> + /*
>> + * Reject this pack if any of its windows are in use,
>> + * but the previously selected pack did not have any
>> + * inuse windows. Otherwise, record that this pack
>> + * has windows in use.
>> + */
>> + if (w->inuse_cnt) {
>> + if (*accept_windows_inuse)
>> + has_windows_inuse = 1;
>> + else
>> + return;
>> + }
>> +
>> + if (w->last_used > this_mru_w->last_used)
>> + this_mru_w = w;
>> +
>> + /*
>> + * Reject this pack if it has windows that have been
>> + * used more recently than the previously selected pack.
>> + * If the previously selected pack had windows inuse and
>> + * we have not encountered a window in this pack that is
>> + * inuse, skip this check since we prefer a pack with no
>> + * inuse windows to one that has inuse windows.
>> + */
>> + if (*mru_w && *accept_windows_inuse == has_windows_inuse &&
>> + this_mru_w->last_used > (*mru_w)->last_used)
>> + return;
>
> The "*accept_windows_inuse == has_windows_inuse" part is hard to
> grok, together with the fact that this statement is evaluated for
> each and every "w", even though it is about this_mru_w and that
> variable is not updated in every iteration of the loop. Can you
> clarify/simplify this part of the code a bit more?
>
> For example, would the above be equivalent to this?
>
> if (w->last_used < this_mru_w->last_used)
> continue;
>
> this_mru_w = w;
> if (has_windows_inuse && *mru_w &&
> w->last_used > (*mru_w)->last_used)
> return;
>
> That is, if we already know a more recently used window in this
> pack, we do not have to do anything to maintain mru_w. Otherwise,
> remember that this window is the most recently used one in this
> pack, and if it is newer than the newest one from the pack we are
> going to pick, we refrain from picking this pack.
>
> But we do not reject ourselves if we haven't seen a window that is
> in use (yet).
No that wouldn't be the same. The function of "*accept_windows_inuse
== has_windows_inuse" and the testing of this_mru_w in every loop
rather than w, is too subtle. I tried to draw attention to it in the
comment, but I agree it's not enough.
The case that your example would not catch is when the new pack's mru
window has already been found, but has_windows_inuse is not set until
later. When has_windows_inuse is later set, we need to test
this_mru_w regardless of whether we have just assigned it.
For example, if mru_w points to a pack with last_used == 11 and
*accept_windows_inuse = 1, and p->windows looks like this:
last_used in_use
12 0
10 1
Then the first time through the loop, this_mru_w would be set to the
first window with last_used equal to 12. The if statement that tests
"this_mru_w->last_used > (*mru_w)->last_used" would be skipped since
has_windows_inuse would still be 0. The second time through the loop,
this_mru_w would _not_ be reset, but has_windows_inuse _would_ be set.
This time, we would want to enter the last for loop so that we can
reject the pack.
I'll try to rework this loop or add comments to clarify.
-Brandon
>> + }
>> +
>> + /*
>> + * Select this pack.
>> + */
>> + *mru_w = this_mru_w;
>> + *lru_p = p;
>> + *accept_windows_inuse = has_windows_inuse;
>> +}
>> +
>> +static int close_one_pack(void)
>> +{
>> + struct packed_git *p, *lru_p = NULL;
>> + struct pack_window *mru_w = NULL;
>> + int accept_windows_inuse = 1;
>> +
>> + for (p = packed_git; p; p = p->next) {
>> + if (p->pack_fd == -1)
>> + continue;
>> + find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
>>
Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
Jeff King writes: > On Fri, Aug 02, 2013 at 03:54:02AM -0700, Jeff King wrote: > >> We need to revert that commit before the release. It can either be >> replaced with: >> >> 1. A "--split" (or similar) option to use the behavior only when >> desired. >> >> 2. Enabling splitting only when %(rest) is used in the output format. > > Of the two, I think the latter is more sensible; the former is > unnecessarily placing the burden on the user to match "--split" with > their use of "%(rest)". The second is pointless without the first. > > A patch to implement (2) is below. As I'd queue this on top of the revert, I had to wrangle it a bit to make it relative, i.e. "this resurrects what the other reverted patch did but in a weaker/safer form". This will be kept outside this cycle. Thanks for a quick fix. -- To unsubscribe from this list: send the line "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: Rewriting git-repack.sh in C
On Fri, Aug 02, 2013 at 09:10:59PM +0700, Duy Nguyen wrote:
> On Fri, Aug 2, 2013 at 8:48 PM, Stefan Beller
> wrote:
> > Hello,
> >
> > I'd like to rewrite the repack shell script in C.
> > So I tried the naive approach reading the man page and
> > the script itself and write C program by matching each block/line
> > of the script with a function in C
> >
> > ...
> >
> > So my question is, how you'd generally approach rewriting a
> > shell script in C.
>
> Start a new process via start_command/run_command interface. It's
> safer to retain the process boundary at this stage. You can try to
> integrate further later.
I was in the middle of something and somehow read this as "rewriting
git-rebase.sh" :-X
For git-repack, because it ends with a single pack-objects call, we
might not need a new process after all (very much like tail call
optimization). This is what I got for some time, but still not polish
it for submission (and it may be a bit broken after the last
rebase). Maybe you can work off this, or from scratch if you think
it's too messy. It basically teaches pack-objects extra features that
repack needs, then make repack a wrapper of pack-objects.
-- 8< --
commit 25569a3958c3272b3eb5fa50dea680948f7a2768 (build-in-repack)
Author: Nguyễn Thái Ngọc Duy
Date: Wed Nov 9 19:21:39 2011 +0700
Build in git-repack
pack-objects learns a few more options to take over what's been done
by git-repack.sh. cmd_repack() becomes a wrapper around
cmd_pack_objects().
diff --git a/Makefile b/Makefile
index 0f931a2..b4010a6 100644
--- a/Makefile
+++ b/Makefile
@@ -460,7 +460,6 @@ SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
-SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
SCRIPT_SH += git-stash.sh
SCRIPT_SH += git-submodule.sh
@@ -584,6 +583,7 @@ BUILT_INS += git-init$X
BUILT_INS += git-merge-subtree$X
BUILT_INS += git-peek-remote$X
BUILT_INS += git-repo-config$X
+BUILT_INS += git-repack$X
BUILT_INS += git-show$X
BUILT_INS += git-stage$X
BUILT_INS += git-status$X
diff --git a/builtin.h b/builtin.h
index 64bab6b..feb958f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -117,6 +117,7 @@ extern int cmd_reflog(int argc, const char **argv, const
char *prefix);
extern int cmd_remote(int argc, const char **argv, const char *prefix);
extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);
extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);
+extern int cmd_repack(int argc, const char **argv, const char *prefix);
extern int cmd_repo_config(int argc, const char **argv, const char *prefix);
extern int cmd_rerere(int argc, const char **argv, const char *prefix);
extern int cmd_reset(int argc, const char **argv, const char *prefix);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f069462..1742ea1 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -18,10 +18,17 @@
#include "refs.h"
#include "streaming.h"
#include "thread-utils.h"
+#include "sigchain.h"
static const char *pack_usage[] = {
N_("git pack-objects --stdout [options...] [< ref-list | <
object-list]"),
N_("git pack-objects [options...] base-name [< ref-list | <
object-list]"),
+ N_("git pack-objects --repack [options...]"),
+ NULL
+};
+
+static char const * const repack_usage[] = {
+ N_("git repack [options]"),
NULL
};
@@ -103,6 +110,15 @@ static struct object_entry *locate_object_entry(const
unsigned char *sha1);
static uint32_t written, written_delta;
static uint32_t reused, reused_delta;
+#define REPACK_IN_PROGRESS (1 << 0)
+#define REPACK_UPDATE_INFO (1 << 1)
+#define REPACK_ALL_INTO_ONE(1 << 2)
+#define REPACK_REMOVE_REDUNDANT(1 << 3)
+
+static int repack_flags, nr_written_packs;
+static int repack_usedeltabaseoffset;
+static struct string_list written_packs;
+static struct string_list backup_files;
static void *get_delta(struct object_entry *entry)
{
@@ -792,9 +808,19 @@ static void write_pack_file(void)
snprintf(tmpname, sizeof(tmpname), "%s-", base_name);
finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
- &pack_idx_opts, sha1);
+ &pack_idx_opts, sha1,
+ repack_flags & REPACK_IN_PROGRESS ?
+ &backup_files : NULL);
free(pack_tmp_name);
- puts(sha1_to_hex(sha1));
+ if (repack_flags & REPACK_IN_PROGRESS) {
+ int len = strlen(tmpname);
+ char *s = xmalloc(len + 2);
+ memcpy(s, tmpname, len - 4);
+ memcpy(s
Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
Jeff King writes:
> We need to revert that commit before the release. It can either be
> replaced with:
>
> 1. A "--split" (or similar) option to use the behavior only when
> desired.
>
> 2. Enabling splitting only when %(rest) is used in the output format.
>
> And I suppose it is too late in the cycle for either of those to go into
> v1.8.4. That's a shame, but I think losing that particular patch does
> not affect the rest of the series, so we are OK to ship without it.
>
> Thanks Joey for a timely bug report.
Thanks. Will do this to jk/cat-file-batch-optim topic and merge it
to 'master' for now.
-- >8 --
Subject: [PATCH] Revert "cat-file: split --batch input lines on whitespace"
This reverts commit c334b87b30c1464a1ab563fe1fb8de5eaf0e5bac; the
update assumed that people only used the command to read from
"rev-list --objects" output, whose lines begin with a 40-hex object
name followed by a whitespace, but it turns out that scripts feed
random extended SHA-1 expressions (e.g. "HEAD:$pathname") in which
a whitespace has to be kept.
---
Documentation/git-cat-file.txt | 10 ++
builtin/cat-file.c | 20 +---
t/t1006-cat-file.sh| 7 ---
3 files changed, 3 insertions(+), 34 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 3ddec0b..10fbc6a 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -88,10 +88,8 @@ BATCH OUTPUT
If `--batch` or `--batch-check` is given, `cat-file` will read objects
from stdin, one per line, and print information about them.
-Each line is split at the first whitespace boundary. All characters
-before that whitespace are considered as a whole object name, and are
-parsed as if given to linkgit:git-rev-parse[1]. Characters after that
-whitespace can be accessed using the `%(rest)` atom (see below).
+Each line is considered as a whole object name, and is parsed as if
+given to linkgit:git-rev-parse[1].
You can specify the information shown for each object by using a custom
``. The `` is copied literally to stdout for each
@@ -112,10 +110,6 @@ newline. The available atoms are:
The size, in bytes, that the object takes up on disk. See the
note about on-disk sizes in the `CAVEATS` section below.
-`rest`::
- The text (if any) found after the first run of whitespace on the
- input line (i.e., the "rest" of the line).
-
If no format is specified, the default format is `%(objectname)
%(objecttype) %(objectsize)`.
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 163ce6c..4253460 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -119,7 +119,6 @@ struct expand_data {
enum object_type type;
unsigned long size;
unsigned long disk_size;
- const char *rest;
/*
* If mark_query is true, we do not expand anything, but rather
@@ -164,9 +163,6 @@ static void expand_atom(struct strbuf *sb, const char
*atom, int len,
data->info.disk_sizep = &data->disk_size;
else
strbuf_addf(sb, "%lu", data->disk_size);
- } else if (is_atom("rest", atom, len)) {
- if (!data->mark_query && data->rest)
- strbuf_addstr(sb, data->rest);
} else
die("unknown format element: %.*s", len, atom);
}
@@ -277,21 +273,7 @@ static int batch_objects(struct batch_options *opt)
warn_on_object_refname_ambiguity = 0;
while (strbuf_getline(&buf, stdin, '\n') != EOF) {
- char *p;
- int error;
-
- /*
-* Split at first whitespace, tying off the beginning of the
-* string and saving the remainder (or NULL) in data.rest.
-*/
- p = strpbrk(buf.buf, " \t");
- if (p) {
- while (*p && strchr(" \t", *p))
- *p++ = '\0';
- }
- data.rest = p;
-
- error = batch_one_object(buf.buf, opt, &data);
+ int error = batch_one_object(buf.buf, opt, &data);
if (error)
return error;
}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index d499d02..4e911fb 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -78,13 +78,6 @@ $content"
echo $sha1 | git cat-file --batch-check="%(objecttype) %(objectname)"
>actual &&
test_cmp expect actual
'
-
-test_expect_success '--batch-check with %(rest)' '
- echo "$type this is some extra content" >expect &&
- echo "$sha1this is some extra content" |
- git cat-file --batch-check="%(objecttype) %(rest)" >actual &&
- test_cmp expect actual
-'
}
hello_content="Hello World"
--
1.8.4-rc1-125-g7a0ec02
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
Re: [PATCH v3] sha1_file: introduce close_one_pack() to close packs on fd pressure
Brandon Casey writes:
> +/*
> + * The LRU pack is the one with the oldest MRU window, preferring packs
> + * with no used windows, or the oldest mtime if it has no windows allocated.
> + */
> +static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p,
> struct pack_window **mru_w, int *accept_windows_inuse)
> +{
> + struct pack_window *w, *this_mru_w;
> + int has_windows_inuse = 0;
> +
> + /*
> + * Reject this pack if it has windows and the previously selected
> + * one does not. If this pack does not have windows, reject
> + * it if the pack file is newer than the previously selected one.
> + */
> + if (*lru_p && !*mru_w && (p->windows || p->mtime > (*lru_p)->mtime))
> + return;
> +
> + for (w = this_mru_w = p->windows; w; w = w->next) {
> + /*
> + * Reject this pack if any of its windows are in use,
> + * but the previously selected pack did not have any
> + * inuse windows. Otherwise, record that this pack
> + * has windows in use.
> + */
> + if (w->inuse_cnt) {
> + if (*accept_windows_inuse)
> + has_windows_inuse = 1;
> + else
> + return;
> + }
> +
> + if (w->last_used > this_mru_w->last_used)
> + this_mru_w = w;
> +
> + /*
> + * Reject this pack if it has windows that have been
> + * used more recently than the previously selected pack.
> + * If the previously selected pack had windows inuse and
> + * we have not encountered a window in this pack that is
> + * inuse, skip this check since we prefer a pack with no
> + * inuse windows to one that has inuse windows.
> + */
> + if (*mru_w && *accept_windows_inuse == has_windows_inuse &&
> + this_mru_w->last_used > (*mru_w)->last_used)
> + return;
The "*accept_windows_inuse == has_windows_inuse" part is hard to
grok, together with the fact that this statement is evaluated for
each and every "w", even though it is about this_mru_w and that
variable is not updated in every iteration of the loop. Can you
clarify/simplify this part of the code a bit more?
For example, would the above be equivalent to this?
if (w->last_used < this_mru_w->last_used)
continue;
this_mru_w = w;
if (has_windows_inuse && *mru_w &&
w->last_used > (*mru_w)->last_used)
return;
That is, if we already know a more recently used window in this
pack, we do not have to do anything to maintain mru_w. Otherwise,
remember that this window is the most recently used one in this
pack, and if it is newer than the newest one from the pack we are
going to pick, we refrain from picking this pack.
But we do not reject ourselves if we haven't seen a window that is
in use (yet).
> + }
> +
> + /*
> + * Select this pack.
> + */
> + *mru_w = this_mru_w;
> + *lru_p = p;
> + *accept_windows_inuse = has_windows_inuse;
> +}
> +
> +static int close_one_pack(void)
> +{
> + struct packed_git *p, *lru_p = NULL;
> + struct pack_window *mru_w = NULL;
> + int accept_windows_inuse = 1;
> +
> + for (p = packed_git; p; p = p->next) {
> + if (p->pack_fd == -1)
> + continue;
> + find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
> + }
> +
> + if (lru_p) {
> + close(lru_p->pack_fd);
> + pack_open_fds--;
> + lru_p->pack_fd = -1;
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> void unuse_pack(struct pack_window **w_cursor)
> {
> struct pack_window *w = *w_cursor;
> @@ -768,7 +845,7 @@ static int open_packed_git_1(struct packed_git *p)
> pack_max_fds = 1;
> }
>
> - while (pack_max_fds <= pack_open_fds && unuse_one_window(NULL, -1))
> + while (pack_max_fds <= pack_open_fds && close_one_pack())
> ; /* nothing */
>
> p->pack_fd = git_open_noatime(p->pack_name);
--
To unsubscribe from this list: send the line "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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
On Fri, Aug 2, 2013 at 8:27 AM, Joey Hess wrote:
> Jeff King wrote:
>> By the way, Joey, I am not sure how safe "git cat-file --batch-check" is
>> for arbitrary filenames. In particular, I don't know how it would react
>> to a filename with an embedded newline (and I do not think it will undo
>> quoting). Certainly that does not excuse this regression; even if what
>> you are doing is not 100% reliable, it is good enough in sane situations
>> and we should not be breaking it. But you may want to double-check the
>> behavior of your scripts in such a case, and we may need to add a "-z"
>> to support it reliably.
>
> Yes, I would prefer to have a -z mode. I think my code otherwise handles
> newlines.
>
> Thanks for the quick fix. I agree that only enabling the behavior with
> %{rest} makes sense.
>
> --
> see shy jo
/methinks we've identified a gap in our test coverage. Care to add a
test that covers the functionality that git-annex depends on?
-Brandon
--
To unsubscribe from this list: send the line "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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
Jeff King wrote:
> By the way, Joey, I am not sure how safe "git cat-file --batch-check" is
> for arbitrary filenames. In particular, I don't know how it would react
> to a filename with an embedded newline (and I do not think it will undo
> quoting). Certainly that does not excuse this regression; even if what
> you are doing is not 100% reliable, it is good enough in sane situations
> and we should not be breaking it. But you may want to double-check the
> behavior of your scripts in such a case, and we may need to add a "-z"
> to support it reliably.
Yes, I would prefer to have a -z mode. I think my code otherwise handles
newlines.
Thanks for the quick fix. I agree that only enabling the behavior with
%{rest} makes sense.
--
see shy jo
signature.asc
Description: Digital signature
Re: [PATCH v2] Provide some linguistic guidance for the documentation.
On 13-08-02 02:25 AM, Jonathan Nieder wrote: Junio C Hamano wrote: Is that accurate? My impression has been: The documentation liberally mixes US and UK English (en_US/UK) norms for spelling and grammar, which is somewhat unfortunate. In an ideal world, it would have been better if it consistently used only one and not the other, and we would have picked en_US. I'm not convinced that would be better, even in an ideal world. It's certainly useful to have a consistent spelling of each term to make searching with "grep" easier. But searches with "grep" do not work well with line breaks anyway, and search engines for larger collections of documents seem to know about the usual spelling variants (along with knowing about stemming, etc). Unless we are planning to provide a separate en_GB translation, it seems unfortunate to consistently have everything spelled in the natural way for one group of people and in an unnatural way for another, just in the name of having a convention. Personally I find it distracting when the norms are mixed. I don't think the current mishmash pleases anyone (as evidenced by the steady stream of patches that change spellings). I am not sure it makes sense for the documentation to say "A huge disruptive patch of such-and-such specific kind of no immediate benefit is unwelcome". Isn't there some more general principle that implies that? Or the CodingGuidelines could simply say The documentation uses a mixture of U.S. and British English. I'm hoping this patch will help the list avoid seeing patches that merely flip between alternate spellings. (Perhaps the commit message should state this?) I think it's important to be clear about the kind of work the git community wants to see. So I don't think that single sentence by itself would be helpful. In the case of alternate spellings in the documentation, I think there's a temptation to create a blanket patch that "fixes" lots of perceived misspellings since such changes are mere typographic tweaks. It's a bit easy to overlook the disruptive nature of such a patch, so IMO it bears repeating here. Are you suggesting maybe that there should be no "official" dialect? That the guidance should simply say that we don't want to see patches that merely flip between alternate spellings (because we don't really care)? M. -- To unsubscribe from this list: send the line "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: Rewriting git-repack.sh in C
On Fri, Aug 2, 2013 at 8:48 PM, Stefan Beller wrote: > Hello, > > I'd like to rewrite the repack shell script in C. > So I tried the naive approach reading the man page and > the script itself and write C program by matching each block/line > of the script with a function in C > > Now I stumble upon other git commands (git pack-objects). > What's the best way to approach such a plumbing command? > > I don't think just calling cmd_pack_objects(argc, **argv) would > be the right thing to do, as we're not using all the command > line parameters, so some of the logic in cmd_pack_object could > be skipped. > Another approach would be to use some of the functions as used > by cmd_pack_objects, but these mostly reside in builtin/pack_objects.c > They'd need to be moved up to pack.h/pack.c. > > So my question is, how you'd generally approach rewriting a > shell script in C. Start a new process via start_command/run_command interface. It's safer to retain the process boundary at this stage. You can try to integrate further later. -- 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
Rewriting git-repack.sh in C
Hello, I'd like to rewrite the repack shell script in C. So I tried the naive approach reading the man page and the script itself and write C program by matching each block/line of the script with a function in C Now I stumble upon other git commands (git pack-objects). What's the best way to approach such a plumbing command? I don't think just calling cmd_pack_objects(argc, **argv) would be the right thing to do, as we're not using all the command line parameters, so some of the logic in cmd_pack_object could be skipped. Another approach would be to use some of the functions as used by cmd_pack_objects, but these mostly reside in builtin/pack_objects.c They'd need to be moved up to pack.h/pack.c. So my question is, how you'd generally approach rewriting a shell script in C. Stefan signature.asc Description: OpenPGP digital signature
Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
On Fri, Aug 02, 2013 at 03:54:02AM -0700, Jeff King wrote:
> We need to revert that commit before the release. It can either be
> replaced with:
>
> 1. A "--split" (or similar) option to use the behavior only when
> desired.
>
> 2. Enabling splitting only when %(rest) is used in the output format.
Of the two, I think the latter is more sensible; the former is
unnecessarily placing the burden on the user to match "--split" with
their use of "%(rest)". The second is pointless without the first.
A patch to implement (2) is below.
By the way, Joey, I am not sure how safe "git cat-file --batch-check" is
for arbitrary filenames. In particular, I don't know how it would react
to a filename with an embedded newline (and I do not think it will undo
quoting). Certainly that does not excuse this regression; even if what
you are doing is not 100% reliable, it is good enough in sane situations
and we should not be breaking it. But you may want to double-check the
behavior of your scripts in such a case, and we may need to add a "-z"
to support it reliably.
The "rev-list --objects" output may contain such paths, of course, but
they will be quoted, and "%(rest)" does not care (it is not trying to
interpret the paths, but will reliably relay the quoted bits to the
output).
-- >8 --
Subject: [PATCH] cat-file: only split on whitespace when %(rest) is used
Commit c334b87 recently taught `cat-file --batch-check` to
split input lines on whitespace, and stash everything after
the first token into the %(rest) output format element. That
commit claims:
Object names cannot contain spaces, so any input with
spaces would have resulted in a "missing" line.
But that is not correct. Refs, object sha1s, and various
peeling suffixes cannot contain spaces, but some object
names can. In particular:
1. Tree paths like "[]:path with whitespace"
2. Reflog specifications like "@{2 days ago}"
3. Commit searches like "rev^{/grep me}" or ":/grep me"
To remain backwards compatible, we cannot split on
whitespace by default. This patch teaches cat-file to only
do the splitting when "%(rest)" is used by the output
format. Since that element did not exist at all until
c334b87, old scripts cannot be affected.
The existence of object names with spaces does mean that you
cannot reliably do:
echo ":path with space and other data" |
git cat-file --batch-check="%(objectname) %(rest)"
as it would split the path and feed only ":path" to
get_sha1. But that command is nonsensical. If you wanted to
see "and other data" in "%(rest)", git cannot possibly know
where the filename ends and the "rest" begins.
It might be more robust to have something like "-z" to
separate the input elements. But this patch is still a
reasonable step before having that. It makes the easy cases
easy; people who do not care about %(rest) do not have to
consider it, and the %(rest) code handles the spaces and
newlines of "rev-list --objects" correctly.
Hard cases remain hard but possible (if you might get
whitespace in your input, you do not get to use %(rest) and
must split and join the output yourself using more flexible
tools). And most importantly, it does not preclude us from
having different splitting rules later if a "-z" (or
similar) option is added. So we can make the hard
cases easier later, if we choose to.
Signed-off-by: Jeff King
---
Documentation/git-cat-file.txt | 16
builtin/cat-file.c | 31 +--
t/t1006-cat-file.sh| 8
3 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 3ddec0b..21cffe2 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -86,12 +86,9 @@ If `--batch` or `--batch-check` is given, `cat-file` will
read objects
If `--batch` or `--batch-check` is given, `cat-file` will read objects
-from stdin, one per line, and print information about them.
-
-Each line is split at the first whitespace boundary. All characters
-before that whitespace are considered as a whole object name, and are
-parsed as if given to linkgit:git-rev-parse[1]. Characters after that
-whitespace can be accessed using the `%(rest)` atom (see below).
+from stdin, one per line, and print information about them. By default,
+the whole line is considered as an object, as if it were fed to
+linkgit:git-rev-parse[1].
You can specify the information shown for each object by using a custom
``. The `` is copied literally to stdout for each
@@ -113,8 +110,11 @@ newline. The available atoms are:
note about on-disk sizes in the `CAVEATS` section below.
`rest`::
- The text (if any) found after the first run of whitespace on the
- input line (i.e., the "rest" of the line).
+ If this atom is used in the output string, input lines are split
+ at the first whitespace boundary. All characters before that
+ whites
Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces
On Thu, Aug 01, 2013 at 11:40:03PM -0700, Jonathan Nieder wrote: > > Commit c334b87b30c1464a1ab563fe1fb8de5eaf0e5bac caused a reversion in > > git-cat-file --batch. > > > > With an older version: > > > > joey@gnu:~/tmp/rrr>git cat-file --batch > > :file name > > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 blob 0 > > > > With the new version: > > > > joey@wren:~/tmp/r>git cat-file --batch > > :file name > > :file missing > [...] > Oh dear. Luckily you caught this before the final 1.8.4 release. I > wonder if we should just revert c334b87b (cat-file: split --batch > input lines on whitespace, 2013-07-11) for now. Ugh. Yeah, the incorrect assumption from the commit message of c334b87b is "Object names cannot contain spaces...". Refs cannot, but filename specifiers after a colon can. We need to revert that commit before the release. It can either be replaced with: 1. A "--split" (or similar) option to use the behavior only when desired. 2. Enabling splitting only when %(rest) is used in the output format. And I suppose it is too late in the cycle for either of those to go into v1.8.4. That's a shame, but I think losing that particular patch does not affect the rest of the series, so we are OK to ship without it. Thanks Joey for a timely bug report. -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 04/13] contrib/subtree: Fix Synopsis
"David A. Greene" writes: > diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh > index 771f39d..8a23f58 100755 > --- a/contrib/subtree/git-subtree.sh > +++ b/contrib/subtree/git-subtree.sh > @@ -9,6 +9,7 @@ if [ $# -eq 0 ]; then > fi > OPTS_SPEC="\ > git subtree add --prefix= > +git subtree add --prefix= > git subtree merge --prefix= > git subtree pull --prefix= > git subtree push --prefix= > diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt > index c5bce41..7ba853e 100644 > --- a/contrib/subtree/git-subtree.txt > +++ b/contrib/subtree/git-subtree.txt > @@ -9,7 +9,8 @@ git-subtree - Merge subtrees together and split repository > into subtrees > SYNOPSIS > > [verse] > -'git subtree' add -P > +'git subtree' add -P > +'git subtree' add -P or ? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "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/13] contrib/subtree: Better Error Handling for add
"David A. Greene" writes:
> elif [ $# -eq 2 ]; then
> - "cmd_add_repository" "$@"
> + git rev-parse -q --verify "$2^{commit}" >/dev/null ||
> + die "'$2' does not refer to a commit"
That doesn't make any sense. $2 is a commit in a remote repository
which hasn't been fetched yet, so it can never be verified against the
local repository.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
