[PATCH v4 0/5] git-remote-mediawiki: new tool to preview local changes without pushing
From: Benoit Person benoit.per...@ensimag.fr
The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
preview content without pushing would be a nice thing to have.
changes from v3:
- Rewrite all commit messages.
- No more \ No newline at end of file.
- Rename GitMediawiki.pm into Git::Mediawiki.pm (so it moves GitMedawiki.pm
into Git/Mediawiki.pm).
- Remove from the Makefile the copy_pm target (see below 'Add a bin-wrapper').
- Use of 'install' insted of 'cp' in the Makefile.
- Comment on the install_pm target in the Makefile.
- Add a bin-wrapper for git to test scripts without 'make install'-ing them.
- Move verbose option handling from previous v3-4/4 (introduction of preview
tool) into v4-4/5 (introduction of git-mw).
- Refactor some code into subroutines to clean the global 'preview'
subroutine.
- Rewrite some error messages to make them more concise while still giving
the same amount of information.
- Use 'remote.${remote_name}.mwIDcontent' instead of 'mediawiki.IDContent'
as config item for the lookup ID used to combine template + new content.
- Remove comments about what's going on in the preview subroutine.
- Use 'clean_filename' (and not 'smudge_filename') in the preview tool to find
the correct mediawiki page name based on a filename.
- Remove space/tab mixup in the 'help' subroutine.
changes from v2:
- Add a way to test, without installation, code that uses GitMediawiki.pm.
- Move more constants to GitMediawiki.pm
- Remove the encapsulation of Git::config calls into a git_cmd_try one.
- Remove the --blob option, distinction between files and blobs is now
automatic.
- Add a --verbose option to output more information on what's going on.
- Rewrote the doc and the commit message.
- Rewrote of the template retrieving code (see 'get_template' sub).
- Use a configuration variable to define the content ID search in the
template. Default value set as 'bodyContent' since it seems more standard
than 'mw-content-text'.
- Final content is now saved as utf-8 to solve encoding issues.
- Perlcritic changes:
- Update for loops style to a more perlish one.
- All 'print's specify their output streams.
-- Same useless warnings left in git-remote-mediawiki.perl after
célestin's
work and git-mw.perl after this patch :) .
changes from v1:
- add new package GitMediawiki
- move some of git-remote-mediawiki functions into the package
- update git-remote-mediawiki to use those moved functions
- add a hacky-way to install it in the Makefile
- use it in the new git mw tool
- add a way to give to the preview tool blobs as argument
- add a fallback when the upstream's branch remote is not a mediawiki remote
- update the `autoload` option to use `git web--browse` and not `xdg-open`
- update the way we find the upstream's branch remote name
This serie is based on the 'master' branch merged with célestin's patch series.
[1] https://github.com/moy/Git-Mediawiki/issues/7
Benoit Person (5):
git-remote-mediawiki: Introduction of Git::Mediawiki.pm
git-remote-mediawiki: new git bin-wrapper for developement
git-remote-mediawiki: factoring code between git-remote-mediawiki and
Git::Mediawiki
git-remote-mediawiki: Adding git-mw command
git-remote-mediawiki: Add preview subcommand into git mw.
contrib/mw-to-git/Git/Mediawiki.pm | 100
contrib/mw-to-git/Makefile | 24 +-
contrib/mw-to-git/git | 25 ++
contrib/mw-to-git/git-mw.perl | 359
contrib/mw-to-git/git-remote-mediawiki.perl | 85 +--
5 files changed, 518 insertions(+), 75 deletions(-)
create mode 100644 contrib/mw-to-git/Git/Mediawiki.pm
create mode 100755 contrib/mw-to-git/git
create mode 100644 contrib/mw-to-git/git-mw.perl
--
1.8.3.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/5] git-remote-mediawiki: new git bin-wrapper for developement
From: Benoit Person benoit.per...@ensimag.fr
The introduction of the Git::Mediawiki package makes it impossible to test,
without installation, git-remote-mediawiki and git-mw.
Using a git bin-wrapper enables us to define proper $GITPERLLIB to force the
use of the developement version of the Git::Mediawiki package, bypassing its
installed version if any.
An alternate solution was to 'install' all the files required at each build
but it pollutes the toplevel with untracked files.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
contrib/mw-to-git/git | 25 +
1 file changed, 25 insertions(+)
create mode 100755 contrib/mw-to-git/git
diff --git a/contrib/mw-to-git/git b/contrib/mw-to-git/git
new file mode 100755
index 000..25b60ad
--- /dev/null
+++ b/contrib/mw-to-git/git
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+# git executable wrapper script for Git-Mediawiki to run tests without
+# installing all the scripts and perl packages.
+
+# based on $GIT_ROOT_DIR/wrap-for-bin.sh
+
+
+GIT_ROOT_DIR=../..
+GIT_EXEC_PATH=$(cd $(dirname $0) cd ${GIT_ROOT_DIR} pwd)
+
+if test -n $NO_SET_GIT_TEMPLATE_DIR
+then
+ unset GIT_TEMPLATE_DIR
+else
+ GIT_TEMPLATE_DIR=$GIT_EXEC_PATH'/templates/blt'
+ export GIT_TEMPLATE_DIR
+fi
+# Hack to make the `use lib` call works with multiple paths
+GITPERLLIB=$GIT_EXEC_PATH'/contrib/mw-to-git:'$GIT_EXEC_PATH'/perl/blib/lib'
+GIT_TEXTDOMAINDIR=$GIT_EXEC_PATH'/po/build/locale'
+PATH=$GIT_EXEC_PATH'/contrib/mw-to-git:'$GIT_EXEC_PATH'/bin-wrappers:'$PATH
+export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
+
+exec ${GIT_EXEC_PATH}/git $@
\ No newline at end of file
--
1.8.3.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/5] git-remote-mediawiki: Introduction of Git::Mediawiki.pm
From: Benoit Person benoit.per...@ensimag.fr
Currently, the mw-to-git project contains only a remote helper
(git-remote-mediawiki.perl). To inmprove the user experience while working
with mediawiki remotes, new tools, designed for such cases, should be
created. To achieve this goal, the project needs a way to share code between
several scripts (remote helper, commands, ... ).
A perl package offer the best way to handle such case:
Each script can select what should be imported in its namespace.
The package namespacing limits the use of side effects in the shared code.
An alternate solution is to concatenate a toolset file with each *.perl
when 'make'-ing the project. In that scheme, everything is imported in the
script's namespace. Plus, files should be renamed in order to chain to Git's
toplevel makefile.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
contrib/mw-to-git/Git/Mediawiki.pm | 24
contrib/mw-to-git/Makefile | 21 +++--
2 files changed, 43 insertions(+), 2 deletions(-)
create mode 100644 contrib/mw-to-git/Git/Mediawiki.pm
diff --git a/contrib/mw-to-git/Git/Mediawiki.pm
b/contrib/mw-to-git/Git/Mediawiki.pm
new file mode 100644
index 000..805f42a
--- /dev/null
+++ b/contrib/mw-to-git/Git/Mediawiki.pm
@@ -0,0 +1,24 @@
+package Git::Mediawiki;
+
+use 5.008;
+use strict;
+use Git;
+
+BEGIN {
+
+our ($VERSION, @ISA, @EXPORT, @EXPORT_OK);
+
+# Totally unstable API.
+$VERSION = '0.01';
+
+require Exporter;
+
+@ISA = qw(Exporter);
+
+@EXPORT = ();
+
+# Methods which can be called as standalone functions as well:
+@EXPORT_OK = ();
+}
+
+1; # Famous last words
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 1fb2424..becd322 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -4,16 +4,33 @@
#
## Build git-remote-mediawiki
+GIT_MEDIAWIKI_PM=Git/Mediawiki.pm
SCRIPT_PERL=git-remote-mediawiki.perl
GIT_ROOT_DIR=../..
HERE=contrib/mw-to-git/
SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \
+-s --no-print-directory instlibdir)
all: build
-build install clean:
+# Run 'make install' from Git's toplevel before using this
+install_pm:
+ install $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
+
+build:
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+build-perl-script
+
+install: install_pm
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
-$@-perl-script
+install-perl-script
+
+clean:
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+clean-perl-script
+ rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
+
perlcritic:
perlcritic -2 *.perl
--
1.8.3.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/5] git-remote-mediawiki: Add preview subcommand into git mw.
From: Benoit Person benoit.per...@ensimag.fr
In the current state, a user of git-remote-mediawiki can edit the markup text
locally, but has to push to the remote wiki to see how the page is rendererd.
Add a new 'git mw preview' command that allows rendering the markup text on
the remote wiki without actually pushing any change on the wiki.
This uses Mediawiki's API to render the markup and inserts it in an actual
HTML page from the wiki so that CSS can be rendered properly. Most links
should work when the page exists on the remote.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
contrib/mw-to-git/Git/Mediawiki.pm | 3 +-
contrib/mw-to-git/git-mw.perl | 301 -
2 files changed, 302 insertions(+), 2 deletions(-)
diff --git a/contrib/mw-to-git/Git/Mediawiki.pm
b/contrib/mw-to-git/Git/Mediawiki.pm
index 47fe4f4..d13c4df 100644
--- a/contrib/mw-to-git/Git/Mediawiki.pm
+++ b/contrib/mw-to-git/Git/Mediawiki.pm
@@ -19,7 +19,7 @@ require Exporter;
# Methods which can be called as standalone functions as well:
@EXPORT_OK = qw(clean_filename smudge_filename connect_maybe
- EMPTY HTTP_CODE_OK);
+ EMPTY HTTP_CODE_OK HTTP_CODE_PAGE_NOT_FOUND);
}
# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
@@ -30,6 +30,7 @@ use constant EMPTY = q{};
# HTTP codes
use constant HTTP_CODE_OK = 200;
+use constant HTTP_CODE_PAGE_NOT_FOUND = 404;
sub clean_filename {
my $filename = shift;
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
index 4a3e4a9..d4f412a 100644
--- a/contrib/mw-to-git/git-mw.perl
+++ b/contrib/mw-to-git/git-mw.perl
@@ -12,6 +12,14 @@ use strict;
use warnings;
use Getopt::Long;
+use URI::URL qw(url);
+use LWP::UserAgent;
+use HTML::TreeBuilder;
+
+use Git;
+use MediaWiki::API;
+use Git::Mediawiki qw(clean_filename connect_maybe
+ EMPTY HTTP_CODE_PAGE_NOT_FOUND);
# By default, use UTF-8 to communicate with Git and the user
binmode STDERR, ':encoding(UTF-8)';
@@ -26,9 +34,26 @@ sub v_print {
return;
}
+# Preview parameters
+my $file_name = EMPTY;
+my $remote_name = EMPTY;
+my $preview_file_name = EMPTY;
+my $autoload = 0;
+sub file {
+ $file_name = shift;
+ return $file_name;
+}
+
my %commands = (
'help' =
- [\help, {}, \help]
+ [\help, {}, \help],
+ 'preview' =
+ [\preview, {
+ '' = \file,
+ 'output|o=s' = \$preview_file_name,
+ 'remote|r=s' = \$remote_name,
+ 'autoload|a' = \$autoload
+ }, \preview_help]
);
# Search for sub-command
@@ -47,6 +72,279 @@ GetOptions( %{$cmd-[1]},
# Launch command
{$cmd-[0]};
+# Preview Functions
+
+# @TODO : add documentation for verbose option
+sub preview_help {
+ print {*STDOUT} 'END';
+USAGE: git mw preview [--remote|-r remote name] [--autoload|-a]
+ [--output|-o output filename] [--verbose|-v]
+ blob | filename
+
+DESCRIPTION:
+Preview is an utiliy to preview local content of a mediawiki repo as if it was
+pushed on the remote.
+
+For that, preview searches for the remote name of the current branch's upstream
+if --remote is not set. If that remote is not found or if it is not a
mediawiki,
+it lists all mediawiki remotes configured and asks you to replay your command
+with the --remote option set properly.
+
+Then, it searches for a file named 'filename'. If it's not found in the current
+dir, it will assume it's a blob.
+
+The content retrieved in the file (or in the blob) will then be parsed by the
+distant mediawiki and combined with a template retrieved from the mediawiki.
+
+Finally, preview will save the HTML result in a file. and autoload it in your
+default web browser if the option --autoload is present.
+
+OPTIONS:
+ -r remote name, --remote remote name
+ If the remote is a mediawiki, the template and the parse engine
used for
+ the preview will be those of that remote.
+ If not, a list of valid remotes will be shown.
+
+ -a, --autoload
+ Try to load the HTML output in a new tab (or new window) of
your default
+ web browser.
+
+ -o output filename, --output output filename
+ Change the HTML output filename. Default filename is based on
the input
+ filename with its extension replaced by '.html'.
+
+ -v, --verbose
+ Show more information on what's going on under the hood.
+END
+ exit;
+}
+
+sub preview {
+ my $wiki;
+ my ($remote_url, $wiki_page_name);
+ my ($new_content, $template);
+
[PATCH v4 3/5] git-remote-mediawiki: factoring code between git-remote-mediawiki and Git::Mediawiki
From: Benoit Person benoit.per...@ensimag.fr
For now, Git::Mediawiki contains nothing.
This first patch moves some of git-remote-mediawiki.perl's factorisable code
into Git::Mediawiki. In the same time, it removes the side effects of that code
and renames the fucntions and constants moved to expose a better API.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
contrib/mw-to-git/Git/Mediawiki.pm | 77 +-
contrib/mw-to-git/git-remote-mediawiki.perl | 85 +
2 files changed, 89 insertions(+), 73 deletions(-)
diff --git a/contrib/mw-to-git/Git/Mediawiki.pm
b/contrib/mw-to-git/Git/Mediawiki.pm
index 805f42a..47fe4f4 100644
--- a/contrib/mw-to-git/Git/Mediawiki.pm
+++ b/contrib/mw-to-git/Git/Mediawiki.pm
@@ -18,7 +18,82 @@ require Exporter;
@EXPORT = ();
# Methods which can be called as standalone functions as well:
-@EXPORT_OK = ();
+@EXPORT_OK = qw(clean_filename smudge_filename connect_maybe
+ EMPTY HTTP_CODE_OK);
+}
+
+# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
+use constant SLASH_REPLACEMENT = '%2F';
+
+# Used to test for empty strings
+use constant EMPTY = q{};
+
+# HTTP codes
+use constant HTTP_CODE_OK = 200;
+
+sub clean_filename {
+ my $filename = shift;
+ $filename =~ s{@{[SLASH_REPLACEMENT]}}{/}g;
+ # [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
+ # Do a variant of URL-encoding, i.e. looks like URL-encoding,
+ # but with _ added to prevent MediaWiki from thinking this is
+ # an actual special character.
+ $filename =~ s/[\[\]\{\}\|]/sprintf(_%%_%x, ord($))/ge;
+ # If we use the uri escape before
+ # we should unescape here, before anything
+
+ return $filename;
+}
+
+sub smudge_filename {
+ my $filename = shift;
+ $filename =~ s{/}{@{[SLASH_REPLACEMENT]}}g;
+ $filename =~ s/ /_/g;
+ # Decode forbidden characters encoded in clean_filename
+ $filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf('%c', hex($1))/ge;
+ return $filename;
+}
+
+sub connect_maybe {
+ my $wiki = shift;
+ if ($wiki) {
+ return $wiki;
+ }
+
+ my $remote_name = shift;
+ my $remote_url = shift;
+ my ($wiki_login, $wiki_password, $wiki_domain);
+
+ $wiki_login = Git::config(remote.${remote_name}.mwLogin);
+ $wiki_password = Git::config(remote.${remote_name}.mwPassword);
+ $wiki_domain = Git::config(remote.${remote_name}.mwDomain);
+
+ $wiki = MediaWiki::API-new;
+ $wiki-{config}-{api_url} = ${remote_url}/api.php;
+ if ($wiki_login) {
+ my %credential = (
+ 'url' = $remote_url,
+ 'username' = $wiki_login,
+ 'password' = $wiki_password
+ );
+ Git::credential(\%credential);
+ my $request = {lgname = $credential{username},
+ lgpassword = $credential{password},
+ lgdomain = $wiki_domain};
+ if ($wiki-login($request)) {
+ Git::credential(\%credential, 'approve');
+ print {*STDERR} qq(Logged in mediawiki user
$credential{username}.\n);
+ } else {
+ print {*STDERR} qq(Failed to log in mediawiki user
$credential{username} on ${remote_url}\n);
+ print {*STDERR} ' (error ' .
+ $wiki-{error}-{code} . ': ' .
+ $wiki-{error}-{details} . )\n;
+ Git::credential(\%credential, 'reject');
+ exit 1;
+ }
+ }
+
+ return $wiki;
}
1; # Famous last words
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 9ff45fd..a336887 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -14,6 +14,8 @@
use strict;
use MediaWiki::API;
use Git;
+use Git::Mediawiki qw(clean_filename smudge_filename connect_maybe
+ EMPTY HTTP_CODE_OK);
use DateTime::Format::ISO8601;
use warnings;
@@ -23,9 +25,6 @@ binmode STDOUT, ':encoding(UTF-8)';
use URI::Escape;
-# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
-use constant SLASH_REPLACEMENT = '%2F';
-
# It's not always possible to delete pages (may require some
# privileges). Deleted pages are replaced with this content.
use constant DELETED_CONTENT = [[Category:Deleted]]\n;
@@ -40,8 +39,6 @@ use constant NULL_SHA1 =
'';
# Used on Git's side to reflect empty edit messages on the wiki
use constant EMPTY_MESSAGE
[PATCH v4 4/5] git-remote-mediawiki: Adding git-mw command
From: Benoit Person benoit.per...@ensimag.fr
For now, git-remote-mediawiki is only a remote-helper. This patch adds a new
toolset script in which we will be able to build new tools for
git-remote-mediawiki.
This toolset uses a subcommand-mechanism to launch the proper action. For now
only the 'help' subcommand is implemented. It also provides some generic code
for the verbose and help command line options.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
contrib/mw-to-git/Makefile| 7 ++---
contrib/mw-to-git/git-mw.perl | 60 +++
2 files changed, 64 insertions(+), 3 deletions(-)
create mode 100644 contrib/mw-to-git/git-mw.perl
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index becd322..e2ae798 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -6,6 +6,7 @@
GIT_MEDIAWIKI_PM=Git/Mediawiki.pm
SCRIPT_PERL=git-remote-mediawiki.perl
+SCRIPT_PERL+=git-mw.perl
GIT_ROOT_DIR=../..
HERE=contrib/mw-to-git/
@@ -20,15 +21,15 @@ install_pm:
install $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
build:
- $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
build-perl-script
install: install_pm
- $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
install-perl-script
clean:
- $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
clean-perl-script
rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
new file mode 100644
index 000..4a3e4a9
--- /dev/null
+++ b/contrib/mw-to-git/git-mw.perl
@@ -0,0 +1,60 @@
+#!/usr/bin/perl
+
+# Copyright (C) 2013
+# Benoit Person benoit.per...@ensimag.imag.fr
+# Celestin Matte celestin.ma...@ensimag.imag.fr
+# License: GPL v2 or later
+
+# Set of tools for git repo with a mediawiki remote.
+# Documentation bugtracker: https://github.com/moy/Git-Mediawiki/
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+
+# By default, use UTF-8 to communicate with Git and the user
+binmode STDERR, ':encoding(UTF-8)';
+binmode STDOUT, ':encoding(UTF-8)';
+
+# Global parameters
+my $verbose = 0;
+sub v_print {
+ if ($verbose) {
+ return print {*STDERR} @_;
+ }
+ return;
+}
+
+my %commands = (
+ 'help' =
+ [\help, {}, \help]
+);
+
+# Search for sub-command
+my $cmd = $commands{'help'};
+for (0..@ARGV-1) {
+ if (defined $commands{$ARGV[$_]}) {
+ $cmd = $commands{$ARGV[$_]};
+ splice @ARGV, $_, 1;
+ last;
+ }
+};
+GetOptions( %{$cmd-[1]},
+ 'help|h' = \{$cmd-[2]},
+ 'verbose|v' = \$verbose);
+
+# Launch command
+{$cmd-[0]};
+
+## Help Functions
##
+
+sub help {
+ print {*STDOUT} 'END';
+usage: git mw command args
+
+git mw commands are:
+helpDisplay help information about git mw
+END
+ exit;
+}
--
1.8.3.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] config doc: rewrite push.default section
On Thu, Jun 20, 2013 at 5:27 AM, Junio C Hamano gits...@pobox.com wrote: +* `simple` - a safer version of `current`; push the current branch + to update a branch with the same name on the receiving end, with a + safety feature: in central workflows, error out if your current + branch is not set to integrate with the branch with the same name, + to ensure that a `push` and a `push` are symmetrical. to ensure that a `push` and a `pull` are symmetrical. Otherwise, this looks good to me. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
On 06/19/2013 09:18 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
Handle simple transactions for the packed-refs file at the
packed_ref_cache level via new functions lock_packed_refs(),
commit_packed_refs(), and rollback_packed_refs().
Only allow the packed ref cache to be modified (via add_packed_ref())
while the packed refs file is locked.
Change clone to add the new references within a transaction.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/clone.c | 7 -
refs.c | 83
++---
refs.h | 27 +--
3 files changed, 98 insertions(+), 19 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 66bff57..b0c000a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -489,17 +489,22 @@ static struct ref *wanted_peer_refs(const struct ref
*refs,
return local_refs;
}
+static struct lock_file packed_refs_lock;
+
static void write_remote_refs(const struct ref *local_refs)
{
const struct ref *r;
+lock_packed_refs(packed_refs_lock, LOCK_DIE_ON_ERROR);
+
for (r = local_refs; r; r = r-next) {
if (!r-peer_ref)
continue;
add_packed_ref(r-peer_ref-name, r-old_sha1);
}
-pack_refs(PACK_REFS_ALL);
+if (commit_packed_refs())
+die_errno(unable to overwrite old ref-pack file);
}
The calling convention used here looks somewhat strange. You allow
callers to specify which lock-file structure is used when locking,
but when you are done, commit_packed_refs() does not take any
parameter.
lock_packed_refs() make the singleton in-core packed-ref-cache be
aware of which lock it is under, so commit_packed_refs() does not
need to be told (the singleton already knows what lockfile is in
effect), so I am not saying the code is broken, though.
Does the caller need to even have an access to this lock_file
instance?
No it doesn't. My reasoning was as follows:
lock_file instances can never be freed, because they are added to a
linked list in the atexit handler but never removed. Therefore they
have to be allocated statically (or allocated dynamically then leaked).
[I just noticed that lock_ref_sha1_basic() leaks a struct lock_file
every time that it is called.]
I wanted to leave the way open to allow other packed refs caches to be
locked. But since all packed refs caches are allocated dynamically, the
lock_file instance cannot be part of struct packed_ref_cache. So I
thought that the easiest approach is to rely on the caller, who
presumably can know that only a finite number of locks are in use at
once, to supply a usable lock_file instance.
But currently only the main packed ref cache can be locked, so it would
be possible for lock_packed_refs() to use the static packlock instance
for locking. I will change it to do so.
If/when we add support for locking other packed ref caches, we can
change the API to use a caller-supplied lock_file. Or even better,
implement a way to remove a lock_file instance from the atexit list;
then lock_packed_refs() could use dynamically-allocated lock_file
instances without having to leak them.
v3 to come.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Junio C Hamano: But my understanding is that the reordering using printf() is the mechanism we suggest l10n folks to use when the order of parameters given to printf does not match the preferred word order in the message in their language. It's documented in the gettext manual, and seems to be used in the zh_CN.po to change the word order in quite a few places. -- \\// Peter - http://www.softwolves.pp.se/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Am 6/20/2013 9:56, schrieb Peter Krefting: Junio C Hamano: But my understanding is that the reordering using printf() is the mechanism we suggest l10n folks to use when the order of parameters given to printf does not match the preferred word order in the message in their language. It's documented in the gettext manual, and seems to be used in the zh_CN.po to change the word order in quite a few places. It is fine to use %n$ in translated strings as long as gettext is enabled only on systems that have a sufficiently capable printf and these formats are not used in the source code. But you can't have this string: Splitting a commit while rebasing branch '%2$s' on '%3$s'. neither in the template nor in the translation, because the numbers must begin at 1 (and must be used without gaps). -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] send-email: Fix documentation of --signed-off-by-cc option
From: Namhyung Kim namhyung@lge.com The man page says it'll control the Cc: lines being added also, but this is not true. Reported-by: Minchan Kim minc...@kernel.org Signed-off-by: Namhyung Kim namhyung@lge.com --- Documentation/git-send-email.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 40a9a9a..5694d98 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -238,7 +238,7 @@ Automating the value of 'sendemail.identity'. --[no-]signed-off-by-cc:: - If this is set, add emails found in Signed-off-by: or Cc: lines to the + If this is set, add emails found in Signed-off-by: lines to the cc list. Default is the value of 'sendemail.signedoffbycc' configuration value; if that is unspecified, default to --signed-off-by-cc. -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/5] git-remote-mediawiki: Introduction of Git::Mediawiki.pm
This serie has a conflict with cm/remote-mediawiki. We probably want to wait for other mediawiki patch series to hit master (they are all marked as will merge to master in Junio's last what's cooking) before merging this one. benoit.per...@ensimag.fr writes: From: Benoit Person benoit.per...@ensimag.fr Currently, the mw-to-git project contains only a remote helper (git-remote-mediawiki.perl). To inmprove the user experience while working s/inmprove/improve/ A perl package offer the best way to handle such case: s/offer/offers/ An alternate solution is to concatenate a toolset file with each *.perl when 'make'-ing the project. In that scheme, everything is imported in the script's namespace. Plus, files should be renamed in order to chain to Git's toplevel makefile. Perhaps say explicitely hence this solution is not acceptable or so. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/12] for_each_ref: load all loose refs before packed refs
From: Jeff King p...@peff.net
If we are iterating through the refs using for_each_ref (or
any of its sister functions), we can get into a race
condition with a simultaneous pack-refs --prune that looks
like this:
0. We have a large number of loose refs, and a few packed
refs. refs/heads/z/foo is loose, with no matching entry
in the packed-refs file.
1. Process A starts iterating through the refs. It loads
the packed-refs file from disk, then starts lazily
traversing through the loose ref directories.
2. Process B, running pack-refs --prune, writes out the
new packed-refs file. It then deletes the newly packed
refs, including refs/heads/z/foo.
3. Meanwhile, process A has finally gotten to
refs/heads/z (it traverses alphabetically). It
descends, but finds nothing there. It checks its
cached view of the packed-refs file, but it does not
mention anything in refs/heads/z/ at all (it predates
the new file written by B in step 2).
The traversal completes successfully without mentioning
refs/heads/z/foo at all (the name, of course, isn't
important; but the more refs you have and the farther down
the alphabetical list a ref is, the more likely it is to hit
the race). If refs/heads/z/foo did exist in the packed refs
file at state 0, we would see an entry for it, but it would
show whatever sha1 the ref had the last time it was packed
(which could be an arbitrarily long time ago).
This can be especially dangerous when process A is git
prune, as it means our set of reachable tips will be
incomplete, and we may erroneously prune objects reachable
from that tip (the same thing can happen if repack -ad is
used, as it simply drops unreachable objects that are
packed).
This patch solves it by loading all of the loose refs for
our traversal into our in-memory cache, and then refreshing
the packed-refs cache. Because a pack-refs writer will
always put the new packed-refs file into place before
starting the prune, we know that any loose refs we fail to
see will either truly be missing, or will have already been
put in the packed-refs file by the time we refresh.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 39 +++
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 60f2507..787cc1c 100644
--- a/refs.c
+++ b/refs.c
@@ -750,6 +750,21 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
}
/*
+ * Load all of the refs from the dir into our in-memory cache. The hard work
+ * of loading loose refs is done by get_ref_dir(), so we just need to recurse
+ * through all of the sub-directories. We do not even need to care about
+ * sorting, as traversal order does not matter to us.
+ */
+static void prime_ref_dir(struct ref_dir *dir)
+{
+ int i;
+ for (i = 0; i dir-nr; i++) {
+ struct ref_entry *entry = dir-entries[i];
+ if (entry-flag REF_DIR)
+ prime_ref_dir(get_ref_dir(entry));
+ }
+}
+/*
* Return true iff refname1 and refname2 conflict with each other.
* Two reference names conflict if one of them exactly matches the
* leading components of the other; e.g., foo/bar conflicts with
@@ -1603,15 +1618,31 @@ void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refname)
static int do_for_each_entry(struct ref_cache *refs, const char *base,
each_ref_entry_fn fn, void *cb_data)
{
- struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs);
- struct ref_dir *packed_dir = get_packed_ref_dir(packed_ref_cache);
- struct ref_dir *loose_dir = get_loose_refs(refs);
+ struct packed_ref_cache *packed_ref_cache;
+ struct ref_dir *loose_dir;
+ struct ref_dir *packed_dir;
int retval = 0;
+ /*
+* We must make sure that all loose refs are read before accessing the
+* packed-refs file; this avoids a race condition in which loose refs
+* are migrated to the packed-refs file by a simultaneous process, but
+* our in-memory view is from before the migration.
get_packed_ref_cache()
+* takes care of making sure our view is up to date with what is on
+* disk.
+*/
+ loose_dir = get_loose_refs(refs);
+ if (base *base) {
+ loose_dir = find_containing_dir(loose_dir, base, 0);
+ }
+ if (loose_dir)
+ prime_ref_dir(loose_dir);
+
+ packed_ref_cache = get_packed_ref_cache(refs);
acquire_packed_ref_cache(packed_ref_cache);
+ packed_dir = get_packed_ref_dir(packed_ref_cache);
if (base *base) {
packed_dir = find_containing_dir(packed_dir, base, 0);
- loose_dir = find_containing_dir(loose_dir, base, 0);
}
if (packed_dir loose_dir) {
--
1.8.3.1
--
To unsubscribe from this list: send the line
[PATCH v3 12/12] refs: do not invalidate the packed-refs cache unnecessarily
Now that we keep track of the packed-refs file metadata, we can detect
when the packed-refs file has been modified since we last read it, and
we do so automatically every time that get_packed_ref_cache() is
called. So there is no need to invalidate the cache automatically
when lock_packed_refs() is called; usually the old copy will still be
valid.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 787cc1c..038e5c7 100644
--- a/refs.c
+++ b/refs.c
@@ -2136,11 +2136,14 @@ int lock_packed_refs(int flags)
{
struct packed_ref_cache *packed_ref_cache;
- /* Discard the old cache because it might be invalid: */
- clear_packed_ref_cache(ref_cache);
if (hold_lock_file_for_update(packlock, git_path(packed-refs),
flags) 0)
return -1;
- /* Read the current packed-refs while holding the lock: */
+ /*
+* Get the current packed-refs while holding the lock. If the
+* packed-refs file has been modified since we last read it,
+* this will automatically invalidate the cache and re-read
+* the packed-refs file.
+*/
packed_ref_cache = get_packed_ref_cache(ref_cache);
packed_ref_cache-lock = packlock;
/* Increment the reference count to prevent it from being freed: */
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 06/12] do_for_each_entry(): increment the packed refs cache refcount
This function calls a user-supplied callback function which could do
something that causes the packed refs cache to be invalidated. So
acquire a reference count on the data structure to prevent our copy
from being freed while we are iterating over it.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 80c172f..f33d224 100644
--- a/refs.c
+++ b/refs.c
@@ -1590,10 +1590,12 @@ void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refname)
static int do_for_each_entry(struct ref_cache *refs, const char *base,
each_ref_entry_fn fn, void *cb_data)
{
- struct ref_dir *packed_dir = get_packed_refs(refs);
+ struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs);
+ struct ref_dir *packed_dir = get_packed_ref_dir(packed_ref_cache);
struct ref_dir *loose_dir = get_loose_refs(refs);
int retval = 0;
+ acquire_packed_ref_cache(packed_ref_cache);
if (base *base) {
packed_dir = find_containing_dir(packed_dir, base, 0);
loose_dir = find_containing_dir(loose_dir, base, 0);
@@ -1614,6 +1616,7 @@ static int do_for_each_entry(struct ref_cache *refs,
const char *base,
loose_dir, 0, fn, cb_data);
}
+ release_packed_ref_cache(packed_ref_cache);
return retval;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 10/12] get_packed_ref_cache: reload packed-refs file when it changes
From: Jeff King p...@peff.net
Once we read the packed-refs file into memory, we cache it
to save work on future ref lookups. However, our cache may
be out of date with respect to what is on disk if another
process is simultaneously packing the refs. Normally it
is acceptable for us to be a little out of date, since there
is no guarantee whether we read the file before or after the
simultaneous update. However, there is an important special
case: our packed-refs file must be up to date with respect
to any loose refs we read. Otherwise, we risk the following
race condition:
0. There exists a loose ref refs/heads/master.
1. Process A starts and looks up the ref master. It
first checks $GIT_DIR/master, which does not exist. It
then loads (and caches) the packed-refs file to see if
master exists in it, which it does not.
2. Meanwhile, process B runs pack-refs --all --prune. It
creates a new packed-refs file which contains
refs/heads/master, and removes the loose copy at
$GIT_DIR/refs/heads/master.
3. Process A continues its lookup, and eventually tries
$GIT_DIR/refs/heads/master. It sees that the loose ref
is missing, and falls back to the packed-refs file. But
it examines its cached version, which does not have
refs/heads/master. After trying a few other prefixes,
it reports master as a non-existent ref.
There are many variants (e.g., step 1 may involve process A
looking up another ref entirely, so even a fully qualified
refname can fail). One of the most interesting ones is if
refs/heads/master is already packed. In that case process
A will not see it as missing, but rather will report
whatever value happened to be in the packed-refs file before
process B repacked (which might be an arbitrarily old
value).
We can fix this by making sure we reload the packed-refs
file from disk after looking at any loose refs. That's
unacceptably slow, so we can check its stat()-validity as a
proxy, and read it only when it appears to have changed.
Reading the packed-refs file after performing any loose-ref
system calls is sufficient because we know the ordering of
the pack-refs process: it always makes sure the newly
written packed-refs file is installed into place before
pruning any loose refs. As long as those operations by B
appear in their executed order to process A, by the time A
sees the missing loose ref, the new packed-refs file must be
in place.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index a5a5b5d..60f2507 100644
--- a/refs.c
+++ b/refs.c
@@ -825,6 +825,9 @@ struct packed_ref_cache {
* when it is unlocked.
*/
struct lock_file *lock;
+
+ /* The metadata from when this packed-refs cache was read */
+ struct stat_validity validity;
};
/*
@@ -862,6 +865,7 @@ static int release_packed_ref_cache(struct packed_ref_cache
*packed_refs)
{
if (!--packed_refs-referrers) {
free_ref_entry(packed_refs-root);
+ stat_validity_clear(packed_refs-validity);
free(packed_refs);
return 1;
} else {
@@ -1053,19 +1057,26 @@ static void read_packed_refs(FILE *f, struct ref_dir
*dir)
*/
static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
{
+ const char *packed_refs_file;
+
+ if (*refs-name)
+ packed_refs_file = git_path_submodule(refs-name,
packed-refs);
+ else
+ packed_refs_file = git_path(packed-refs);
+
+ if (refs-packed
+ !stat_validity_check(refs-packed-validity, packed_refs_file))
+ clear_packed_ref_cache(refs);
+
if (!refs-packed) {
- const char *packed_refs_file;
FILE *f;
refs-packed = xcalloc(1, sizeof(*refs-packed));
acquire_packed_ref_cache(refs-packed);
refs-packed-root = create_dir_entry(refs, , 0, 0);
- if (*refs-name)
- packed_refs_file = git_path_submodule(refs-name,
packed-refs);
- else
- packed_refs_file = git_path(packed-refs);
f = fopen(packed_refs_file, r);
if (f) {
+ stat_validity_update(refs-packed-validity,
fileno(f));
read_packed_refs(f, get_ref_dir(refs-packed-root));
fclose(f);
}
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 04/12] refs: implement simple transactions for the packed-refs file
Handle simple transactions for the packed-refs file at the
packed_ref_cache level via new functions lock_packed_refs(),
commit_packed_refs(), and rollback_packed_refs().
Only allow the packed ref cache to be modified (via add_packed_ref())
while the packed refs file is locked.
Change clone to add the new references within a transaction.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/clone.c | 5 +++-
refs.c | 88 +
refs.h | 26 +++--
3 files changed, 98 insertions(+), 21 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 66bff57..14b1323 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -493,13 +493,16 @@ static void write_remote_refs(const struct ref
*local_refs)
{
const struct ref *r;
+ lock_packed_refs(LOCK_DIE_ON_ERROR);
+
for (r = local_refs; r; r = r-next) {
if (!r-peer_ref)
continue;
add_packed_ref(r-peer_ref-name, r-old_sha1);
}
- pack_refs(PACK_REFS_ALL);
+ if (commit_packed_refs())
+ die_errno(unable to overwrite old ref-pack file);
}
static void write_followtags(const struct ref *refs, const char *msg)
diff --git a/refs.c b/refs.c
index 373d95b..b345799 100644
--- a/refs.c
+++ b/refs.c
@@ -808,6 +808,13 @@ static int is_refname_available(const char *refname, const
char *oldrefname,
struct packed_ref_cache {
struct ref_entry *root;
+
+ /*
+* Iff the packed-refs file associated with this instance is
+* currently locked for writing, this points at the associated
+* lock (which is owned by somebody else).
+*/
+ struct lock_file *lock;
};
/*
@@ -826,9 +833,14 @@ static struct ref_cache {
char name[1];
} ref_cache, *submodule_ref_caches;
+/* Lock used for the main packed-refs file: */
+static struct lock_file packlock;
+
static void clear_packed_ref_cache(struct ref_cache *refs)
{
if (refs-packed) {
+ if (refs-packed-lock)
+ die(internal error: packed-ref cache cleared while
locked);
free_ref_entry(refs-packed-root);
free(refs-packed);
refs-packed = NULL;
@@ -1038,7 +1050,12 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
void add_packed_ref(const char *refname, const unsigned char *sha1)
{
- add_ref(get_packed_refs(ref_cache),
+ struct packed_ref_cache *packed_ref_cache =
+ get_packed_ref_cache(ref_cache);
+
+ if (!packed_ref_cache-lock)
+ die(internal error: packed refs not locked);
+ add_ref(get_packed_ref_dir(packed_ref_cache),
create_ref_entry(refname, sha1, REF_ISPACKED, 1));
}
@@ -2035,6 +2052,52 @@ static int write_packed_entry_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
+int lock_packed_refs(int flags)
+{
+ struct packed_ref_cache *packed_ref_cache;
+
+ /* Discard the old cache because it might be invalid: */
+ clear_packed_ref_cache(ref_cache);
+ if (hold_lock_file_for_update(packlock, git_path(packed-refs),
flags) 0)
+ return -1;
+ /* Read the current packed-refs while holding the lock: */
+ packed_ref_cache = get_packed_ref_cache(ref_cache);
+ packed_ref_cache-lock = packlock;
+ return 0;
+}
+
+int commit_packed_refs(void)
+{
+ struct packed_ref_cache *packed_ref_cache =
+ get_packed_ref_cache(ref_cache);
+ int error = 0;
+
+ if (!packed_ref_cache-lock)
+ die(internal error: packed-refs not locked);
+ write_or_die(packed_ref_cache-lock-fd,
+PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+
+ do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
+0, write_packed_entry_fn,
+packed_ref_cache-lock-fd);
+ if (commit_lock_file(packed_ref_cache-lock))
+ error = -1;
+ packed_ref_cache-lock = NULL;
+ return error;
+}
+
+void rollback_packed_refs(void)
+{
+ struct packed_ref_cache *packed_ref_cache =
+ get_packed_ref_cache(ref_cache);
+
+ if (!packed_ref_cache-lock)
+ die(internal error: packed-refs not locked);
+ rollback_lock_file(packed_ref_cache-lock);
+ packed_ref_cache-lock = NULL;
+ clear_packed_ref_cache(ref_cache);
+}
+
struct ref_to_prune {
struct ref_to_prune *next;
unsigned char sha1[20];
@@ -2148,28 +2211,22 @@ static void prune_refs(struct ref_to_prune *r)
}
}
-static struct lock_file packlock;
-
int pack_refs(unsigned int flags)
{
struct pack_refs_cb_data cbdata;
- int fd;
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- fd = hold_lock_file_for_update(packlock,
[PATCH v3 09/12] add a stat_validity struct
It can sometimes be useful to know whether a path in the
filesystem has been updated without going to the work of
opening and re-reading its content. We trust the stat()
information on disk already to handle index updates, and we
can use the same trick here.
This patch introduces a stat_validity struct which
encapsulates the concept of checking the stat-freshness of a
file. It is implemented on top of struct stat_data to
reuse the logic about which stat entries to trust for a
particular platform, but hides the complexity behind two
simple functions: check and update.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h | 27 +++
read-cache.c | 30 ++
2 files changed, 57 insertions(+)
diff --git a/cache.h b/cache.h
index f158fed..50f33db 100644
--- a/cache.h
+++ b/cache.h
@@ -1360,4 +1360,31 @@ int checkout_fast_forward(const unsigned char *from,
int sane_execvp(const char *file, char *const argv[]);
+/*
+ * A struct to encapsulate the concept of whether a file has changed
+ * since we last checked it. This uses criteria similar to those used
+ * for the index.
+ */
+struct stat_validity {
+ struct stat_data *sd;
+};
+
+void stat_validity_clear(struct stat_validity *sv);
+
+/*
+ * Returns 1 if the path is a regular file (or a symlink to a regular
+ * file) and matches the saved stat_validity, 0 otherwise. A missing
+ * or inaccessible file is considered a match if the struct was just
+ * initialized, or if the previous update found an inaccessible file.
+ */
+int stat_validity_check(struct stat_validity *sv, const char *path);
+
+/*
+ * Update the stat_validity from a file opened at descriptor fd. If
+ * the file is missing, inaccessible, or not a regular file, then
+ * future calls to stat_validity_check will match iff one of those
+ * conditions continues to be true.
+ */
+void stat_validity_update(struct stat_validity *sv, int fd);
+
#endif /* CACHE_H */
diff --git a/read-cache.c b/read-cache.c
index 5660b37..b15bc09 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1950,3 +1950,33 @@ void *read_blob_data_from_index(struct index_state
*istate, const char *path, un
*size = sz;
return data;
}
+
+void stat_validity_clear(struct stat_validity *sv)
+{
+ free(sv-sd);
+ sv-sd = NULL;
+}
+
+int stat_validity_check(struct stat_validity *sv, const char *path)
+{
+ struct stat st;
+
+ if (stat(path, st) 0)
+ return sv-sd == NULL;
+ if (!sv-sd)
+ return 0;
+ return S_ISREG(st.st_mode) !match_stat_data(sv-sd, st);
+}
+
+void stat_validity_update(struct stat_validity *sv, int fd)
+{
+ struct stat st;
+
+ if (fstat(fd, st) 0 || !S_ISREG(st.st_mode))
+ stat_validity_clear(sv);
+ else {
+ if (!sv-sd)
+ sv-sd = xcalloc(1, sizeof(struct stat_data));
+ fill_stat_data(sv-sd, st);
+ }
+}
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 07/12] packed_ref_cache: increment refcount when locked
Increment the packed_ref_cache reference count while it is locked to
prevent its being freed.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index f33d224..a5a5b5d 100644
--- a/refs.c
+++ b/refs.c
@@ -820,7 +820,9 @@ struct packed_ref_cache {
/*
* Iff the packed-refs file associated with this instance is
* currently locked for writing, this points at the associated
-* lock (which is owned by somebody else).
+* lock (which is owned by somebody else). The referrer count
+* is also incremented when the file is locked and decremented
+* when it is unlocked.
*/
struct lock_file *lock;
};
@@ -2099,6 +2101,8 @@ int lock_packed_refs(int flags)
/* Read the current packed-refs while holding the lock: */
packed_ref_cache = get_packed_ref_cache(ref_cache);
packed_ref_cache-lock = packlock;
+ /* Increment the reference count to prevent it from being freed: */
+ acquire_packed_ref_cache(packed_ref_cache);
return 0;
}
@@ -2119,6 +2123,7 @@ int commit_packed_refs(void)
if (commit_lock_file(packed_ref_cache-lock))
error = -1;
packed_ref_cache-lock = NULL;
+ release_packed_ref_cache(packed_ref_cache);
return error;
}
@@ -2131,6 +2136,7 @@ void rollback_packed_refs(void)
die(internal error: packed-refs not locked);
rollback_lock_file(packed_ref_cache-lock);
packed_ref_cache-lock = NULL;
+ release_packed_ref_cache(packed_ref_cache);
clear_packed_ref_cache(ref_cache);
}
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 08/12] Extract a struct stat_data from cache_entry
Add public functions fill_stat_data() and match_stat_data() to work
with it. This infrastructure will later be used to check the validity
of other types of file.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/ls-files.c | 12 +++--
cache.h| 33 +---
read-cache.c | 151 +
3 files changed, 116 insertions(+), 80 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2202072..6a0730f 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -165,11 +165,13 @@ static void show_ce_entry(const char *tag, struct
cache_entry *ce)
}
write_name(ce-name, ce_namelen(ce));
if (debug_mode) {
- printf( ctime: %d:%d\n, ce-ce_ctime.sec, ce-ce_ctime.nsec);
- printf( mtime: %d:%d\n, ce-ce_mtime.sec, ce-ce_mtime.nsec);
- printf( dev: %d\tino: %d\n, ce-ce_dev, ce-ce_ino);
- printf( uid: %d\tgid: %d\n, ce-ce_uid, ce-ce_gid);
- printf( size: %d\tflags: %x\n, ce-ce_size, ce-ce_flags);
+ struct stat_data *sd = ce-ce_stat_data;
+
+ printf( ctime: %d:%d\n, sd-sd_ctime.sec, sd-sd_ctime.nsec);
+ printf( mtime: %d:%d\n, sd-sd_mtime.sec, sd-sd_mtime.nsec);
+ printf( dev: %d\tino: %d\n, sd-sd_dev, sd-sd_ino);
+ printf( uid: %d\tgid: %d\n, sd-sd_uid, sd-sd_gid);
+ printf( size: %d\tflags: %x\n, sd-sd_size, ce-ce_flags);
}
}
diff --git a/cache.h b/cache.h
index 820aa05..f158fed 100644
--- a/cache.h
+++ b/cache.h
@@ -119,15 +119,19 @@ struct cache_time {
unsigned int nsec;
};
+struct stat_data {
+ struct cache_time sd_ctime;
+ struct cache_time sd_mtime;
+ unsigned int sd_dev;
+ unsigned int sd_ino;
+ unsigned int sd_uid;
+ unsigned int sd_gid;
+ unsigned int sd_size;
+};
+
struct cache_entry {
- struct cache_time ce_ctime;
- struct cache_time ce_mtime;
- unsigned int ce_dev;
- unsigned int ce_ino;
+ struct stat_data ce_stat_data;
unsigned int ce_mode;
- unsigned int ce_uid;
- unsigned int ce_gid;
- unsigned int ce_size;
unsigned int ce_flags;
unsigned int ce_namelen;
unsigned char sha1[20];
@@ -511,6 +515,21 @@ extern int limit_pathspec_to_literal(void);
#define HASH_FORMAT_CHECK 2
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum
object_type type, const char *path, unsigned flags);
extern int index_path(unsigned char *sha1, const char *path, struct stat *st,
unsigned flags);
+
+/*
+ * Record to sd the data from st that we use to check whether a file
+ * might have changed.
+ */
+extern void fill_stat_data(struct stat_data *sd, struct stat *st);
+
+/*
+ * Return 0 if st is consistent with a file not having been changed
+ * since sd was filled. If there are differences, return a
+ * combination of MTIME_CHANGED, CTIME_CHANGED, OWNER_CHANGED,
+ * INODE_CHANGED, and DATA_CHANGED.
+ */
+extern int match_stat_data(const struct stat_data *sd, struct stat *st);
+
extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
#define REFRESH_REALLY 0x0001 /* ignore_valid */
diff --git a/read-cache.c b/read-cache.c
index 5e30746..5660b37 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -67,6 +67,61 @@ void rename_index_entry_at(struct index_state *istate, int
nr, const char *new_n
add_index_entry(istate, new,
ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
}
+void fill_stat_data(struct stat_data *sd, struct stat *st)
+{
+ sd-sd_ctime.sec = (unsigned int)st-st_ctime;
+ sd-sd_mtime.sec = (unsigned int)st-st_mtime;
+ sd-sd_ctime.nsec = ST_CTIME_NSEC(*st);
+ sd-sd_mtime.nsec = ST_MTIME_NSEC(*st);
+ sd-sd_dev = st-st_dev;
+ sd-sd_ino = st-st_ino;
+ sd-sd_uid = st-st_uid;
+ sd-sd_gid = st-st_gid;
+ sd-sd_size = st-st_size;
+}
+
+int match_stat_data(const struct stat_data *sd, struct stat *st)
+{
+ int changed = 0;
+
+ if (sd-sd_mtime.sec != (unsigned int)st-st_mtime)
+ changed |= MTIME_CHANGED;
+ if (trust_ctime check_stat
+ sd-sd_ctime.sec != (unsigned int)st-st_ctime)
+ changed |= CTIME_CHANGED;
+
+#ifdef USE_NSEC
+ if (check_stat sd-sd_mtime.nsec != ST_MTIME_NSEC(*st))
+ changed |= MTIME_CHANGED;
+ if (trust_ctime check_stat
+ sd-sd_ctime.nsec != ST_CTIME_NSEC(*st))
+ changed |= CTIME_CHANGED;
+#endif
+
+ if (check_stat) {
+ if (sd-sd_uid != (unsigned int) st-st_uid ||
+ sd-sd_gid != (unsigned int) st-st_gid)
+ changed |= OWNER_CHANGED;
+ if (sd-sd_ino != (unsigned int) st-st_ino)
+ changed |= INODE_CHANGED;
+ }
+
+#ifdef USE_STDEV
+ /*
+* st_dev breaks on network
[PATCH v3 05/12] refs: manage lifetime of packed refs cache via reference counting
In struct packed_ref_cache, keep a count of the number of users of the
data structure. Only free the packed ref cache when the reference
count goes to zero rather than when the packed ref cache is cleared.
This mechanism will be used to prevent the cache data structure from
being freed while it is being iterated over.
So far, only the reference in struct ref_cache::packed is counted;
other users will be adjusted in separate commits.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index b345799..80c172f 100644
--- a/refs.c
+++ b/refs.c
@@ -810,6 +810,14 @@ struct packed_ref_cache {
struct ref_entry *root;
/*
+* Count of references to the data structure in this instance,
+* including the pointer from ref_cache::packed if any. The
+* data will not be freed as long as the reference count is
+* nonzero.
+*/
+ unsigned int referrers;
+
+ /*
* Iff the packed-refs file associated with this instance is
* currently locked for writing, this points at the associated
* lock (which is owned by somebody else).
@@ -836,14 +844,38 @@ static struct ref_cache {
/* Lock used for the main packed-refs file: */
static struct lock_file packlock;
+/*
+ * Increment the reference count of *packed_refs.
+ */
+static void acquire_packed_ref_cache(struct packed_ref_cache *packed_refs)
+{
+ packed_refs-referrers++;
+}
+
+/*
+ * Decrease the reference count of *packed_refs. If it goes to zero,
+ * free *packed_refs and return true; otherwise return false.
+ */
+static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)
+{
+ if (!--packed_refs-referrers) {
+ free_ref_entry(packed_refs-root);
+ free(packed_refs);
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
static void clear_packed_ref_cache(struct ref_cache *refs)
{
if (refs-packed) {
- if (refs-packed-lock)
+ struct packed_ref_cache *packed_refs = refs-packed;
+
+ if (packed_refs-lock)
die(internal error: packed-ref cache cleared while
locked);
- free_ref_entry(refs-packed-root);
- free(refs-packed);
refs-packed = NULL;
+ release_packed_ref_cache(packed_refs);
}
}
@@ -1024,6 +1056,7 @@ static struct packed_ref_cache
*get_packed_ref_cache(struct ref_cache *refs)
FILE *f;
refs-packed = xcalloc(1, sizeof(*refs-packed));
+ acquire_packed_ref_cache(refs-packed);
refs-packed-root = create_dir_entry(refs, , 0, 0);
if (*refs-name)
packed_refs_file = git_path_submodule(refs-name,
packed-refs);
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 03/12] refs: wrap the packed refs cache in a level of indirection
As we know, we can solve any problem in this manner. In this case,
the problem is to avoid freeing a packed refs cache while somebody is
using it. So add a level of indirection as a prelude to
reference-counting the packed refs cache.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 9f1a007..373d95b 100644
--- a/refs.c
+++ b/refs.c
@@ -806,6 +806,10 @@ static int is_refname_available(const char *refname, const
char *oldrefname,
return 1;
}
+struct packed_ref_cache {
+ struct ref_entry *root;
+};
+
/*
* Future: need to be in struct repository
* when doing a full libification.
@@ -813,7 +817,7 @@ static int is_refname_available(const char *refname, const
char *oldrefname,
static struct ref_cache {
struct ref_cache *next;
struct ref_entry *loose;
- struct ref_entry *packed;
+ struct packed_ref_cache *packed;
/*
* The submodule name, or for the main repo. We allocate
* length 1 rather than FLEX_ARRAY so that the main ref_cache
@@ -825,7 +829,8 @@ static struct ref_cache {
static void clear_packed_ref_cache(struct ref_cache *refs)
{
if (refs-packed) {
- free_ref_entry(refs-packed);
+ free_ref_entry(refs-packed-root);
+ free(refs-packed);
refs-packed = NULL;
}
}
@@ -996,24 +1001,39 @@ static void read_packed_refs(FILE *f, struct ref_dir
*dir)
}
}
-static struct ref_dir *get_packed_refs(struct ref_cache *refs)
+/*
+ * Get the packed_ref_cache for the specified ref_cache, creating it
+ * if necessary.
+ */
+static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
{
if (!refs-packed) {
const char *packed_refs_file;
FILE *f;
- refs-packed = create_dir_entry(refs, , 0, 0);
+ refs-packed = xcalloc(1, sizeof(*refs-packed));
+ refs-packed-root = create_dir_entry(refs, , 0, 0);
if (*refs-name)
packed_refs_file = git_path_submodule(refs-name,
packed-refs);
else
packed_refs_file = git_path(packed-refs);
f = fopen(packed_refs_file, r);
if (f) {
- read_packed_refs(f, get_ref_dir(refs-packed));
+ read_packed_refs(f, get_ref_dir(refs-packed-root));
fclose(f);
}
}
- return get_ref_dir(refs-packed);
+ return refs-packed;
+}
+
+static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache
*packed_ref_cache)
+{
+ return get_ref_dir(packed_ref_cache-root);
+}
+
+static struct ref_dir *get_packed_refs(struct ref_cache *refs)
+{
+ return get_packed_ref_dir(get_packed_ref_cache(refs));
}
void add_packed_ref(const char *refname, const unsigned char *sha1)
--
1.8.3.1
--
To unsubscribe from this list: send the line 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 00/12] Fix some reference-related races
v3 of mh/ref-races. Thanks to Junio for the suggestion implemented in this version. Change since v2: * Change lock_packed_refs() to use its own struct lock_file rather than requiring the caller to supply its own. Please note that this patch series's usage of stat()/fstat() causes breakages under cygwin, pointed out by Ramsay Jones. He has graciously offered to work on this issue. Patch 12/12 is still optional--it avoids a little bit of work when rewriting the packed-refs file (including when deleting a reference that has a packed version), but relies on the stat-based freshness check not giving a false negative. Peff seems to lean slightly against merging the last patch and AFAIK nobody else has expressed an opinion. (Of course, if the test cannot be relied upon in the check before writing, then we should not forget that its failure in a check before reading can also cause problems, however rarely.) Jeff King (2): get_packed_ref_cache: reload packed-refs file when it changes for_each_ref: load all loose refs before packed refs Michael Haggerty (10): repack_without_ref(): split list curation and entry writing pack_refs(): split creation of packed refs and entry writing refs: wrap the packed refs cache in a level of indirection refs: implement simple transactions for the packed-refs file refs: manage lifetime of packed refs cache via reference counting do_for_each_entry(): increment the packed refs cache refcount packed_ref_cache: increment refcount when locked Extract a struct stat_data from cache_entry add a stat_validity struct refs: do not invalidate the packed-refs cache unnecessarily builtin/clone.c| 5 +- builtin/ls-files.c | 12 +- cache.h| 60 -- read-cache.c | 181 ++ refs.c | 319 - refs.h | 26 - 6 files changed, 469 insertions(+), 134 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line 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 01/12] repack_without_ref(): split list curation and entry writing
The repack_without_ref() function first removes the deleted ref from
the internal packed-refs list, then writes the packed-refs list to
disk, omitting any broken or stale entries. This patch splits that
second step into multiple passes:
* collect the list of refnames that should be deleted from packed_refs
* delete those refnames from the cache
* write the remainder to the packed-refs file
The purpose of this change is to make the write the remainder part
reusable.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 62 ++
1 file changed, 50 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 42a7e17..1184b99 100644
--- a/refs.c
+++ b/refs.c
@@ -1998,6 +1998,23 @@ static void write_packed_entry(int fd, char *refname,
unsigned char *sha1,
}
}
+/*
+ * An each_ref_entry_fn that writes the entry to a packed-refs file.
+ */
+static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
+{
+ int *fd = cb_data;
+ enum peel_status peel_status = peel_entry(entry, 0);
+
+ if (peel_status != PEEL_PEELED peel_status != PEEL_NON_TAG)
+ error(internal error: %s is not a valid packed reference!,
+ entry-name);
+ write_packed_entry(*fd, entry-name, entry-u.value.sha1,
+ peel_status == PEEL_PEELED ?
+ entry-u.value.peeled : NULL);
+ return 0;
+}
+
struct ref_to_prune {
struct ref_to_prune *next;
unsigned char sha1[20];
@@ -2117,14 +2134,25 @@ int pack_refs(unsigned int flags)
return 0;
}
-static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
+/*
+ * If entry is no longer needed in packed-refs, add it to the string
+ * list pointed to by cb_data. Reasons for deleting entries:
+ *
+ * - Entry is broken.
+ * - Entry is overridden by a loose ref.
+ * - Entry does not point at a valid object.
+ *
+ * In the first and third cases, also emit an error message because these
+ * are indications of repository corruption.
+ */
+static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
{
- int *fd = cb_data;
- enum peel_status peel_status;
+ struct string_list *refs_to_delete = cb_data;
if (entry-flag REF_ISBROKEN) {
/* This shouldn't happen to packed refs. */
error(%s is broken!, entry-name);
+ string_list_append(refs_to_delete, entry-name);
return 0;
}
if (!has_sha1_file(entry-u.value.sha1)) {
@@ -2134,7 +2162,7 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
if (read_ref_full(entry-name, sha1, 0, flags))
/* We should at least have found the packed ref. */
die(Internal error);
- if ((flags REF_ISSYMREF) || !(flags REF_ISPACKED))
+ if ((flags REF_ISSYMREF) || !(flags REF_ISPACKED)) {
/*
* This packed reference is overridden by a
* loose reference, so it is OK that its value
@@ -2143,9 +2171,11 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
* collected. For this purpose we don't even
* care whether the loose reference itself is
* invalid, broken, symbolic, etc. Silently
-* omit the packed reference from the output.
+* remove the packed reference.
*/
+ string_list_append(refs_to_delete, entry-name);
return 0;
+ }
/*
* There is no overriding loose reference, so the fact
* that this reference doesn't refer to a valid object
@@ -2154,14 +2184,10 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
* the output.
*/
error(%s does not point to a valid object!, entry-name);
+ string_list_append(refs_to_delete, entry-name);
return 0;
}
- peel_status = peel_entry(entry, 0);
- write_packed_entry(*fd, entry-name, entry-u.value.sha1,
- peel_status == PEEL_PEELED ?
- entry-u.value.peeled : NULL);
-
return 0;
}
@@ -2169,6 +2195,8 @@ static int repack_without_ref(const char *refname)
{
int fd;
struct ref_dir *packed;
+ struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
+ struct string_list_item *ref_to_delete;
if (!get_packed_ref(refname))
return 0; /* refname does not exist in packed refs */
@@ -2180,7 +2208,8 @@ static int repack_without_ref(const char *refname)
}
clear_packed_ref_cache(ref_cache);
packed = get_packed_refs(ref_cache);
-
Re: [PATCH v4 5/5] git-remote-mediawiki: Add preview subcommand into git mw.
benoit.per...@ensimag.fr writes:
+# @TODO : add documentation for verbose option
I guess that's not applicable anymore.
+distant mediawiki and combined with a template retrieved from the mediawiki.
s/distant/remote/
+sub preview {
+ my $wiki;
+ my ($remote_url, $wiki_page_name);
+ my ($new_content, $template);
+ my $file_content;
+
+ if ($file_name eq EMPTY) {
+ die Missing file argument, see `git mw help` \n;
Unneeded space before \n.
+ for (@{ $html_tree-extract_links() }) {
+ my ($link, $element, $attr) = @{ $_ };
+ my $url = url($link)-canonical;
+ $element-attr($attr, URI-new_abs($url, $remote_url));
+ }
Extracting this into a function like make_links_absolute would have
made it clearer.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/5] git-remote-mediawiki: new tool to preview local changes without pushing
benoit.per...@ensimag.fr writes: From: Benoit Person benoit.per...@ensimag.fr The #7 issue on git-mediawiki's issue tracker [1] states that the ability to preview content without pushing would be a nice thing to have. I make minor remarks, but on overall, the code looks good. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/12] Fix some reference-related races
On 06/19/2013 08:56 PM, Jeff King wrote: On Wed, Jun 19, 2013 at 09:51:21AM +0200, Michael Haggerty wrote: Re-roll of mh/ref-races. Thanks to Peff, Junio, and Ramsay for reviewing v1. Thanks. I just read through them again. Everything looks good to me. Patches 10 and 11 are missing my signoff, but obviously: Signed-off-by: Jeff King p...@peff.net The last patch is still optional--it avoids a little bit of work when rewriting the packed-refs file, but relies on the stat-based freshness check not giving a false negative. I don't have a real problem with it, but given the cygwin confusions that Ramsay mentioned, maybe it is better to hold back on it for now? It sounds like the cygwin problems go the other way (false positives instead of false negatives). I just realized that there is another long-standing problem in loose/packed refs handling: Suppose there is a loose reference for which an old version is still present in the packed-refs file. If the reference is deleted: 1. delete_ref() gets a lock on the loose reference file 2. delete_ref() deletes the loose reference file while retaining the lock 3. delete_ref() calls repack_without_ref() 4. repack_without_ref() tries to acquire a lock on the packed-refs file to delete the packed version of the reference. If the packed-refs file is already locked by another process (and there is no reason why that cannot be, and there is only one attempt to acquire the lock), then repack_without_ref() emits an error and returns with an error code. delete_ref() passes the error along, but doesn't restore the loose ref. The net result is that an old version of the ref (namely the one in the loose refs file) has been revived. That version might even point at an object that has already been garbage collected. This problem is similar to race conditions that have been discussed earlier, but this problem has nothing to do with the freshness/staleness of the packed-refs file WRT to the loose reference; it only depends on the packed-refs file being locked by another process at an inopportune time. I think this problem would also be fixed by the locking scheme that I proposed earlier [1]: to acquire and hold the packed-refs lock across the modification of *both* files, and to rewrite the packed-refs file *before* deleting the loose-refs file (because rewriting the packed-refs file without the to-be-deleted reference is a logical NOP). Michael [1] http://article.gmane.org/gmane.comp.version-control.git/212131 -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/5] Reroll patches against v1.8.3.1
Alexey Shumkin (5): t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs t7102 (reset): don't hardcode SHA-1 in expected outputs t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting pretty: user format ignores i18n.logOutputEncoding setting builtin/reset.c | 8 +- builtin/rev-list.c | 1 + builtin/shortlog.c | 1 + log-tree.c | 1 + submodule.c | 3 + t/t4041-diff-submodule-option.sh | 25 +++-- t/t4205-log-pretty-formats.sh| 179 - t/t6006-rev-list-format.sh | 209 --- t/t7102-reset.sh | 37 ++- 9 files changed, 299 insertions(+), 165 deletions(-) -- 1.8.3.1.15.g5c23c1e -- To unsubscribe from this list: send the line 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 v5 1/5] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use
them instead of hardcoding.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
t/t6006-rev-list-format.sh | 142 +
1 file changed, 79 insertions(+), 63 deletions(-)
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 0393c9f..d32e65e 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -7,8 +7,21 @@ test_description='git rev-list --pretty=format test'
test_tick
test_expect_success 'setup' '
-touch foo git add foo git commit -m added foo
- echo changed foo git commit -a -m changed foo
+ : foo
+ git add foo
+ git commit -m added foo
+ head1=$(git rev-parse --verify HEAD)
+ head1_short=$(git rev-parse --short $head1)
+ tree1=$(git cat-file commit HEAD | sed -n -e s/^tree //p -e /^$/q)
+ tree1_short=$(git rev-parse --short $tree1)
+ echo changed foo
+ git commit -a -m changed foo
+ head2=$(git rev-parse --verify HEAD)
+ head2_short=$(git rev-parse --short $head2)
+ head2_parent=$(git cat-file commit HEAD | sed -n -e s/^parent //p -e
/^$/q)
+ head2_parent_short=$(git rev-parse --short $head2_parent)
+ tree2=$(git cat-file commit HEAD | sed -n -e s/^tree //p -e /^$/q)
+ tree2_short=$(git rev-parse --short $tree2)
'
# usage: test_format name format_string expected_output
@@ -32,49 +45,49 @@ has_no_color () {
test_cmp expect $1
}
-test_format percent %%h 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format percent %%h EOF
+commit $head2
%h
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
%h
EOF
-test_format hash %H%n%h 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-131a310eb913d107dd3c09a65d1651175898735d
-131a310
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cf
+test_format hash %H%n%h EOF
+commit $head2
+$head2
+$head2_short
+commit $head1
+$head1
+$head1_short
EOF
-test_format tree %T%n%t 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-fe722612f26da5064c32ca3843aa154bdb0b08a0
-fe72261
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-4d5fcadc293a348e88f777dc0920f11e7d71441c
-4d5fcad
+test_format tree %T%n%t EOF
+commit $head2
+$tree2
+$tree2_short
+commit $head1
+$tree1
+$tree1_short
EOF
-test_format parents %P%n%p 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cf
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format parents %P%n%p EOF
+commit $head2
+$head1
+$head2_parent_short
+commit $head1
EOF
# we don't test relative here
-test_format author %an%n%ae%n%ad%n%aD%n%at 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format author %an%n%ae%n%ad%n%aD%n%at EOF
+commit $head2
A U Thor
aut...@example.com
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
A U Thor
aut...@example.com
Thu Apr 7 15:13:13 2005 -0700
@@ -82,14 +95,14 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format committer %cn%n%ce%n%cd%n%cD%n%ct 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format committer %cn%n%ce%n%cd%n%cD%n%ct EOF
+commit $head2
C O Mitter
commit...@example.com
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
C O Mitter
commit...@example.com
Thu Apr 7 15:13:13 2005 -0700
@@ -97,43 +110,43 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format encoding %e 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format encoding %e EOF
+commit $head2
+commit $head1
EOF
-test_format subject %s 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format subject %s EOF
+commit $head2
changed foo
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
added foo
EOF
-test_format body %b 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format body %b EOF
+commit $head2
+commit $head1
EOF
-test_format raw-body %B 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format raw-body %B EOF
+commit $head2
changed foo
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
added foo
EOF
-test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy EOF
+commit $head2
[31mfoo[32mbar[34mbaz[mxyzzy
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
[31mfoo[32mbar[34mbaz[mxyzzy
EOF
-test_format advanced-colors '%C(red yellow bold)foo%C(reset)' 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format advanced-colors '%C(red yellow
[PATCH v5 3/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com --- t/t4205-log-pretty-formats.sh | 48 +++ 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 26fbfde..6d8d457 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -101,7 +101,11 @@ test_expect_failure 'NUL termination with --stat' ' test_expect_success 'setup more commits' ' test_commit message one one one message-one - test_commit message two two two message-two + test_commit message two two two message-two + head1=$(git rev-list --max-count=1 --abbrev-commit HEAD~0) + head2=$(git rev-list --max-count=1 --abbrev-commit HEAD~1) + head3=$(git rev-list --max-count=1 --abbrev-commit HEAD~2) + head4=$(git rev-list --max-count=1 --abbrev-commit HEAD~3) ' test_expect_success 'left alignment formatting' ' @@ -117,18 +121,18 @@ EOF test_cmp expected actual ' -test_expect_success 'left alignment formatting at the nth column' ' - git log --pretty=format:%h %|(40)%s actual +test_expect_success 'left alignment formatting at the nth column' + git log --pretty='format:%h %|(40)%s' actual # complete the incomplete line at the end echo actual qz_to_tab_space \EOF expected -fa33ab1 message twoZ -7cd6c63 message oneZ -1711bf9 add barZ -af20c06 initialZ +$head1 message twoZ +$head2 message oneZ +$head3 add barZ +$head4 initialZ EOF test_cmp expected actual -' + test_expect_success 'left alignment formatting with no padding' ' git log --pretty=format:%(1)%s actual @@ -195,18 +199,18 @@ EOF test_cmp expected actual ' -test_expect_success 'right alignment formatting at the nth column' ' - git log --pretty=format:%h %|(40)%s actual +test_expect_success 'right alignment formatting at the nth column' + git log --pretty='format:%h %|(40)%s' actual # complete the incomplete line at the end echo actual qz_to_tab_space \EOF expected -fa33ab1 message two -7cd6c63 message one -1711bf9 add bar -af20c06 initial +$head1 message two +$head2 message one +$head3 add bar +$head4 initial EOF test_cmp expected actual -' + test_expect_success 'right alignment formatting with no padding' ' git log --pretty=format:%(1)%s actual @@ -234,18 +238,18 @@ EOF test_cmp expected actual ' -test_expect_success 'center alignment formatting at the nth column' ' - git log --pretty=format:%h %|(40)%s actual +test_expect_success 'center alignment formatting at the nth column' + git log --pretty='format:%h %|(40)%s' actual # complete the incomplete line at the end echo actual qz_to_tab_space \EOF expected -fa33ab1 message two Z -7cd6c63 message one Z -1711bf9 add barZ -af20c06 initialZ +$head1 message two Z +$head2 message one Z +$head3 add barZ +$head4 initialZ EOF test_cmp expected actual -' + test_expect_success 'center alignment formatting with no padding' ' git log --pretty=format:%(1)%s actual -- 1.8.3.1.15.g5c23c1e -- To unsubscribe from this list: send the line 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 v5 5/5] pretty: user format ignores i18n.logOutputEncoding setting
The following two commands are expected to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it format %s.
Log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted with the latter even
when i18n.logOutputEncoding and terminal encoding are the same.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
and
$ git reset --hard
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
builtin/reset.c | 8 ++--
builtin/rev-list.c | 1 +
builtin/shortlog.c | 1 +
log-tree.c | 1 +
submodule.c | 3 +++
t/t4041-diff-submodule-option.sh | 10 +-
t/t4205-log-pretty-formats.sh| 34 +-
t/t6006-rev-list-format.sh | 8
t/t7102-reset.sh | 2 +-
9 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 6032131..b23ed63 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -92,11 +92,15 @@ static int reset_index(const unsigned char *sha1, int
reset_type, int quiet)
static void print_new_head_line(struct commit *commit)
{
- const char *hex, *body;
+ const char *hex, *body, *encoding;
hex = find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV);
printf(_(HEAD is now at %s), hex);
- body = strstr(commit-buffer, \n\n);
+ encoding = get_log_output_encoding();
+ body = logmsg_reencode(commit, NULL, encoding);
+ if (!body)
+ body = commit-buffer;
+ body = strstr(body, \n\n);
if (body) {
const char *eol;
size_t len;
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 67701be..a5ec30d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -111,6 +111,7 @@ static void show_commit(struct commit *commit, void *data)
ctx.date_mode = revs-date_mode;
ctx.date_mode_explicit = revs-date_mode_explicit;
ctx.fmt = revs-commit_format;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(ctx, commit, buf);
if (revs-graph) {
if (buf.len) {
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 1fd6f8a..1434f8f 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -137,6 +137,7 @@ void shortlog_add_commit(struct shortlog *log, struct
commit *commit)
ctx.subject = ;
ctx.after_subject = ;
ctx.date_mode = DATE_NORMAL;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(ctx, commit, ufbuf);
buffer = ufbuf.buf;
} else if (*buffer) {
diff --git a/log-tree.c b/log-tree.c
index 1946e9c..5277d3e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -616,6 +616,7 @@ void show_log(struct rev_info *opt)
ctx.fmt = opt-commit_format;
ctx.mailmap = opt-mailmap;
ctx.color = opt-diffopt.use_color;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(ctx, commit, msgbuf);
if (opt-add_signoff)
diff --git a/submodule.c b/submodule.c
index 1821a5b..baa8669 100644
--- a/submodule.c
+++ b/submodule.c
@@ -222,10 +222,13 @@ static void print_submodule_summary(struct rev_info *rev,
FILE *f,
static const char format[] = %m %s;
struct strbuf sb = STRBUF_INIT;
struct commit *commit;
+ const char *log_output_encoding;
+ log_output_encoding = get_log_output_encoding();
while ((commit = get_revision(rev))) {
struct pretty_print_context ctx = {0};
ctx.date_mode = rev-date_mode;
+ ctx.output_encoding = log_output_encoding;
strbuf_setlen(sb, 0);
strbuf_addstr(sb, line_prefix);
if (commit-object.flags SYMMETRIC_LEFT) {
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 22bf4df..ce192b0 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -94,7 +94,7 @@ test_expect_success 'diff.submodule does not affect plumbing'
'
commit_file sm1
head2=$(add_file sm1 foo3)
-test_expect_failure 'modified submodule(forward)' '
+test_expect_success 'modified submodule(forward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head1..$head2:
@@ -103,7 +103,7 @@ test_expect_failure 'modified submodule(forward)' '
test_cmp expected actual
'
-test_expect_failure 'modified submodule(forward)' '
+test_expect_success 'modified submodule(forward)' '
[PATCH v5 2/5] t7102 (reset): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com --- t/t7102-reset.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index df82ec9..05dfb27 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -192,7 +192,8 @@ test_expect_success \ 'changing files and redo the last commit should succeed' ' echo 3rd line 2nd file secondfile git commit -a -C ORIG_HEAD - check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d + head4=$(git rev-parse --verify HEAD) + check_changes $head4 test $(git rev-parse ORIG_HEAD) = \ $head5 ' @@ -211,7 +212,7 @@ test_expect_success \ git reset --hard HEAD~2 check_changes ddaefe00f1da16864591c61fdc7adb5d7cd6b74e test $(git rev-parse ORIG_HEAD) = \ - 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d + $head4 ' .diff_expect @@ -326,10 +327,11 @@ test_expect_success '--hard reset to HEAD should clear a failed merge' ' git checkout branch2 echo 3rd line in branch2 secondfile git commit -a -m change in branch2 + head3=$(git rev-parse --verify HEAD) test_must_fail git pull . branch1 git reset --hard - check_changes 77abb337073fb4369a7ad69ff6f5ec0e4d6b54bb + check_changes $head3 ' .diff_expect -- 1.8.3.1.15.g5c23c1e -- To unsubscribe from this list: send the line 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 v5 4/5] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
The following two commands are expected to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it format %s.
Log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted with the latter even
when i18n.logOutputEncoding and terminal encoding are the same.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
and
$ git reset --hard
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
t/t4041-diff-submodule-option.sh | 35 +
t/t4205-log-pretty-formats.sh| 149 ---
t/t6006-rev-list-format.sh | 85 +++---
t/t7102-reset.sh | 29 +++-
4 files changed, 200 insertions(+), 98 deletions(-)
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 32d4a60..22bf4df 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -1,6 +1,7 @@
#!/bin/sh
#
# Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin
+# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding tests)
#
test_description='Support for verbose submodule differences in git diff
@@ -10,6 +11,9 @@ This test tries to verify the sanity of the --submodule
option of git diff.
. ./test-lib.sh
+# String added in Russian, encoded in UTF-8, used in
+# sample commit log messages in add_file() function below.
+added=$(printf
\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275)
add_file () {
(
cd $1
@@ -19,7 +23,8 @@ add_file () {
echo $name $name
git add $name
test_tick
- git commit -m Add $name || exit
+ msg_added_iso88595=$(echo Add $name ($added $name) |
iconv -f utf-8 -t iso88595)
+ git -c 'i18n.commitEncoding=iso88595' commit -m
$msg_added_iso88595
done /dev/null
git rev-parse --short --verify HEAD
)
@@ -89,29 +94,29 @@ test_expect_success 'diff.submodule does not affect
plumbing' '
commit_file sm1
head2=$(add_file sm1 foo3)
-test_expect_success 'modified submodule(forward)' '
+test_expect_failure 'modified submodule(forward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head1..$head2:
- Add foo3
+ Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
-test_expect_success 'modified submodule(forward)' '
+test_expect_failure 'modified submodule(forward)' '
git diff --submodule=log actual
cat expected -EOF
Submodule sm1 $head1..$head2:
- Add foo3
+ Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
-test_expect_success 'modified submodule(forward) --submodule' '
+test_expect_failure 'modified submodule(forward) --submodule' '
git diff --submodule actual
cat expected -EOF
Submodule sm1 $head1..$head2:
- Add foo3
+ Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
@@ -138,25 +143,25 @@ head3=$(
git rev-parse --short --verify HEAD
)
-test_expect_success 'modified submodule(backward)' '
+test_expect_failure 'modified submodule(backward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head2..$head3 (rewind):
- Add foo3
- Add foo2
+ Add foo3 ($added foo3)
+ Add foo2 ($added foo2)
EOF
test_cmp expected actual
'
head4=$(add_file sm1 foo4 foo5)
-test_expect_success 'modified submodule(backward and forward)' '
+test_expect_failure 'modified submodule(backward and forward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head2...$head4:
- Add foo5
- Add foo4
- Add foo3
- Add foo2
+ Add foo5 ($added foo5)
+ Add foo4 ($added foo4)
+ Add foo3 ($added foo3)
+ Add foo2 ($added foo2)
EOF
test_cmp expected actual
'
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 6d8d457..6a56019 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1,20 +1,43 @@
#!/bin/sh
#
# Copyright (c) 2010, Will Palmer
+# Copyright (c) 2011, Alexey Shumkin (+ non-UTF-8 commit encoding tests)
#
test_description='Test pretty formats'
. ./test-lib.sh
+commit_msg() {
+ # String initial commit partly in Russian, encoded in UTF-8,
+ # used as a commit log message below.
+ msg=$(printf initial
Re: [PATCH v4 5/5] git-remote-mediawiki: Add preview subcommand into git mw.
benoit.per...@ensimag.fr writes:
+ for (@{ $html_tree-extract_links() }) {
+ my ($link, $element, $attr) = @{ $_ };
+ my $url = url($link)-canonical;
+ $element-attr($attr, URI-new_abs($url, $remote_url));
Actually, this breaks some relative links, like table of content links,
because you consider them relative to URLs like http://example.com/wiki/
but not http://example.com/wiki/index.php/My_Page.
Ideally, links pointing to stg like #anchor should not be modified.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Junio C Hamano wrote:
Decouple `simple` from `upstream` completely, and change it to mean
`current` with a safety feature: a `push` and `pull` should not be
asymmetrical in the special case of central workflows.
Double negation confused my parser. 'push' and 'pull' should be
kept symmetrical in central workflows?
They're not the same thing. It is very much intentional and intended:
the safety net is not to ensure that the push and pull are
symmetrical (i.e. among other things, error out if
branch.$branch.merge is unset), but rather ensure that the push and
pull are never asymmetrical.
Without any configuration the current branch is pushed out, which
loosens the safety we implemented in the current 'safer upstream'.
I am not convinced this is a good change. I am not convinced this is
a bad change, either, yet, but this loosening smells bad.
Provided that we would want to keep the Push the current one to the
same name but you have to have it set up as your integration source
safety for central workflow (which I am starting to think we
should), we would want something like this on top of your entire
series, I think. The behaviour change can be seen in the revert of
one test you made to the test that expects simple to fail due to
the safety.
Now I'd like to question what you are labelling as safety. What is
the consequence of erroring out when branch.$branch.merge is unset
when pushing using `upstream`? For me, it only means additional
inconvenience: any new branches I create can't be pushed out without
explicitly setting branch.$branch.merge to an invalid value. What
is invalid about it? The fact that it doesn't exist, @{u} still
doesn't resolve, and git branch -u doesn't work. Hell, even git push
-u doesn't work! So, what is this huge safety that can justify
inconveniencing me like this? By making sure that
branch.$branch.merge is set, my prompt responds immediately to
divergence, and this is awesome. Predictably, I use git push -u when
I push out a new branch with `current`. So, unless you have a damn
good reason to inconvenience me in the name of safety,
branch.$branch.merge should default to refs/heads/$branch, unless set
explicitly.
I didn't want to contaminate this series with an unrelated improvement
to `upstream`, which is why you don't see the change here: it is
orthogonal to designing a good `simple`, and I only brought it up to
question the safety you're carrying over to `simple`; what
obligation does `simple` have to carry over this feature? I've made
it clear that I want a clean break from `upstream`, and I find your
proposal is very inelegant: `simple` has two modes of operation; when
branch.$branch.remote is equal to $pushremote, branch.$branch.merge
must be set and equal to $branch (the `upstream` mode); when
branch.$branch.remote is unequal to $pushremote, don't care about
whether branch.$branch.merge is set (the `current` mode). My proposal
is much smoother than this modal operation, no?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Splitting a rev list into 2 sets
Hello, I'd like to write a script that would parse commits in one of my repo. Ideally this script should accept any revision ranges that git-rev-list would accept. This script should consider commits in master differently than the ones in others branches. To get the commit set which can't be reached by master (ie commits which are specific to branches other than master) I would do: # $@ is the range spec passed to the script git rev-list $@ ^master | check_other_commit But I don't know if it's possible to use a different git-rev-list command to get the rest of the commits, ie the ones that are reachable by the specified range and master. One way to do that is to record the first commit set got by the first rev-list command and check that the ones returned by git rev-list $@ are not in the record. But I'm wondering if someone can see another solution more elegant ? Thanks -- Francis -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] glossary: define committish (a.k.a. commit-ish)
Richard Hansen wrote: In the meantime, I'll rework the patch series to avoid using the word ref when defining committish and tree-ish. This is a good way forward. I'm curious about what you're planning on substituting it with though: the extended SHA-1 expression that Jonathan hinted at? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] glossary: define committish (a.k.a. commit-ish)
Richard Hansen wrote:
* Aren't HEAD, FETCH_HEAD, ORIG_HEAD also refs?
HEAD is a symbolic ref [*1*], FETCH_HEAD is not a ref [*2*], and
ORIG_HEAD is a ref.
ref
A binding of a name to an object or other ref (in which case it
is a symref). Refs are stored in the repository.
What is binding of a name? It's a file that contains exactly one
line: a 40-character SHA-1 hex. End of story. Like I explained in
[*1*], I see no advantage in grouping symbolic refs with refs.
The ref namespace is hierarchical. Different subhierarchies
are used for different purposes (e.g. the refs/heads/ hierarchy
is used to represent local branches).
Good.
[Footnotes]
*1*: HEAD is the one and only symbolic ref, unless you create more by
hand using `git symbolic-ref`. Unfortunately, no other symbolic ref
can ever be on the same footing as HEAD: the sources hard-code HEAD
in a large number of places, and reworking it to truly support
symbolic refs is probably not worth the pain at all. If you want to
convince yourself that this is true, run `git symbolic-ref H HEAD` and
then try to resolve H@{u}: HEAD@{u} resolves fine, doesn't it?
*2*: It's not an ordinary file like COMMIT_EDITMSG either, in that
rev-parse can operate on it. It is not a ref because it contains
references to several objects; cat it and see for yourself.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/5] git-remote-mediawiki: new tool to preview local changes without pushing
Ok, thanks for all the reviews, Final version (I hope :) ) will come when all the mediawiki patches will have graduated to 'master' then. Benoit Person -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/12] Fix some reference-related races
On Thu, Jun 20, 2013 at 11:09:49AM +0200, Michael Haggerty wrote: If the packed-refs file is already locked by another process (and there is no reason why that cannot be, and there is only one attempt to acquire the lock), then repack_without_ref() emits an error and returns with an error code. delete_ref() passes the error along, but doesn't restore the loose ref. [...] I think this problem would also be fixed by the locking scheme that I proposed earlier [1]: to acquire and hold the packed-refs lock across the modification of *both* files, and to rewrite the packed-refs file *before* deleting the loose-refs file (because rewriting the packed-refs file without the to-be-deleted reference is a logical NOP). Yeah, I agree. You could also roll back the loose deletion, but I'd rather just try to do it atomically. I don't think this increases lock contention, since delete_ref would need to lock the packed-refs file anyway. However, there is the related change that we should probably lock the packed-refs file before checking is this ref in the packed-refs file? in repack_without_ref. Which does increase lock contention, but is more correct. We should also consider deadlock issues. If the order is always acquire packed-refs lock, then acquire loose locks, we are fine. If this does loose-then-packed, we are also fine with the current code, as git-pack-refs does not prune the loose refs while under the packed-refs lock. But I seem to recall discussion of pruning them under the packed-refs lock, which would deadlock if repack_without_ref does loose-then-packed. But I guess we do not actually block on locks, but rather just die (and release our locks), so deadlock is not an option for us. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
On Thu, Jun 20, 2013 at 09:49:51AM +0200, Michael Haggerty wrote: [I just noticed that lock_ref_sha1_basic() leaks a struct lock_file every time that it is called.] I noticed that recently, too. I have a patch series about 90% complete that abstracts the tempfile handling (the ultimate goal of which is to optionally clean up tmp_* files in the objects/ directory). It refactors the lockfile cleanup, and it would not be too hard to have a committed or rolled-back lockfile actually remove itself from the to clean at exit list. Which would make it perfectly safe to have a lockfile as an automatic variable as long as you commit or rollback before leaving the function. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] send-email: sanitize author when writing From line
sender is now sanitized, but we didn't sanitize
author when checking whether From: line is needed
in the message body.
As a result git started writing duplicate From lines
when author matched sender and has utf8 characters.
Reported-by: SZEDER Gábor sze...@ira.uka.de
Tested-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
git-send-email.perl | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 671762b..ecbf56f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1259,6 +1259,7 @@ foreach my $t (@files) {
open my $fh, , $t or die can't open file $t;
my $author = undef;
+ my $sauthor = undef;
my $author_encoding;
my $has_content_type;
my $body_encoding;
@@ -1297,7 +1298,7 @@ foreach my $t (@files) {
}
elsif (/^From:\s+(.*)$/i) {
($author, $author_encoding) =
unquote_rfc2047($1);
- my $sauthor = sanitize_address($author);
+ $sauthor = sanitize_address($author);
next if $suppress_cc{'author'};
next if $suppress_cc{'self'} and $sauthor eq
$sender;
printf((mbox) Adding cc: %s from line '%s'\n,
@@ -1393,7 +1394,7 @@ foreach my $t (@files) {
$subject = quote_subject($subject, $auto_8bit_encoding);
}
- if (defined $author and $author ne $sender) {
+ if (defined $sauthor and $sauthor ne $sender) {
$message = From: $author\n\n$message;
if (defined $author_encoding) {
if ($has_content_type) {
--
MST
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] send-email: add test for duplicate utf8 name
Verify that author name is not duplicated if it matches sender, even if it is in utf8. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- t/t9001-send-email.sh | 14 ++ 1 file changed, 14 insertions(+) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 9f46f22..020acc4 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -956,6 +956,20 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' ' grep ^From: Füñný Nâmé odd_?=m...@example.com msgtxt1 ' +test_expect_success $PREREQ 'utf8 sender is not duplicated' ' + clean_fake_sendmail + test_commit weird_sender + test_when_finished git reset --hard HEAD^ + git commit --amend --author Füñný Nâmé odd_?=m...@example.com + git format-patch --stdout -1 funny_name.patch + git send-email --from=Füñný Nâmé odd_?=m...@example.com \ + --to=nob...@example.com \ + --smtp-server=$(pwd)/fake.sendmail \ + funny_name.patch + grep ^From: msgtxt1 msgfrom + test_line_count = 1 msgfrom +' + test_expect_success $PREREQ 'sendemail.composeencoding works' ' clean_fake_sendmail git config sendemail.composeencoding iso-8859-1 -- MST -- To unsubscribe from this list: send the line 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] pull: pull into void by fast-forwarding from empty tree
The logic for pulling into an unborn branch was originally designed to be used on a newly-initialized repository (d09e79c, git-pull: allow pulling into an empty repository, 2006-11-16). It thus did not initially deal with uncommitted changes in the unborn branch. The case of an _unstaged_ untracked file was fixed by by 4b3ffe5 (pull: do not clobber untracked files on initial pull, 2011-03-25). However, it still clobbered existing staged files, both when the file exists in the merged commit (it will be overwritten), and when it does not (it will be lost!). We fix this by doing a two-way merge, where the current side of the merge is an empty tree, and the target side is HEAD (already updated to FETCH_HEAD at this point). This amounts to claiming that all work in the index was done vs. an empty tree, and thus all content of the index is precious. Reported-by: Stefan Schüßler m...@stefanschuessler.de Signed-off-by: Thomas Rast tr...@inf.ethz.ch --- Stefan Schüßler m...@stefanschuessler.de writes: I think there's a bug in git pull. Doing a git pull in a fresh repository without any commits removes files in the index. [...] Thanks for the bug report! git-pull.sh | 9 - t/t5520-pull.sh | 24 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/git-pull.sh b/git-pull.sh index 638aabb..1f84383 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -266,10 +266,17 @@ case $merge_head in ;; esac +# Pulling into unborn branch: a shorthand for branching off +# FETCH_HEAD, for lazy typers. if test -z $orig_head then git update-ref -m initial pull HEAD $merge_head $curr_head - git read-tree -m -u HEAD || exit 1 + # Two-way merge: we claim the index is based on an empty tree, + # and try to fast-forward to HEAD. This ensures we will not + # lose index/worktree changes that the user already made on + # the unborn branch. + empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904 + git read-tree -m -u $empty_tree HEAD || exit 1 exit fi diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 6af6c63..927903c 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -57,6 +57,30 @@ test_expect_success 'pulling into void does not overwrite untracked files' ' ) ' +test_expect_success 'pulling into void does not overwrite staged files' ' + git init cloned-staged-colliding + ( + cd cloned-staged-colliding + echo alternate content file + git add file + test_must_fail git pull .. master + echo alternate content expect + test_cmp expect file + ) +' + +test_expect_success 'pulling into void does not remove new staged files' ' + git init cloned-staged-new + ( + cd cloned-staged-new + echo new tracked file newfile + git add newfile + git pull .. master + echo new tracked file expect + test_cmp expect newfile + ) +' + test_expect_success 'test . as a remote' ' git branch copy master -- 1.8.3.1.664.gae9f72a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name
Michael S. Tsirkin m...@redhat.com writes:
Verify that author name is not duplicated if it matches
sender, even if it is in utf8.
Small nit: if you make two patches out of it, add the tests first with
test_expect_failure. Then flip it to test_expect_success in the actual
code change. That makes it easy to verify that the test actually checks
the right thing, and that it was your code change that fixed it.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name
On Thu, Jun 20, 2013 at 02:41:37PM +0200, Thomas Rast wrote:
Michael S. Tsirkin m...@redhat.com writes:
Verify that author name is not duplicated if it matches
sender, even if it is in utf8.
Small nit: if you make two patches out of it, add the tests first with
test_expect_failure. Then flip it to test_expect_success in the actual
code change. That makes it easy to verify that the test actually checks
the right thing, and that it was your code change that fixed it.
I did this by reverting 1/2 and rerunning.
But applying in reverse order means bisect can give us
a setup where some tests fail, I thought it's a
good idea to avoid that.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name
Michael S. Tsirkin m...@redhat.com writes:
On Thu, Jun 20, 2013 at 02:41:37PM +0200, Thomas Rast wrote:
Michael S. Tsirkin m...@redhat.com writes:
Verify that author name is not duplicated if it matches
sender, even if it is in utf8.
Small nit: if you make two patches out of it, add the tests first with
test_expect_failure. Then flip it to test_expect_success in the actual
code change. That makes it easy to verify that the test actually checks
the right thing, and that it was your code change that fixed it.
I did this by reverting 1/2 and rerunning.
But applying in reverse order means bisect can give us
a setup where some tests fail, I thought it's a
good idea to avoid that.
That's why you need to test_expect_*failure* in the commit that adds the
tests -- essentially saying I know this is broken!.
Yes, it's a roundabout way. But splitting code and tests in the way you
just posted is equally roundabout, while not having the benefit that one
can check out the commit at patch 1 and verify that it is indeed broken
(showing up as still have known breakage).
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pull: pull into void by fast-forwarding from empty tree
On Thu, Jun 20, 2013 at 02:36:03PM +0200, Thomas Rast wrote: The logic for pulling into an unborn branch was originally designed to be used on a newly-initialized repository (d09e79c, git-pull: allow pulling into an empty repository, 2006-11-16). It thus did not initially deal with uncommitted changes in the unborn branch. The case of an _unstaged_ untracked file was fixed by by 4b3ffe5 (pull: do not clobber untracked files on initial pull, 2011-03-25). However, it still clobbered existing staged files, both when the file exists in the merged commit (it will be overwritten), and when it does not (it will be lost!). Yeah, in 4beffe5 I just assumed that using read-tree -m would give us the protections we need. But obviously I didn't think about the fact that we are not giving it enough information. We fix this by doing a two-way merge, where the current side of the merge is an empty tree, and the target side is HEAD (already updated to FETCH_HEAD at this point). This amounts to claiming that all work in the index was done vs. an empty tree, and thus all content of the index is precious. This seems like the correct fix; it is giving read-tree the right information to make the decision. Thanks for working on this. +test_expect_success 'pulling into void does not overwrite staged files' ' + git init cloned-staged-colliding + ( + cd cloned-staged-colliding + echo alternate content file + git add file + test_must_fail git pull .. master + echo alternate content expect + test_cmp expect file + ) +' + +test_expect_success 'pulling into void does not remove new staged files' ' + git init cloned-staged-new + ( + cd cloned-staged-new + echo new tracked file newfile + git add newfile + git pull .. master + echo new tracked file expect + test_cmp expect newfile + ) +' Do we want to also check the index state after each pull? In the former case, I think it should obviously represent a conflict. In the latter, we should be retaining the index contents of newfile. These are basic things that read-tree's two-way merge should get right (and are presumably tested elsewhere), but it might be worth confirming the desired behavior here in case somebody later tries to tweak this code path not to use read-tree. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name
On Thu, Jun 20, 2013 at 02:48:15PM +0200, Thomas Rast wrote:
Michael S. Tsirkin m...@redhat.com writes:
On Thu, Jun 20, 2013 at 02:41:37PM +0200, Thomas Rast wrote:
Michael S. Tsirkin m...@redhat.com writes:
Verify that author name is not duplicated if it matches
sender, even if it is in utf8.
Small nit: if you make two patches out of it, add the tests first with
test_expect_failure. Then flip it to test_expect_success in the actual
code change. That makes it easy to verify that the test actually checks
the right thing, and that it was your code change that fixed it.
I did this by reverting 1/2 and rerunning.
But applying in reverse order means bisect can give us
a setup where some tests fail, I thought it's a
good idea to avoid that.
That's why you need to test_expect_*failure* in the commit that adds the
tests -- essentially saying I know this is broken!.
Yes, it's a roundabout way. But splitting code and tests in the way you
just posted is equally roundabout, while not having the benefit that one
can check out the commit at patch 1 and verify that it is indeed broken
(showing up as still have known breakage).
I'll try to do it like this next time, I hope for now Junio
can take it as is.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Splitting a rev list into 2 sets
On Thu, Jun 20, 2013 at 6:14 AM, Francis Moreau francis.m...@gmail.com wrote: I'd like to write a script that would parse commits in one of my repo. Ideally this script should accept any revision ranges that git-rev-list would accept. This script should consider commits in master differently than the ones in others branches. To get the commit set which can't be reached by master (ie commits which are specific to branches other than master) I would do: # $@ is the range spec passed to the script git rev-list $@ ^master | check_other_commit But I don't know if it's possible to use a different git-rev-list command to get the rest of the commits, ie the ones that are reachable by the specified range and master. One way to do that is to record the first commit set got by the first rev-list command and check that the ones returned by git rev-list $@ are not in the record. But I'm wondering if someone can see another solution more elegant ? I do not know if I would call this elegant, but I think this codification of your One way to do that is at least small and mostly readable: git rev-list $@ |grep -v -f (git rev-list $@ ^master) Phil -- To unsubscribe from this list: send the line 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] pull: merge into unborn by fast-forwarding from empty tree
The logic for pulling into an unborn branch was originally designed to be used on a newly-initialized repository (d09e79c, git-pull: allow pulling into an empty repository, 2006-11-16). It thus did not initially deal with uncommitted changes in the unborn branch. The case of an _unstaged_ untracked file was fixed by 4b3ffe5 (pull: do not clobber untracked files on initial pull, 2011-03-25). However, it still clobbered existing staged files, both when the file exists in the merged commit (it will be overwritten), and when it does not (it will be deleted). We fix this by doing a two-way merge, where the current side of the merge is an empty tree, and the target side is HEAD (already updated to FETCH_HEAD at this point). This amounts to claiming that all work in the index was done vs. an empty tree, and thus all content of the index is precious. Reported-by: Stefan Schüßler m...@stefanschuessler.de Signed-off-by: Thomas Rast tr...@inf.ethz.ch --- Jeff King p...@peff.net writes: Do we want to also check the index state after each pull? In the former case, I think it should obviously represent a conflict. In the latter, we should be retaining the index contents of newfile. These are basic things that read-tree's two-way merge should get right (and are presumably tested elsewhere), but it might be worth confirming the desired behavior here in case somebody later tries to tweak this code path not to use read-tree. Right, good point. I also reworded the subject and message somewhat to read better. git-pull.sh | 9 - t/t5520-pull.sh | 29 + 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/git-pull.sh b/git-pull.sh index 638aabb..1f84383 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -266,10 +266,17 @@ case $merge_head in ;; esac +# Pulling into unborn branch: a shorthand for branching off +# FETCH_HEAD, for lazy typers. if test -z $orig_head then git update-ref -m initial pull HEAD $merge_head $curr_head - git read-tree -m -u HEAD || exit 1 + # Two-way merge: we claim the index is based on an empty tree, + # and try to fast-forward to HEAD. This ensures we will not + # lose index/worktree changes that the user already made on + # the unborn branch. + empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904 + git read-tree -m -u $empty_tree HEAD || exit 1 exit fi diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 6af6c63..ed4d9c8 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -57,6 +57,35 @@ test_expect_success 'pulling into void does not overwrite untracked files' ' ) ' +test_expect_success 'pulling into void does not overwrite staged files' ' + git init cloned-staged-colliding + ( + cd cloned-staged-colliding + echo alternate content file + git add file + test_must_fail git pull .. master + echo alternate content expect + test_cmp expect file + git cat-file blob :file file.index + test_cmp expect file.index + ) +' + + +test_expect_success 'pulling into void does not remove new staged files' ' + git init cloned-staged-new + ( + cd cloned-staged-new + echo new tracked file newfile + git add newfile + git pull .. master + echo new tracked file expect + test_cmp expect newfile + git cat-file blob :newfile newfile.index + test_cmp expect newfile.index + ) +' + test_expect_success 'test . as a remote' ' git branch copy master -- 1.8.3.1.664.gae9f72a -- To unsubscribe from this list: send the line 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: Splitting a rev list into 2 sets
On Thu, Jun 20, 2013 at 1:26 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Francis Moreau wrote: To get the commit set which can't be reached by master (ie commits which are specific to branches other than master) I would do: # $@ is the range spec passed to the script git rev-list $@ ^master | check_other_commit But I don't know if it's possible to use a different git-rev-list command to get the rest of the commits, ie the ones that are reachable by the specified range and master. One way to do that is to record the first commit set got by the first rev-list command and check that the ones returned by git rev-list $@ are not in the record. I don't fully understand your query, because almost anything is possible with rev-list: $ git rev-list foo..bar master # reachable from master, bar, not foo What I _suspect_ you're asking is for help when you can't construct this foo..bar master programmatically (or when you cannot express your criterion as arguments to rev-list). You want an initial commit set, and filter it at various points in your program using various criteria, right? Yes, I would like to be sure that I haven't missed some magic syntax for rev-list before going further in my poor man solution :) Basically I have an initial set (or can be several different sets) expressed as a revision specification described by git-rev-list man page. I just want to find the common set of commit which are part of the initial sets *and* is reachable by master. I would write it: git rev-list $@ --and master In that case, I'd suggest something like this: Thanks for the details example. -- Francis -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
On Thu, Jun 20, 2013 at 03:06:01PM +0200, Thomas Rast wrote: +test_expect_success 'pulling into void does not overwrite staged files' ' + git init cloned-staged-colliding + ( + cd cloned-staged-colliding + echo alternate content file + git add file + test_must_fail git pull .. master + echo alternate content expect + test_cmp expect file + git cat-file blob :file file.index + test_cmp expect file.index + ) +' I naively would have expected this to leave us in a conflicted state over file. But I guess read-tree just rejects it, because we are not doing a real three-way merge. I'm not sure it is that big a deal; this is more about safety than about creating a conflicted/resolvable state. -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: Splitting a rev list into 2 sets
Hi, On Thu, Jun 20, 2013 at 3:04 PM, Phil Hord phil.h...@gmail.com wrote: On Thu, Jun 20, 2013 at 6:14 AM, Francis Moreau francis.m...@gmail.com wrote: I'd like to write a script that would parse commits in one of my repo. Ideally this script should accept any revision ranges that git-rev-list would accept. This script should consider commits in master differently than the ones in others branches. To get the commit set which can't be reached by master (ie commits which are specific to branches other than master) I would do: # $@ is the range spec passed to the script git rev-list $@ ^master | check_other_commit But I don't know if it's possible to use a different git-rev-list command to get the rest of the commits, ie the ones that are reachable by the specified range and master. One way to do that is to record the first commit set got by the first rev-list command and check that the ones returned by git rev-list $@ are not in the record. But I'm wondering if someone can see another solution more elegant ? I do not know if I would call this elegant, but I think this codification of your One way to do that is at least small and mostly readable: git rev-list $@ |grep -v -f (git rev-list $@ ^master) Yes, thanks. But I wanted to be sure that git-rev-list can't display the intersection of several sets before going forward. -- Francis -- To unsubscribe from this list: send the line 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] clone: regression in error messages in master
Hi, So this should explain the problem: # using v1.8.3.1 $ git clone https://google.com Cloning into 'google.com'... fatal: repository 'https://google.com/' not found # using master $ git clone https://google.com Cloning into 'google.com'... fatal: repository 'https://google.com/' not found fatal: Reading from helper 'git-remote-https' failed To figure out where the regression was coming from, I ran a bisect with this script: #!/bin/sh make clean make -j 8 cd t sh -v -i clone-message.sh where clone-message.sh is: test_description=clone-message . ./test-lib.sh test_expect_success setup ' rm -fr .git test_create_repo src ( cd src file git add file git commit -m initial echo 1 file git add file git commit -m updated ) ' test_expect_success 'clone invalid URL' ' rm -fr dst test_must_fail git clone https://google.com 2msg test_i18ngrep repository .* not found msg ! test_i18ngrep git-remote-https msg ' test_done The bisect pointed me to: 81d340d4 (transport-helper: report errors properly, 2013-04-10). $ git clone https://google.com Cloning into 'google.com'... fatal: https://google.com/info/refs?service=git-upload-pack not found: did you run git update-server-info on the server? fatal: Reading from remote helper failed What?! Okay, the last Reading from remote helper failed was introduced by this commit; my clone-message.sh has a bug. So I commented out the first test_i18ngrep and ran it. Result: c096955 (transport-helper: mention helper name when it dies, 2013-04-10). This is not the real culprit: it just changed the message string that 81d340d4 originally introduced. Okay, so am I reporting a valid bug? Going through remote-curl, I can see that it dies in remote-curl.c:213 if HTTP_TARGET_MISSING. If that is the case, what is the point of printing the second message about the remote helper program not being present? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Jeff King p...@peff.net writes:
On Thu, Jun 20, 2013 at 03:06:01PM +0200, Thomas Rast wrote:
+test_expect_success 'pulling into void does not overwrite staged files' '
+git init cloned-staged-colliding
+(
+cd cloned-staged-colliding
+echo alternate content file
+git add file
+test_must_fail git pull .. master
+echo alternate content expect
+test_cmp expect file
+git cat-file blob :file file.index
+test_cmp expect file.index
+)
+'
I naively would have expected this to leave us in a conflicted state
over file. But I guess read-tree just rejects it, because we are not
doing a real three-way merge. I'm not sure it is that big a deal; this
is more about safety than about creating a conflicted/resolvable state.
Note that the test_must_fail essentially tests that the merge is rejected.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Splitting a rev list into 2 sets
Francis Moreau francis.m...@gmail.com writes:
Hello,
I'd like to write a script that would parse commits in one of my repo.
Ideally this script should accept any revision ranges that
git-rev-list would accept.
This script should consider commits in master differently than the
ones in others branches.
To get the commit set which can't be reached by master (ie commits
which are specific to branches other than master) I would do:
# $@ is the range spec passed to the script
git rev-list $@ ^master | check_other_commit
But I don't know if it's possible to use a different git-rev-list
command to get the rest of the commits, ie the ones that are reachable
by the specified range and master.
One way to do that is to record the first commit set got by the first
rev-list command and check that the ones returned by git rev-list $@
are not in the record.
But I'm wondering if someone can see another solution more elegant ?
I think there's a cute way. Suppose your arguments are of the form
p1 p2 ... --not n1 n2 ...
that is each pX is positive, and each nX is negative. Then as you
observed, building the difference with master is easy: just add it to
the negative args.
Intersecting with master is harder, because you don't know what parts of
it (if any) are in the range. But the --boundary option can help: these
are the commits where the positive and negative ranges first met, and
prevented the walk from continuing.
So the part of master reachable from p1, p2, etc. is exactly the set of
boundary commits of 'p1 p2 ... ^master'. And on top of that, excluding
the parts reachable from the n's is easy. So you can do:
positive=$(git rev-parse $@ | grep -v '^\^')
negative=$(git rev-parse $@ | grep '^\^')
boundary=$(git rev-list --boundary $positive ^master | sed -n 's/^-//p')
# the intersection is
git rev-list $boundary $negative
I haven't tested it much, however.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Request] Git reset should be able to ignore file permissions
@Matthieu Ok, I'm replacing with Reset only files which actually changed (not those with only stat information change) @Junio I'm not sure what you're asking, sorry, I'm not able to understand your question. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Thomas Rast tr...@inf.ethz.ch writes:
Jeff King p...@peff.net writes:
On Thu, Jun 20, 2013 at 03:06:01PM +0200, Thomas Rast wrote:
+test_expect_success 'pulling into void does not overwrite staged files' '
+ git init cloned-staged-colliding
+ (
+ cd cloned-staged-colliding
+ echo alternate content file
+ git add file
+ test_must_fail git pull .. master
+ echo alternate content expect
+ test_cmp expect file
+ git cat-file blob :file file.index
+ test_cmp expect file.index
+ )
+'
I naively would have expected this to leave us in a conflicted state
over file. But I guess read-tree just rejects it, because we are not
doing a real three-way merge. I'm not sure it is that big a deal; this
is more about safety than about creating a conflicted/resolvable state.
Note that the test_must_fail essentially tests that the merge is rejected.
Bah, no it doesn't, a conflicting merge also returns a nonzero status.
Sigh.
If you meant we should actually conflict, I'm not sure what options
there would be other than actually calling a merge driver. And we could
actually do this like so (it'll obviously break the tests):
diff --git i/git-pull.sh w/git-pull.sh
index 1f84383..b3d36a8 100755
--- i/git-pull.sh
+++ w/git-pull.sh
@@ -276,7 +276,7 @@ then
# lose index/worktree changes that the user already made on
# the unborn branch.
empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
- git read-tree -m -u $empty_tree HEAD || exit 1
+ git merge-recursive $empty_tree -- $(git write-tree) HEAD || exit 1
exit
fi
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
On Thu, Jun 20, 2013 at 03:33:34PM +0200, Thomas Rast wrote: I naively would have expected this to leave us in a conflicted state over file. But I guess read-tree just rejects it, because we are not doing a real three-way merge. I'm not sure it is that big a deal; this is more about safety than about creating a conflicted/resolvable state. Note that the test_must_fail essentially tests that the merge is rejected. Bah, no it doesn't, a conflicting merge also returns a nonzero status. Sigh. If you meant we should actually conflict, Yes, that's what I meant. I'm not sure what options there would be other than actually calling a merge driver. And we could actually do this like so (it'll obviously break the tests): I'd rather not invoke a merge driver directly if we can avoid it. I think you could get rid of this special code-path entirely if you just lied to the git-merge and said the ancestor and current tree are fake commits with an empty tree, and then followed the usual path. But that lying through git-merge is ugly and complicated (and is more or less what you're doing with the merge-recursive patch here). diff --git i/git-pull.sh w/git-pull.sh index 1f84383..b3d36a8 100755 --- i/git-pull.sh +++ w/git-pull.sh @@ -276,7 +276,7 @@ then # lose index/worktree changes that the user already made on # the unborn branch. empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904 - git read-tree -m -u $empty_tree HEAD || exit 1 + git merge-recursive $empty_tree -- $(git write-tree) HEAD || exit 1 I don't think there is any advantage to using merge-recursive over read-tree here, in the sense that there cannot be any interesting content-level merging going on (our ancestor is the empty tree, so we know that differing content cannot be resolved). So I think you could just use read-tree with a 3-way merge, but I cannot seem to provoke it to leave a conflict. Hrm. I also noticed that the procedure in this code path is: 1. Update HEAD with the fetched ref. 2. Checkout the contents with read-tree. I wonder if it would make sense to update HEAD only _after_ we had resolved successfully. As it is now, you are left in a weird state where pull has reported failure, but we actually update the HEAD (and git status afterwards reflects that you are building on top of the pulled HEAD). -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: Splitting a rev list into 2 sets
Francis Moreau wrote: Basically I have an initial set (or can be several different sets) expressed as a revision specification described by git-rev-list man page. I just want to find the common set of commit which are part of the initial sets *and* is reachable by master. That's just a generic list intersection between [a, b, c] and [d, e, f] no? [a, b, c] is a list you built up somehow, and [d, e, f] comes from $(git rev-list master), right? You could go about determining the revision walk boundaries and combine them to set up a revision walk to splice the master line, but what is the point of that? You'll only be painting yourself into a design-corner (you won't be able to do other kinds of filtering), and going around your head to touch your nose. You precisely want list intersection: so write an efficient list intersection in the language of your choice. Why is it a poor man's solution? If anything, your convoluted rev-list solution will probably be more complicated, slower, and bug-ridden. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Thomas Rast tr...@inf.ethz.ch writes:
if test -z $orig_head
then
git update-ref -m initial pull HEAD $merge_head $curr_head
- git read-tree -m -u HEAD || exit 1
+ # Two-way merge: we claim the index is based on an empty tree,
+ # and try to fast-forward to HEAD. This ensures we will not
+ # lose index/worktree changes that the user already made on
+ # the unborn branch.
+ empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ git read-tree -m -u $empty_tree HEAD || exit 1
exit
fi
I just noticed that I broke the chaining here, so don't apply this
just yet. I'll resend once we settle on the best strategy to generate
conflicts (or not).
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Jeff King p...@peff.net writes:
On Thu, Jun 20, 2013 at 03:33:34PM +0200, Thomas Rast wrote:
I naively would have expected this to leave us in a conflicted state
over file. But I guess read-tree just rejects it, because we are not
doing a real three-way merge. I'm not sure it is that big a deal; this
is more about safety than about creating a conflicted/resolvable state.
Note that the test_must_fail essentially tests that the merge is rejected.
Bah, no it doesn't, a conflicting merge also returns a nonzero status.
Sigh.
If you meant we should actually conflict,
Yes, that's what I meant.
[...]
diff --git i/git-pull.sh w/git-pull.sh
index 1f84383..b3d36a8 100755
--- i/git-pull.sh
+++ w/git-pull.sh
@@ -276,7 +276,7 @@ then
# lose index/worktree changes that the user already made on
# the unborn branch.
empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-git read-tree -m -u $empty_tree HEAD || exit 1
+git merge-recursive $empty_tree -- $(git write-tree) HEAD || exit 1
I don't think there is any advantage to using merge-recursive over
read-tree here, in the sense that there cannot be any interesting
content-level merging going on (our ancestor is the empty tree, so we
know that differing content cannot be resolved).
So I think you could just use read-tree with a 3-way merge, but I cannot
seem to provoke it to leave a conflict. Hrm.
I guess read-tree doesn't consider that its job; it leaves the conflict
in the index alright for me if I do this:
git-pull.sh | 4 ++--
t/t5520-pull.sh | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git i/git-pull.sh w/git-pull.sh
index 1f84383..4047d02 100755
--- i/git-pull.sh
+++ w/git-pull.sh
@@ -275,8 +275,8 @@ then
# and try to fast-forward to HEAD. This ensures we will not
# lose index/worktree changes that the user already made on
# the unborn branch.
- empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
- git read-tree -m -u $empty_tree HEAD || exit 1
+ empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ git read-tree -m -u $empty_tree $(git write-tree) HEAD || exit 1
exit
fi
But it won't write the conflict markers in the worktree.
On the topic of do we want to conflict: one issue is that we don't
have a prior state to go to, since it was never committed. Not even the
implicit empty tree can be passed to 'reset --keep'. So it might be
better to *avoid* creating conflict hunks -- and fail -- so as to avoid
giving the user a state that is hard to back out of.
In the same spirit I would also support this:
I wonder if it would make sense to update HEAD only _after_ we had
resolved successfully. As it is now, you are left in a weird state where
pull has reported failure, but we actually update the HEAD (and git
status afterwards reflects that you are building on top of the pulled
HEAD).
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t7400: test of UTF-8 submodule names pass under Mac OS
submodules with names using UTF-8 need core.precomposeunicode true under Mac OS X, set it in the TC. Improve the portability: Not all shells on all OS may understand literal UTF-8 strings. Use a help variable filled by printf, as we do it in e.g. t0050. strange names can be called UTF-8, rephrase the heading Signed-off-by: Torsten Bögershausen tbo...@web.de --- I wasn't fast enough to catch it on pu: fg/submodule-non-ascii-path t/t7400-submodule-basic.sh | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 0376370..fedfa5b 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -889,16 +889,19 @@ test_expect_success 'submodule deinit fails when submodule has a .git directory test -n $(git config --get-regexp submodule\.example\.) ' -test_expect_success 'submodule with strange name works å äö' ' - mkdir å äö +svname=$(printf '\303\245 \303\244\303\266') +test_expect_success 'submodule with UTF-8 name' ' + mkdir $svname ( - cd å äö + cd $svname git init touch sub git add sub git commit -m init sub ) - git submodule add /å äö - test -n $(git submodule | grep å äö) + git config core.precomposeunicode true + git submodule add /$svname + git submodule 2 + test -n $(git submodule | grep $svname) ' test_done -- 1.8.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
Daily deb/rpm builds of git
Hi all, To make it easier to help test changes that are in-flight, I've started using and publishing daily snapshots of -next as packages for Ubuntu 12.10, 13.04 and 13.10, Debian 7.0 and Fedora 17 and 18. If anyone else wants to use these, they can be found on launchpad and openSUSE's build service. They could also be useful to give to people who report issues, as a simple way to check against the latest git development. The .deb packages use Debian's packaging, with only the version numbers changed. The .rpm packages use Fedora's packaging, with only the version numbers changed. So neither are 'vanilla' git, but what people are used to on the respective operating systems. For Ubuntu: sudo apt-add-repository ppa:dennis/git-next Ubuntu alternative, because launchpad is seeing long delays (12.10, quantal) echo 'deb http://download.opensuse.org/repositories/home:/seveas:/git-next/xUbuntu_12.10/ ./' | sudo tee -a /etc/apt/sources.list (13.04, raring) echo 'deb http://download.opensuse.org/repositories/home:/seveas:/git-next/xUbuntu_13.04/ ./' | sudo tee -a /etc/apt/sources.list For Debian 7.0: echo 'deb http://download.opensuse.org/repositories/home:/seveas:/git-next/Debian_7.0/ ./' | sudo tee -a /etc/apt/sources.list For Fedora: (17) sudo wget http://download.opensuse.org/repositories/home:/seveas:/git-next/Fedora_17/home:seveas:git-next.repo -O /etc/yum.repos.d/git-next.repo (18) sudo wget http://download.opensuse.org/repositories/home:/seveas:/git-next/Fedora_18/home:seveas:git-next.repo -O /etc/yum.repos.d/git-next.repo -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line 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] pull: pull into void by fast-forwarding from empty tree
Thomas Rast wrote: diff --git a/git-pull.sh b/git-pull.sh index 638aabb..1f84383 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -266,10 +266,17 @@ case $merge_head in ;; esac +# Pulling into unborn branch: a shorthand for branching off +# FETCH_HEAD, for lazy typers. if test -z $orig_head then git update-ref -m initial pull HEAD $merge_head $curr_head - git read-tree -m -u HEAD || exit 1 + # Two-way merge: we claim the index is based on an empty tree, + # and try to fast-forward to HEAD. This ensures we will not + # lose index/worktree changes that the user already made on + # the unborn branch. + empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904 Perhaps replace this magic with $(git hash-object -t tree /dev/null) or $(git mktree /dev/null)? -- To unsubscribe from this list: send the line 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: Splitting a rev list into 2 sets
Hi,
On Thu, Jun 20, 2013 at 3:20 PM, Thomas Rast tr...@inf.ethz.ch wrote:
Francis Moreau francis.m...@gmail.com writes:
But I'm wondering if someone can see another solution more elegant ?
I think there's a cute way. Suppose your arguments are of the form
Really nice !
p1 p2 ... --not n1 n2 ...
that is each pX is positive, and each nX is negative. Then as you
observed, building the difference with master is easy: just add it to
the negative args.
I didn't know that git-rev-parse could be used to transform any range
specification into that form (p1 p2 .. -not n1 n2..)
Intersecting with master is harder, because you don't know what parts of
it (if any) are in the range. But the --boundary option can help: these
are the commits where the positive and negative ranges first met, and
prevented the walk from continuing.
So the part of master reachable from p1, p2, etc. is exactly the set of
boundary commits of 'p1 p2 ... ^master'. And on top of that, excluding
the parts reachable from the n's is easy. So you can do:
Really clever.
positive=$(git rev-parse $@ | grep -v '^\^')
negative=$(git rev-parse $@ | grep '^\^')
boundary=$(git rev-list --boundary $positive ^master | sed -n 's/^-//p')
# the intersection is
git rev-list $boundary $negative
I think there's a minor issue here, when boundary is empty. Please
correct me if I'm wrong but I think it can only happen if positive is
simply master or a subset of master. In that case I think the solution
is just make boundary equal to positive:
# the intersection is
git rev-list ${boundary:-$positive} $negative
Now I'm going to see if that solution is faster than the initial one.
Great Thanks
--
Francis
--
To unsubscribe from this list: send the line 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] send-email: Fix documentation of --signed-off-by-cc option
Namhyung Kim wrote: diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 40a9a9a..5694d98 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -238,7 +238,7 @@ Automating the value of 'sendemail.identity'. --[no-]signed-off-by-cc:: - If this is set, add emails found in Signed-off-by: or Cc: lines to the + If this is set, add emails found in Signed-off-by: lines to the cc list. Default is the value of 'sendemail.signedoffbycc' configuration value; if that is unspecified, default to --signed-off-by-cc. Correct, but I'd urge you to step back and look at all the options. Cc lines in the body (called bodycc) are parsed: see git-send-email.perl:1361. By default, everyone (cc, author, self, sob, bodycc) is cc'ed by default. You can turn off each of these individually using --supress-cc which can take values (all|cccmd|cc|author|self|sob|body|bodycc). As you can see from git-send-email.perl:400, the real purpose of --signed-off-by-cc is to override a sendemail.suppresscc=sob; however, it can't override any stronger values of suppresscc (body, all). So, it's mostly a no-option, and there is no corresponding --[no-]bodycc-cc. Overall, the way out of this horrible mess of options is to deprecate --[no-]signed-off-by-cc, and modify --supress-cc to have a --no-suppress-cc counterpart, just like --[no-]suppress-from. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
Michael Haggerty mhag...@alum.mit.edu writes: But currently only the main packed ref cache can be locked, so it would be possible for lock_packed_refs() to use the static packlock instance for locking. Perhaps I am missing something from the previous discussions, but I am having trouble understanding the main packed ref cache part of the above. main as opposed to...? Is it envisioned that later somebody can lock one subpart while another can lock a different and non-overlapping subpart, to make changes independently, and somehow their non-overlapping changes will be consolidated into a single consistent result? -- To unsubscribe from this list: send the line 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] peel_onion(): add support for rev^{tag}
Richard Hansen wrote:
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -677,6 +677,8 @@ static int peel_onion(const char *name, int len, unsigned
char *sha1)
sp++; /* beginning of type name, or closing brace for empty */
if (!strncmp(commit_type, sp, 6) sp[6] == '}')
expected_type = OBJ_COMMIT;
+ else if (!strncmp(tag_type, sp, 3) sp[3] == '}')
+ expected_type = OBJ_TAG;
Interesting.
gitrevisions(7) implies that rev^{tag} should work, but before now
it did not:
The wording (especially of rev^{}) special-cases tags as objects to
dereference.
$ git rev-parse --verify v1.8.3.1^{}^{object}
362de916c06521205276acb7f51c99f47db94727
$ git rev-parse --verify v1.8.3.1^{}^{tag}
error: v1.8.3.1^{}^{tag}: expected tag type, but the object dereferences
to tree type
fatal: Needed a single revision
And the points out the problem: while ^{object} means expect
OBJ_ANY, ^{} means expect OBJ_NONE. What does that even mean? See
sha1_name.c:704 where this is handled sneakily: it just calls
deref_tag(), bypassing peel_to_type() altogether. If anything, ^{} is
already a poor-man's version of your ^{tag}; the reason it's a
poor-man's version is precisely because it doesn't error out when
rev isn't a tag, while your ^{tag} does. I would argue that ^{} is
very poorly done and must be deprecated in favor of your ^{tag}. What
is the point of using ^{} if you can't even be sure that what you get
is a deref? peel_to_type() already does the right thing by not using
deref_tag(), and explicitly checking.
Your commit message needs some tweaking, but I'm happy with your patch
otherwise.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
Do you need a genuine Loan to settle your bills and start up a good business? Kindly contact us now with your details to get a secured Loan at a low rate of 2% per Annu, Quick send your details via: stella.fina...@usa.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
On 06/20/2013 07:11 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: But currently only the main packed ref cache can be locked, so it would be possible for lock_packed_refs() to use the static packlock instance for locking. Perhaps I am missing something from the previous discussions, but I am having trouble understanding the main packed ref cache part of the above. main as opposed to...? main as opposed to submodule. Is it envisioned that later somebody can lock one subpart while another can lock a different and non-overlapping subpart, to make changes independently, and somehow their non-overlapping changes will be consolidated into a single consistent result? No, the scenario would be that a git process wants to change a reference in a submodule directly, as opposed to starting another git process within the submodule, as I believe is done now. Maybe it's too far-fetched even to consider... Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
On 06/20/2013 01:55 PM, Jeff King wrote: On Thu, Jun 20, 2013 at 09:49:51AM +0200, Michael Haggerty wrote: [I just noticed that lock_ref_sha1_basic() leaks a struct lock_file every time that it is called.] I noticed that recently, too. I have a patch series about 90% complete that abstracts the tempfile handling (the ultimate goal of which is to optionally clean up tmp_* files in the objects/ directory). It refactors the lockfile cleanup, and it would not be too hard to have a committed or rolled-back lockfile actually remove itself from the to clean at exit list. Which would make it perfectly safe to have a lockfile as an automatic variable as long as you commit or rollback before leaving the function. Cool, then I won't work on that. You might also have to make the lockfile list into a doubly-linked-list to avoid having to do a linear scan to find the entry to delete, unless the total number of entries is known to remain small. Please CC me on the patch series when it is done. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] peel_onion(): add support for rev^{tag}
Richard Hansen rhan...@bbn.com writes:
Barfing on non-tags is the feature this adds. It's otherwise useless,
just like object^{object} is useless except to barf when object
doesn't exist.
Thanks.
I could buy that. And after re-reading the proposed log message,
you do not quite have anything to say that. Instead, you have this:
Note that rev^{tag} is not the same as rev^{object} when rev is
not a tag:
$ git rev-parse --verify v1.8.3.1^{}^{object}
362de916c06521205276acb7f51c99f47db94727
$ git rev-parse --verify v1.8.3.1^{}^{tag}
error: v1.8.3.1^{}^{tag}: expected tag type, but the object deref...
fatal: Needed a single revision
The latter peels v1.8.3.1 to a non-tag (i.e. a commit) and then asks
to peel that commit to a tag, which will of course fail, but that is
not a good example.
Perhaps something like this instead.
Note that rev^{tag} can be used to make sure rev names a tag:
$ git rev-parse --verify v1.8.3.1^{tag}
$ git rev-parse --verify master^{tag}
The former succeeds, while the latter fails.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Johannes Sixt j.s...@viscovery.net writes: Am 6/20/2013 9:56, schrieb Peter Krefting: Junio C Hamano: But my understanding is that the reordering using printf() is the mechanism we suggest l10n folks to use when the order of parameters given to printf does not match the preferred word order in the message in their language. It's documented in the gettext manual, and seems to be used in the zh_CN.po to change the word order in quite a few places. It is fine to use %n$ in translated strings as long as gettext is enabled only on systems that have a sufficiently capable printf and these formats are not used in the source code. But you can't have this string: Splitting a commit while rebasing branch '%2$s' on '%3$s'. neither in the template nor in the translation, because the numbers must begin at 1 (and must be used without gaps). Did any message we saw in the patch (and the discussion to possibly improve it) need to have such a format string, or are you pointing out a common gotcha we may want to warn translators about in po/README? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
Michael Haggerty mhag...@alum.mit.edu writes: On 06/20/2013 07:11 PM, Junio C Hamano wrote: Perhaps I am missing something from the previous discussions, but I am having trouble understanding the main packed ref cache part of the above. main as opposed to...? main as opposed to submodule. I see. No, the scenario would be that a git process wants to change a reference in a submodule directly, as opposed to starting another git process within the submodule, as I believe is done now. Maybe it's too far-fetched even to consider... Perhaps. But the singleton lock because we only handle main packed ref cache does not prevent us from doing so later, so I think v3 is OK. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Thomas Rast tr...@inf.ethz.ch writes: The logic for pulling into an unborn branch was originally designed to be used on a newly-initialized repository (d09e79c, git-pull: allow pulling into an empty repository, 2006-11-16). It thus did not initially deal with uncommitted changes in the unborn branch. The case of an _unstaged_ untracked file was fixed by 4b3ffe5 (pull: do not clobber untracked files on initial pull, 2011-03-25). However, it still clobbered existing staged files, both when the file exists in the merged commit (it will be overwritten), and when it does not (it will be deleted). Perhaps making sure the index is empty is sufficient, then? -- To unsubscribe from this list: send the line 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] t7400: test of UTF-8 submodule names pass under Mac OS
Torsten Bögershausen tbo...@web.de writes: submodules with names using UTF-8 need core.precomposeunicode true under Mac OS X, set it in the TC. I take that TC stands for test case? +test_expect_success 'submodule with UTF-8 name' ' + mkdir $svname ( - cd å äö + cd $svname git init touch sub git add sub git commit -m init sub ) While we are at it, let's fix this broken chain. - git submodule add /å äö - test -n $(git submodule | grep å äö) + git config core.precomposeunicode true and use test_config here. + git submodule add /$svname + git submodule 2 + test -n $(git submodule | grep $svname) ' test_done Will queue with an obvious fix-up. Thanks for catching. -- 8 -- From: Torsten Bögershausen tbo...@web.de submodules with names using UTF-8 need core.precomposeunicode true under Mac OS X, set it in the test case. Improve the portability: - Not all shells on all OS may understand literal UTF-8 strings. - Use a help variable filled by printf, as we do it in e.g. t0050. strange names can be called UTF-8, rephrase the heading. While at it, unbreak -chain in the test, and use test_config. Signed-off-by: Torsten Bögershausen tbo...@web.de Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7400-submodule-basic.sh | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index d5743ee..7e23421 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -868,16 +868,19 @@ test_expect_success 'submodule deinit fails when submodule has a .git directory test -n $(git config --get-regexp submodule\.example\.) ' -test_expect_success 'submodule with strange name works å äö' ' - mkdir å äö +test_expect_success 'submodule with UTF-8 name' ' + svname=$(printf \303\245 \303\244\303\266) + mkdir $svname ( - cd å äö + cd $svname git init - touch sub - git add sub + sub + git add sub git commit -m init sub - ) - git submodule add /å äö - test -n $(git submodule | grep å äö) + ) + test_config core.precomposeunicode true + git submodule add ./$svname + git submodule 2 + test -n $(git submodule | grep $svname) ' test_done -- 1.8.3.1-674-g24fae08 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name
Michael S. Tsirkin m...@redhat.com writes: Verify that author name is not duplicated if it matches sender, even if it is in utf8. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Hmph. There seems to be something wrong in the original message I received from you (look at Cc: line I have above, which was copied by my MUA from the message I am responding to). Visiting http://article.gmane.org/gmane.comp.version-control.git/228485/raw shows this gem: Cc: =?us-ascii?B?PT9VVEYtOD9xP1NaRURFUj0yMEc9QzM9QTFib3I/PQ==?= sze...@ira.uka.de Somebody is wrapping what =?UTF-8?q?... which you had already wrapped into =?us-ascii?B...??? In any case, I think Thomas's first expect failure and then flip it, if you add test and fix as separate patches is a good idea, and the change between the previous one and this seems to be only the last part of this test, so I'll tweak what I already have from the previous on 'pu' locally and push the result out. Please double check the result. Thanks. t/t9001-send-email.sh | 14 ++ 1 file changed, 14 insertions(+) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 9f46f22..020acc4 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -956,6 +956,20 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' ' grep ^From: Füñný Nâmé odd_?=m...@example.com msgtxt1 ' +test_expect_success $PREREQ 'utf8 sender is not duplicated' ' + clean_fake_sendmail + test_commit weird_sender + test_when_finished git reset --hard HEAD^ + git commit --amend --author Füñný Nâmé odd_?=m...@example.com + git format-patch --stdout -1 funny_name.patch + git send-email --from=Füñný Nâmé odd_?=m...@example.com \ + --to=nob...@example.com \ + --smtp-server=$(pwd)/fake.sendmail \ + funny_name.patch + grep ^From: msgtxt1 msgfrom + test_line_count = 1 msgfrom +' + test_expect_success $PREREQ 'sendemail.composeencoding works' ' clean_fake_sendmail git config sendemail.composeencoding iso-8859-1 -- To unsubscribe from this list: send the line 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: Feature request: fetch --prune by default
I would use the config feature to turn on --prune for fetch, and was surprised that it wasn't available - I hit this thread because I figured I somehow missed it in the config docs. Having both global and local settings seems nice. -- View this message in context: http://git.661346.n2.nabble.com/Feature-request-fetch-prune-by-default-tp7563241p7590048.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: Decouple `simple` from `upstream` completely, and change it to mean `current` with a safety feature: a `push` and `pull` should not be asymmetrical in the special case of central workflows. Double negation confused my parser. 'push' and 'pull' should be kept symmetrical in central workflows? They're not the same thing. It is very much intentional and intended: the safety net is not to ensure that the push and pull are symmetrical (i.e. among other things, error out if branch.$branch.merge is unset), but rather ensure that the push and pull are never asymmetrical. H not to ensure that the push and pull are symmetrical rather ensure that the push and pull are never asymmetrical. They still talk the same thing to me. What am I missing? Am I being clueless, or is there something else going on? Your among other things, after reading it three times, unfortunately did not help clarify anything to me, so perhaps somebody other than me (or you for that matter) who is more clueful can help find a different way to explain the difference you are trying to express to me. Help, anybody? Provided that we would want to keep the Push the current one to the same name but you have to have it set up as your integration source safety for central workflow (which I am starting to think we should), we would want something like this on top of your entire series, I think. The behaviour change can be seen in the revert of one test you made to the test that expects simple to fail due to the safety. Now I'd like to question what you are labelling as safety. What is the consequence of erroring out when branch.$branch.merge is unset when pushing using `upstream`? Nothing noteworthy. I wasn't suggesting to change what `upstream` does at all. If you do not have a `upstream` configured, we do not know what branch you are integrating with, and the operation has failed in the code even before we started adding triangular. I do not see a reason to change that in the triangular world; `upstream` is about the central workflow as you originally wrote in the config.txt patch in this series. The name of the branch the repository you fetch from and integrate with does not have anything to do with the name you want to push your derived work as to a different repository I thought we already discussed and agreed on this point. http://thread.gmane.org/gmane.comp.version-control.git/227028/focus=227313 The conclusion is that using push.default=`upstream` is meaningless when you are using a triangular workflow. If you are using a centralized workflow, and if a branch does not have branch.*.merge configured, we do not know to which branch you are pushing it back, so we error out. What I am changing from the patch you posted with the how about this on top patch back to the current behaviour is what 'simple' does for centralized workflow. I didn't want to contaminate this series with an unrelated improvement to `upstream` I think we share that, and it is not just `upstream`, but also `simple` when it is applied to folks who employ a centralized workflow. The safety we need to keep is the one we have had since day one of introducing 'simple' for them. When you are doing a centralized workflow, 'simple' was defined to be 'upstream' with added restriction that the branch at the remote you integrate with must have the same name as the current branch you are pushing, i.e. in [branch frotz] merge = refs/heads/$branch the value of $branch must be 'frotz'; otherwise 'simple' errors out. http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=196199 Now, no existing series has casted in stone the best behaviour for `simple` in a triangular workflow. With this series (and also with my fixup patch I sent last night), it is defined to act as `current`, but it may need a bit more safety to help new users avoid pushing branches they did not intend to (perhaps pushing out `current` only when the branch with the same name already exists at the destination? I dunno). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] log: --author-date-order
Jeff King p...@peff.net writes:
Or we could extend parse_commit() API to take an optional commit
info slab to store not just author date but other non-essential
stuff like people's names, and we arrange that extended API to be
triggered when we know --author-date-order is in effect?
I like the latter option. It takes a non-trivial amount of time to load
the commits from disk, and now we are potentially doing it 2 or 3 times
for a run (once to parse, once to get the author info for topo-sort, and
possibly later to show it if --pretty is given; though I did not check
and maybe we turn off save_commit_buffer with --pretty). It would be
nice to have an extended parse_object that handled that. I'm not sure of
the interface. Maybe variadic with pairs of type/slab, like:
parse_commit_extended(commit,
PARSE_COMMIT_AUTHORDATE, authordate_slab,
PARSE_COMMIT_DONE);
?
What I had in mind actually was a custom slab tailored for each
caller that is an array of struct. If the caller is interested in
authordate and authorname, instead of populating two separate
authordate_slab and authorname_slab, the caller declares a
struct {
unsigned long date;
char name[FLEX_ARRAY];
} author_info;
prepares author_info_slab, and use your commit_parser API to fill
them.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file
On Thu, Jun 20, 2013 at 08:03:43PM +0200, Michael Haggerty wrote: I noticed that recently, too. I have a patch series about 90% complete that abstracts the tempfile handling (the ultimate goal of which is to optionally clean up tmp_* files in the objects/ directory). It refactors the lockfile cleanup, and it would not be too hard to have a committed or rolled-back lockfile actually remove itself from the to clean at exit list. Which would make it perfectly safe to have a lockfile as an automatic variable as long as you commit or rollback before leaving the function. Cool, then I won't work on that. You might also have to make the lockfile list into a doubly-linked-list to avoid having to do a linear scan to find the entry to delete, unless the total number of entries is known to remain small. Yes, I noticed that potential issue, but I don't think it is worth worrying about. We typically only take one lock at a time, or a handful of tempfiles (e.g., one object at a time, or two files for diff). And once it's abstracted out, it would be easy to handle later. The part I am a little stuck on is plugging it into pack-objects/index-pack. Their output handling is a little convoluted because they may be writing to stdout, to a tempfile, to a named file, or even appending to an existing file in the case of index-pack --fix-thin. I don't think it's unmanageable, but I need to spend some more time on the refactoring. Please CC me on the patch series when it is done. Will do. -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 1/3] rebase: guard against missing files in read_basic_state()
Martin von Zweigbergk martinv...@gmail.com writes:
On Thu, Jun 13, 2013 at 3:29 PM, Junio C Hamano gits...@pobox.com wrote:
Ramkumar Ramachandra artag...@gmail.com writes:
A more troublesome is that nobody seems to check the return value of
this function. If head-name, onto or orig-head is missing, is that
an error condition that should make the callers of read_basic_state
stop and refuse to proceed?
Since we unconditionally write those three (and 'quiet'), it seems
reasonable to require all of them to be there when continuing, so I
think you're right that we should fail fast.
The way the cascade is used seems to indicate that, but up to the
point where it sents $verbose. If and only if head-name, onto, orig-head
and quiet can be read in state-dir, verbose in state-dir is checked
and only then $verbose is set.
Martin, this seems to be from your series around early Feburary
2011. Do you recall why these checks are cascaded this way?
I do not offhand think of a good reason.
Neither do I. I think the cascading after 'quiet' is just a mistake on
my part. The consequences are probably close to none, since if one of
earlier commands fail, the other files will probably not be there
either. (Not defending it; I'm happy if it gets fixed, e.g. by making
it fail fast.)
I think this is probably the right thing to do, if we want to honor
the original intention of the earlier part of cascade. Everything
before this new || die reads from a file that should always exist
(e.g. even when not asked to be quiet, that state is not signaled by
the lack of $state_dir/quiet, but by having an empty string in it),
while everything after check optional state variable files (e.g. if
$state_dir/verbose does not exist, it is not an error, but signals
that the user did not ask us to be verbose).
Note that applying this patch _could_ uncover latent bug that was
masked by the lack of die here (maybe later codepath may depended
on not having orig_head at all and the only observable effect was
that in such a case, both quiet and verbose were silently ignored,
because the control did not reach the GIT_QUIET=... and verbose=t
assignments.
git-rebase.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index d0c11a9..90506ba 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -95,7 +95,9 @@ read_basic_state () {
else
orig_head=$(cat $state_dir/head)
fi
- GIT_QUIET=$(cat $state_dir/quiet)
+ GIT_QUIET=$(cat $state_dir/quiet) ||
+ die failed to read basic rebase state from $state_dir
+
test -f $state_dir/verbose verbose=t
test -f $state_dir/strategy strategy=$(cat $state_dir/strategy)
test -f $state_dir/strategy_opts
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/5] Reroll patches against v1.8.3.1
Alexey Shumkin alex.crez...@gmail.com writes: Alexey Shumkin (5): t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs t7102 (reset): don't hardcode SHA-1 in expected outputs t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting pretty: user format ignores i18n.logOutputEncoding setting Next time, please jog readers' memory better by giving a bit more descriptive cover letter. Reroll is known by v5 alrady, and against v1.8.3.1 certainly helps, but what this series is about is unclear, especially because the end-game patch user format ignores is rather badly stated. It is unclear if it is a good thing that user format ignores it and the patch makes sure that is the case, if we currently ignore it in user format and the patch fixes it by making user format pay attention to it, etc. Thanks. builtin/reset.c | 8 +- builtin/rev-list.c | 1 + builtin/shortlog.c | 1 + log-tree.c | 1 + submodule.c | 3 + t/t4041-diff-submodule-option.sh | 25 +++-- t/t4205-log-pretty-formats.sh| 179 - t/t6006-rev-list-format.sh | 209 --- t/t7102-reset.sh | 37 ++- 9 files changed, 299 insertions(+), 165 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] log: --author-date-order
On Thu, Jun 20, 2013 at 12:36:21PM -0700, Junio C Hamano wrote:
I like the latter option. It takes a non-trivial amount of time to load
the commits from disk, and now we are potentially doing it 2 or 3 times
for a run (once to parse, once to get the author info for topo-sort, and
possibly later to show it if --pretty is given; though I did not check
and maybe we turn off save_commit_buffer with --pretty). It would be
nice to have an extended parse_object that handled that. I'm not sure of
the interface. Maybe variadic with pairs of type/slab, like:
parse_commit_extended(commit,
PARSE_COMMIT_AUTHORDATE, authordate_slab,
PARSE_COMMIT_DONE);
?
What I had in mind actually was a custom slab tailored for each
caller that is an array of struct. If the caller is interested in
authordate and authorname, instead of populating two separate
authordate_slab and authorname_slab, the caller declares a
struct {
unsigned long date;
char name[FLEX_ARRAY];
} author_info;
prepares author_info_slab, and use your commit_parser API to fill
them.
Yes, I think it is nicer to stay in one slab if you have multiple
values, but it means more custom code for the caller. If the
commit_parser API is nice, it should not be that much code, though.
It does make it harder to support arbitrary combinations directly in
parse_commit. If a caller wants to also parse_commit and use the same
buffer to pick out its custom information, I think we'd need to do one
of:
1. Give parse_commit a callback, so that the callback can pick out the
data it wants while parse_commit has the commit buffer in memory.
E.g.:
void grab_author_info(const char *buf, unsigned long len, void *data)
{
struct author_info *ai = data;
/* fill fields from buffer */
}
...
parse_commit_extra(commit, grab_author_info,
slab_at(author_slab, commit));
2. Teach parse_commit to operate not only on a raw commit object, but
also on the commit_parser API. Like:
struct commit_parser parser = {0};
/* actually open the object and start our incremental parser */
init_commit_parser(parser, commit);
/* fill in parents, date, etc, as parse_commit does now */
parse_commit_from_parser(commit, parser);
/* fill in whatever extra data we are interested in */
*slab_at(slab, commit) = get_author_date(parser);
/* done, drop the buffer */
close_commit_parser(parser);
The latter would need to handle transferring ownership of the buffer to
struct commit from struct commit_parser when save_commit_buffer is
turned off.
I think we're a bit high-level now to be making such decisions, though,
as we do not even have such a commit_parser API.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
On Thu, Jun 20, 2013 at 11:43:37AM -0700, Junio C Hamano wrote: Thomas Rast tr...@inf.ethz.ch writes: The logic for pulling into an unborn branch was originally designed to be used on a newly-initialized repository (d09e79c, git-pull: allow pulling into an empty repository, 2006-11-16). It thus did not initially deal with uncommitted changes in the unborn branch. The case of an _unstaged_ untracked file was fixed by 4b3ffe5 (pull: do not clobber untracked files on initial pull, 2011-03-25). However, it still clobbered existing staged files, both when the file exists in the merged commit (it will be overwritten), and when it does not (it will be deleted). Perhaps making sure the index is empty is sufficient, then? That would not let you pull when you have foo staged, but upstream does not have foo at all. To be fair, that is quite a corner case, and simply rejecting the pull entirely may be OK. But read-tree already does the hard work for us, so I don't think it is a lot of code either way. -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 v5 5/5] pretty: user format ignores i18n.logOutputEncoding setting
Alexey Shumkin alex.crez...@gmail.com writes:
Subject: Re: [PATCH v5 5/5] pretty: user format ignores i18n.logOutputEncoding
setting
That is a statement of fact, and does not tell much to the reader.
I think you are saying that in the current implementation,
logoutputencoding is not honored in user format, that it is a bug
that needs to be fixed (as opposed to that is by design, the
scripts that read from --format='' is responsible for doing the
conversion), and that this patch fixes it.
So
pretty: --format output should honor logOutputEncoding
or something? At least it says what is the _desired_ outcome with
should, hints that the status-quo is different from that desired
outcome and implies that this is a patch to fix it.
The Subject line is very important as that is the only thing we
see in many summarizing output format, e.g. shortlog, cover letter
and merge message.
The following two commands are expected to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it format %s.
Log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted with the latter even
when i18n.logOutputEncoding and terminal encoding are the same.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
and
$ git reset --hard
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
builtin/reset.c | 8 ++--
builtin/rev-list.c | 1 +
builtin/shortlog.c | 1 +
log-tree.c | 1 +
submodule.c | 3 +++
t/t4041-diff-submodule-option.sh | 10 +-
t/t4205-log-pretty-formats.sh| 34 +-
t/t6006-rev-list-format.sh | 8
t/t7102-reset.sh | 2 +-
9 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 6032131..b23ed63 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -92,11 +92,15 @@ static int reset_index(const unsigned char *sha1, int
reset_type, int quiet)
static void print_new_head_line(struct commit *commit)
{
- const char *hex, *body;
+ const char *hex, *body, *encoding;
hex = find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV);
printf(_(HEAD is now at %s), hex);
- body = strstr(commit-buffer, \n\n);
+ encoding = get_log_output_encoding();
+ body = logmsg_reencode(commit, NULL, encoding);
+ if (!body)
+ body = commit-buffer;
Does this happen? I thought body, without an error, can be the same
as commit-buffer.
+ body = strstr(body, \n\n);
if (body) {
const char *eol;
size_t len;
Do we have a leak here? body may point at a new piece of memory
logmsg_reencode() have allocated to hold the UTF-8 version of your
8859-5 message in commit-buffer.
It would be more like this, no?
diff --git a/builtin/reset.c b/builtin/reset.c
index 6032131..8d22ffe 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -92,11 +92,13 @@ static int reset_index(const unsigned char *sha1, int
reset_type, int quiet)
static void print_new_head_line(struct commit *commit)
{
- const char *hex, *body;
+ const char *hex, *body, *msg, *encoding;
hex = find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV);
printf(_(HEAD is now at %s), hex);
- body = strstr(commit-buffer, \n\n);
+
+ msg = logmsg_reencode(commit, NULL, get_log_output_encoding());
+ body = strstr(msg, \n\n);
if (body) {
const char *eol;
size_t len;
@@ -107,6 +109,7 @@ static void print_new_head_line(struct commit *commit)
}
else
printf(\n);
+ logmsg_free(msg, commit);
}
static void update_index_from_diff(struct diff_queue_struct *q,
--
To unsubscribe from this list: send the line 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 v5 1/5] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
Alexey Shumkin alex.crez...@gmail.com writes: The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com --- t/t6006-rev-list-format.sh | 142 + 1 file changed, 79 insertions(+), 63 deletions(-) diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh index 0393c9f..d32e65e 100755 --- a/t/t6006-rev-list-format.sh +++ b/t/t6006-rev-list-format.sh @@ -7,8 +7,21 @@ test_description='git rev-list --pretty=format test' test_tick test_expect_success 'setup' ' -touch foo git add foo git commit -m added foo - echo changed foo git commit -a -m changed foo + : foo + git add foo + git commit -m added foo + head1=$(git rev-parse --verify HEAD) + head1_short=$(git rev-parse --short $head1) + tree1=$(git cat-file commit HEAD | sed -n -e s/^tree //p -e /^$/q) + tree1_short=$(git rev-parse --short $tree1) + echo changed foo + git commit -a -m changed foo + head2=$(git rev-parse --verify HEAD) + head2_short=$(git rev-parse --short $head2) + head2_parent=$(git cat-file commit HEAD | sed -n -e s/^parent //p -e /^$/q) + head2_parent_short=$(git rev-parse --short $head2_parent) + tree2=$(git cat-file commit HEAD | sed -n -e s/^tree //p -e /^$/q) With modern Git, you can (and would) write this as tree2=$(git rev-parse HEAD:) (the same for tree1). Other than that, looks sensible. Will queue with a tweak. -- To unsubscribe from this list: send the line 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 v5 3/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs
Alexey Shumkin alex.crez...@gmail.com writes: The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com --- t/t4205-log-pretty-formats.sh | 48 +++ 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 26fbfde..6d8d457 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -101,7 +101,11 @@ test_expect_failure 'NUL termination with --stat' ' test_expect_success 'setup more commits' ' test_commit message one one one message-one - test_commit message two two two message-two + test_commit message two two two message-two + head1=$(git rev-list --max-count=1 --abbrev-commit HEAD~0) + head2=$(git rev-list --max-count=1 --abbrev-commit HEAD~1) + head3=$(git rev-list --max-count=1 --abbrev-commit HEAD~2) + head4=$(git rev-list --max-count=1 --abbrev-commit HEAD~3) Hmmph. Why not rev-parse --short like in your 1/5? -- To unsubscribe from this list: send the line 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 v5 4/5] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
Alexey Shumkin alex.crez...@gmail.com writes:
@@ -19,7 +23,8 @@ add_file () {
echo $name $name
git add $name
test_tick
- git commit -m Add $name || exit
+ msg_added_iso88595=$(echo Add $name ($added $name) |
iconv -f utf-8 -t iso88595)
+ git -c 'i18n.commitEncoding=iso88595' commit -m
$msg_added_iso88595
Hmph. Do we know 8859-5 is available or do these need to be
protected with prereq?
Can these tests be done with 8859-1 i.e. something we already depend
on, by changing that $added message to latin-1, or is there something
very Russian specific breakage we want to test here?
If the former, please redo this entire patch (not just t4041) with
8859-1.
If there is some Russian specific breakage you cannot demonstrate
with 8859-1, then please explain it in the log message.
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index d32e65e..36e4cc0 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
...
-# usage: test_format name format_string expected_output
+# usage: test_format [failure] name format_string expected_output
test_format () {
+ local must_fail=0
This breaks tests for non-bash users. local is not even in POSIX.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Jeff King p...@peff.net writes: Perhaps making sure the index is empty is sufficient, then? That would not let you pull when you have foo staged, but upstream does not have foo at all. To be fair, that is quite a corner case, and simply rejecting the pull entirely may be OK. That simplicity was what I was hinting at ;-). But read-tree already does the hard work for us, so I don't think it is a lot of code either way. OK, I just got an impression from reading the back-and-forth between you two that read-tree does not want to deal with that case. But yes, if you say I have this index, and I am straying away from an empty tree to that commit, with two-tree form read-tree -m -u, everything should work correctly, including the bit that says nah, nah, you have added 'foo' but the other guy also adds 'foo', so I'll refuse. So please scratch that short-cut suggestion. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
From: Junio C Hamano gits...@pobox.com Sent: Thursday, June 20, 2013 8:23 PM Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: Decouple `simple` from `upstream` completely, and change it to mean `current` with a safety feature: a `push` and `pull` should not be asymmetrical in the special case of central workflows. Double negation confused my parser. 'push' and 'pull' should be kept symmetrical in central workflows? They're not the same thing. It is very much intentional and intended: the safety net is not to ensure that the push and pull are symmetrical (i.e. among other things, error out if branch.$branch.merge is unset), but rather ensure that the push and pull are never asymmetrical. H not to ensure that the push and pull are symmetrical rather ensure that the push and pull are never asymmetrical. They still talk the same thing to me. What am I missing? Am I being clueless, or is there something else going on? I think it is a case of the user having explicitly set push=Africa and pull=Europe which can't be a setting for simple symmetry. But then again I haven't been following the fine details. (that is there are some defaults that allow stuff to be half set) Your among other things, after reading it three times, unfortunately did not help clarify anything to me, so perhaps somebody other than me (or you for that matter) who is more clueful can help find a different way to explain the difference you are trying to express to me. Help, anybody? Philip [...] -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
On Thu, Jun 20, 2013 at 01:49:13PM -0700, Junio C Hamano wrote: But read-tree already does the hard work for us, so I don't think it is a lot of code either way. OK, I just got an impression from reading the back-and-forth between you two that read-tree does not want to deal with that case. I think I got us off-track with my expectation of ending the one case with a conflicted index. But caring about that is even more unlikely. I think Thomas's original patch is probably a happy medium. As an orthogonal matter, we probably should reverse the order of updating HEAD and the index/working tree, as it does not make much sense to me to do the former if the latter is not possible (and that is the case even with the current code). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Philip Oakley philipoak...@iee.org writes: From: Junio C Hamano gits...@pobox.com Sent: Thursday, June 20, 2013 8:23 PM Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: Double negation confused my parser. 'push' and 'pull' should be kept symmetrical in central workflows? They're not the same thing. It is very much intentional and intended: the safety net is not to ensure that the push and pull are symmetrical (i.e. among other things, error out if branch.$branch.merge is unset), but rather ensure that the push and pull are never asymmetrical. H not to ensure that the push and pull are symmetrical rather ensure that the push and pull are never asymmetrical. They still talk the same thing to me. What am I missing? Am I being clueless, or is there something else going on? I think it is a case of the user having explicitly set push=Africa and pull=Europe which can't be a setting for simple symmetry. Yeah but then that is not a discussion about central workflow. I can understand In a central workflow push and pull should be symmetrical. I can also, with a bit of double-negation brain twisting, understand In a central workflow, push and pull should not be asymmetrical. But when I suggest to avoid double-negation, I was told that these two statements mean different things, and the original should not be rewritten to avoid double-negation, which is where my brain stopped and asked for help. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
May be because they (LKM) are more open to such architectural and organization refactorings? Some maintainers, like Greg Kroah-Hartman and possibly others accept clean up patches, such thing seems to be unacceptable here on git. Looks like there is space here only for features and bug fixes. Nothing else. I'm not saying that is bad at all. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Junio C Hamano wrote:
They're not the same thing. It is very much intentional and intended:
the safety net is not to ensure that the push and pull are
symmetrical (i.e. among other things, error out if
branch.$branch.merge is unset), but rather ensure that the push and
pull are never asymmetrical.
not to ensure that the push and pull are symmetrical
rather ensure that the push and pull are never asymmetrical.
They still talk the same thing to me. What am I missing?
Never mind the wording then. I am proposing preventing asymmetry by
explicitly disallowing a push when $branch is different from
branch.$branch.merge, instead of shutting down immediately when
branch.$branch.merge is unset.
Now I'd like to question what you are labelling as safety. What is
the consequence of erroring out when branch.$branch.merge is unset
when pushing using `upstream`?
Nothing noteworthy.
I wasn't suggesting to change what `upstream` does at all.
No, but I did. I just argued for a sane default for
branch.$branch.merge (the part you snipped out).
The conclusion is that using push.default=`upstream` is meaningless
when you are using a triangular workflow.
Yes, and I agreed with that.
If you are using a
centralized workflow, and if a branch does not have branch.*.merge
configured, we do not know to which branch you are pushing it back,
so we error out.
And I said: have a sane default.
What I am changing from the patch you posted with the how about
this on top patch back to the current behaviour is what 'simple'
does for centralized workflow.
Yes, I know. I read the patch.
When you are doing a centralized workflow, 'simple' was defined
Again, I'm well aware what it _was_ defined as. Was it not clear that
I argued for a change from the very first email where I posted the
patch and changed a test? Do you have a counter-argument, or is it
simply that we must respect its historical meaning?
Now, no existing series has casted in stone the best behaviour for
`simple` in a triangular workflow. With this series (and also with
my fixup patch I sent last night), it is defined to act as `current`,
but it may need a bit more safety to help new users avoid pushing
branches they did not intend to (perhaps pushing out `current` only
when the branch with the same name already exists at the destination?
I dunno).
I see no reason to plan safety features in advance, especially since
we have neither branch.$branch.push nor @{push} yet.
--
To unsubscribe from this list: send the line 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?] remote prune origin interacts badly with clone --mirror and multiple remotes
[git version: next as of yesterday afternoon] If I clone a repo with git clone --mirror, and add other remotes later, 'git remote prune origin' deletes all branches and tags of the other remotes. Easily repeatable example: [core] repositoryformatversion = 0 filemode = true bare = true logallrefupdates = false [remote origin] url = git://github.com/git/git.git fetch = +refs/*:refs/* mirror = true [remote peff] url = git://github.com/peff/git.git fetch = +refs/heads/*:refs/remotes/peff/* 'git remote prune origin' will delete all peff's branches in this case. I'm guessing the wildcards refs/* and refs/remotes/peff/* interact badly in some place, and I'm trying to understand builtin/remote.c to see if I can fix it, but haven't gotten very far yet. git fetch --prune origin and git remote update --prune also show this behaviour. git remote prune peff does not delete non-peff branches in this scenario, further strengthening my belief that the refs/* and refs/remotes/peff/* wildcards interact badly with prune. Or is this considered normal behaviour and is what I'm trying to do simply unsupported? In that case a warning would be welcome when adding remotes to a --mirror'ed repository. -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Without any configuration the current branch is pushed out, which
loosens the safety we implemented in the current 'safer upstream'.
I am not convinced this is a good change. I am not convinced this is
a bad change, either, yet, but this loosening smells bad.
Provided that we would want to keep the Push the current one to the
same name but you have to have it set up as your integration source
safety for central workflow (which I am starting to think we
should), we would want something like this on top of your entire
series, I think. The behaviour change can be seen in the revert of
one test you made to the test that expects simple to fail due to
the safety.
And with the small refactoring of setup_default_push_refspecs (the
important part being that we grab branch in this function, not in
its helper functions per push.default mode), branch.*.push can fall
out rather naturally, like this patch on top.
A footnote unrelated to this patch.
The function is_workflow_triangular() in the how about this patch
needs to be tweaked from the version I am responding to, in order to
take the case where fetch-remote is not defined into account, i.e.
static int is_workflow_triagular(struct remote *remote)
{
struct remote *fetch_remote = remote_get(NULL);
return (fetch_remote fetch_remote != remote);
}
builtin/push.c | 18 +-
remote.c | 5 +
remote.h | 2 ++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/builtin/push.c b/builtin/push.c
index f6c8047..a140b8e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -185,6 +185,15 @@ static void
warn_unspecified_push_default_configuration(void)
warning(%s\n, _(warn_unspecified_push_default_msg));
}
+static void setup_per_branch_push(struct branch *branch)
+{
+ struct strbuf refspec = STRBUF_INIT;
+
+ strbuf_addf(refspec, %s:%s,
+ branch-name, branch-push_name);
+ add_refspec(refspec.buf);
+}
+
static int is_workflow_triagular(struct remote *remote)
{
struct remote *fetch_remote = remote_get(NULL);
@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)
static void setup_default_push_refspecs(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
- int triangular = is_workflow_triagular(remote);
+ int triangular;
+
+ if (branch-push_name) {
+ setup_per_branch_push(branch);
+ return;
+ }
+
+ triangular = is_workflow_triagular(remote);
switch (push_default) {
default:
diff --git a/remote.c b/remote.c
index e71f66d..e033fef 100644
--- a/remote.c
+++ b/remote.c
@@ -372,6 +372,11 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (!value)
return config_error_nonbool(key);
add_merge(branch, xstrdup(value));
+ } else if (!strcmp(subkey, .push)) {
+ if (!value)
+ return config_error_nonbool(key);
+ free(branch-push_name);
+ branch-push_name = xstrdup(value);
}
return 0;
}
diff --git a/remote.h b/remote.h
index cf56724..84e0f72 100644
--- a/remote.h
+++ b/remote.h
@@ -138,6 +138,8 @@ struct branch {
struct refspec **merge;
int merge_nr;
int merge_alloc;
+
+ char *push_name;
};
struct branch *branch_get(const char *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: [PATCH v2] pull: merge into unborn by fast-forwarding from empty tree
Jeff King p...@peff.net writes: I think I got us off-track with my expectation of ending the one case with a conflicted index. But caring about that is even more unlikely. I think Thomas's original patch is probably a happy medium. OK, so should I consider it Reviewed-by: peff? As an orthogonal matter, we probably should reverse the order of updating HEAD and the index/working tree, as it does not make much sense to me to do the former if the latter is not possible (and that is the case even with the current code). Yes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
