[PATCH v2] git-svn: workaround for a bug in svn serf backend
Subversion serf backend in versions 1.8.5 and below has a bug that the function creating the descriptor of a file change -- add_file() -- doesn't make a copy of its third argument when storing it on the returned descriptor. As a result, by the time this field is used (in transactions of file copying or renaming) it may well be released, and the memory reused. One of its possible manifestations is the svn assertion triggering on an invalid path, with a message svn_fspath__skip_ancestor: Assertion `svn_fspath__is_canonical(child_fspath)' failed. This patch works around this bug, by storing the value to be passed as the third argument to add_file() in a local variable with the same scope as the file change descriptor, making sure their lifetime is the same. Cc: Benjamin Pabst Cc: Eric Wong Cc: Jonathan Nieder Signed-off-by: Roman Kagan --- changes since v1: - fix grammar in the patch and the log message - refer to the triggered error message perl/Git/SVN/Editor.pm | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm index b3bcd47..34e8af9 100644 --- a/perl/Git/SVN/Editor.pm +++ b/perl/Git/SVN/Editor.pm @@ -304,8 +304,12 @@ sub C { my ($self, $m, $deletions) = @_; my ($dir, $file) = split_path($m->{file_b}); my $pbat = $self->ensure_path($dir, $deletions); + # workaround for a bug in svn serf backend (v1.8.5 and below): + # store third argument to ->add_file() in a local variable, to make it + # have the same lifetime as $fbat + my $upa = $self->url_path($m->{file_a}); my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat, - $self->url_path($m->{file_a}), $self->{r}); + $upa, $self->{r}); print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q; $self->chg_file($fbat, $m); $self->close_file($fbat,undef,$self->{pool}); @@ -323,8 +327,10 @@ sub R { my ($self, $m, $deletions) = @_; my ($dir, $file) = split_path($m->{file_b}); my $pbat = $self->ensure_path($dir, $deletions); + # workaround for a bug in svn serf backend, see comment in C() above + my $upa = $self->url_path($m->{file_a}); my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat, - $self->url_path($m->{file_a}), $self->{r}); + $upa, $self->{r}); print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q; $self->apply_autoprops($file, $fbat); $self->chg_file($fbat, $m); -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git:// protocol over SSL/TLS
Hello everyone! Quick question is, is it possible to use git:// protocol over SSL/TLS/other secure transport? Or the recommended way to do secure anonymous checkout is to simply use https:// ? Thanks in advance! -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
Sergey Sharybin writes: > Quick question is, is it possible to use git:// protocol over > SSL/TLS/other secure transport? The git protocol itself performs no encryption or authentication by design. This is the job of the transport protocol. > Or the recommended way to do secure anonymous checkout is to simply > use https:// ? Yes. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH 0/5] git checkout --recurse-submodules
Am 26.12.2013 16:58, schrieb Jonathan Nieder: > This patch series comes from > https://github.com/jlehmann/git-submod-enhancements branch > recursive_submodule_checkout. It needed some tiny tweaks to apply to > current "master" and build without warnings, but nothing major, and I > haven't sanity checked it much beyond that and letting the kind folks > that use Debian experimental play with it. Cool! Thanks for rebasing this series and great to hear that more people are using it. > I'm sending it out now to get review and ideas for what needs to > happen next to get this series in shape to be included in git.git. Excellent timing, Heiko and I wanted to work on this topic in the coming days anyway. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
On Fri, 27 Dec 2013 18:59:00 +0600 Sergey Sharybin wrote: > Quick question is, is it possible to use git:// protocol over > SSL/TLS/other secure transport? The Git protocol does not implement it itself but you can channel it over a TLS tunnel (via stunnel for instance). Unfortunately, this means a specialized software and setup on both ends so if the question was about a general client using stock Git then the answer is no, it's impossible. > Or the recommended way to do secure anonymous checkout is to simply > use https:// ? Yes, but it will only be secure if you've managed to verify the server's certificate and do trust its issuer (or a CA higher up the cert's trust chain) -- people tend to confuse "encrypted" with "secure" which is not at all the same thing. -- To unsubscribe from this list: send the line "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/5] submodule: prepare for recursive checkout of submodules
Am 26.12.2013 17:02, schrieb Jonathan Nieder: > From: Jens Lehmann > Date: Mon, 18 Jun 2012 22:17:59 +0200 > > This commit adds the functions needed for configuration, for setting the > default behavior and for determining if a submodule path should be updated > automatically. > > Signed-off-by: Jens Lehmann > Signed-off-by: Jonathan Nieder > --- > Should probably be squashed into a patch that uses and documents this > configuration. I'm not so sure. At the end of the day at least am, bisect, checkout, checkout-index, cherry-pick, merge, pull, read-tree, rebase, reset & stash should have learned this option (and all porcelain must honor the autoupdate setting by default, plumbing only when requested by giving the "--recurse-submodules=config" option to make life easier for scripting). And I think they should learn this one command per commit, making this one - and the yet-to-be-programmed one introducing the autoupdate flag - the base for them. > submodule.c | 36 > submodule.h | 3 +++ > 2 files changed, 39 insertions(+) > > diff --git a/submodule.c b/submodule.c > index 613857e..3f18d4d 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -16,6 +16,8 @@ static struct string_list config_name_for_path; > static struct string_list config_fetch_recurse_submodules_for_name; > static struct string_list config_ignore_for_name; > static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND; > +static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; > +static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT; > static struct string_list changed_submodule_paths; > static int initialized_fetch_ref_tips; > static struct sha1_array ref_tips_before_fetch; > @@ -382,6 +384,34 @@ int parse_fetch_recurse_submodules_arg(const char *opt, > const char *arg) > } > } > > +int parse_update_recurse_submodules_arg(const char *opt, const char *arg) > +{ > + switch (git_config_maybe_bool(opt, arg)) { > + case 1: > + return RECURSE_SUBMODULES_ON; > + case 0: > + return RECURSE_SUBMODULES_OFF; > + default: > + if (!strcmp(arg, "checkout")) > + return RECURSE_SUBMODULES_ON; > + die("bad %s argument: %s", opt, arg); > + } > +} > + > +int submodule_needs_update(const char *path) > +{ > + struct string_list_item *path_option; > + path_option = unsorted_string_list_lookup(&config_name_for_path, path); > + if (!path_option) > + return 0; > + > + /* update can't be "none", "merge" or "rebase" */ > + > + if (option_update_recurse_submodules != RECURSE_SUBMODULES_DEFAULT) > + return 1; > + return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF; > +} > + > void show_submodule_summary(FILE *f, const char *path, > const char *line_prefix, > unsigned char one[20], unsigned char two[20], > @@ -589,6 +619,12 @@ int push_unpushed_submodules(unsigned char new_sha1[20], > const char *remotes_nam > return ret; > } > > +void set_config_update_recurse_submodules(int default_value, int > option_value) > +{ > + config_update_recurse_submodules = default_value; > + option_update_recurse_submodules = option_value; > +} > + > static int is_submodule_commit_present(const char *path, unsigned char > sha1[20]) > { > int is_present = 0; > diff --git a/submodule.h b/submodule.h > index 7beec48..055918c 100644 > --- a/submodule.h > +++ b/submodule.h > @@ -22,12 +22,15 @@ void gitmodules_config(void); > int parse_submodule_config_option(const char *var, const char *value); > void handle_ignore_submodules_arg(struct diff_options *diffopt, const char > *); > int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg); > +int parse_update_recurse_submodules_arg(const char *opt, const char *arg); > +int submodule_needs_update(const char *path); > void show_submodule_summary(FILE *f, const char *path, > const char *line_prefix, > unsigned char one[20], unsigned char two[20], > unsigned dirty_submodule, const char *meta, > const char *del, const char *add, const char *reset); > void set_config_fetch_recurse_submodules(int value); > +void set_config_update_recurse_submodules(int default_value, int > option_value); > void check_for_new_submodule_commits(unsigned char new_sha1[20]); > int fetch_populated_submodules(const struct argv_array *options, > const char *prefix, int command_line_option, > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
Hi, On Fri, Dec 27, 2013 at 7:36 PM, Konstantin Khomoutov wrote: > > The Git protocol does not implement it itself but you can channel it > over a TLS tunnel (via stunnel for instance). Unfortunately, this > means a specialized software and setup on both ends so if the question > was about a general client using stock Git then the answer is no, it's > impossible. Ok, got it. > Yes, but it will only be secure if you've managed to verify the > server's certificate and do trust its issuer (or a CA higher up the > cert's trust chain) -- people tend to confuse "encrypted" with > "secure" which is not at all the same thing. We've got CA-signed certificate atm and it's about to be also EV-signed for our server (git.blender.org). So this is not gonna to be an issue. Cloning over https:// works fine, but we wanted to be sure all the bits are secure. So guess we just need to recommend using https:// protocol instead of git:// for our users? -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "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/5] submodule: teach unpack_trees() to remove submodule contents
Am 26.12.2013 17:12, schrieb Jonathan Nieder: > From: Jens Lehmann > Date: Tue, 19 Jun 2012 20:55:45 +0200 > > Implement the functionality needed to enable work tree manipulating > commands to that a deleted submodule should not only affect the index > (leaving all the files of the submodule in the work tree) but also to > remove the work tree of the superproject (including any untracked > files). > > That will only work properly when the submodule uses a gitfile instead of > a .git directory and no untracked files are present. Otherwise the removal > will fail with a warning (which is just what happened until now). > > Extend rmdir_or_warn() to remove the directories of those submodules which > are scheduled for removal. Also teach verify_clean_submodule() to check > that a submodule configured to be removed is not modified before scheduling > it for removal. > > Signed-off-by: Jens Lehmann > Signed-off-by: Jonathan Nieder > --- > Should this share some code (or just the error message) with the 'git > rm' code that checks whether a submodule is safe to remove? Yes, that would make sense. > rmdir_or_warn is a pretty low-level function --- it feels odd to be > relying on the git repository layout here. On the other hand, that > function only has two callers, so it is possible to check quickly > whether it is safe. > > I assume this is mostly for the sake of the caller in unpack-trees? Yup. > In builtin/apply.c, remove_file is used for deletion and rename > patches. Do we want this logic take effect there, too? I think so. Recursive update should also affect apply and am when requested via command line or configuration. But the apply documentation states that it also handles files outside a git repository, so we would have to disable this logic in that case. > submodule.c| 37 + > submodule.h| 1 + > unpack-trees.c | 6 +++--- > wrapper.c | 3 +++ > 4 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/submodule.c b/submodule.c > index 3f18d4d..a25db46 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -412,6 +412,43 @@ int submodule_needs_update(const char *path) > return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF; > } > > +int depopulate_submodule(const char *path) > +{ > + struct strbuf dot_git = STRBUF_INIT; > + struct child_process cp; > + const char *argv[] = {"rm", "-rf", path, NULL}; > + > + /* Is it populated? */ > + strbuf_addf(&dot_git, "%s/.git", path); > + if (!resolve_gitdir(dot_git.buf)) { > + strbuf_release(&dot_git); > + return 0; > + } > + strbuf_release(&dot_git); > + > + /* Does it have a .git directory? */ > + if (!submodule_uses_gitfile(path)) { > + warning(_("cannot remove submodule '%s' because it (or one of " > + "its nested submodules) uses a .git directory"), > + path); > + return -1; > + } > + > + /* Remove the whole submodule directory */ > + memset(&cp, 0, sizeof(cp)); > + cp.argv = argv; > + cp.env = local_repo_env; > + cp.git_cmd = 0; > + cp.no_stdin = 1; > + if (run_command(&cp)) { > + warning("Could not remove submodule %s", path); > + strbuf_release(&dot_git); > + return -1; > + } > + > + return 0; > +} > + > void show_submodule_summary(FILE *f, const char *path, > const char *line_prefix, > unsigned char one[20], unsigned char two[20], > diff --git a/submodule.h b/submodule.h > index 055918c..df291cf 100644 > --- a/submodule.h > +++ b/submodule.h > @@ -24,6 +24,7 @@ void handle_ignore_submodules_arg(struct diff_options > *diffopt, const char *); > int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg); > int parse_update_recurse_submodules_arg(const char *opt, const char *arg); > int submodule_needs_update(const char *path); > +int depopulate_submodule(const char *path); > void show_submodule_summary(FILE *f, const char *path, > const char *line_prefix, > unsigned char one[20], unsigned char two[20], > diff --git a/unpack-trees.c b/unpack-trees.c > index ad3e9a0..89b506a 100644 > --- a/unpack-trees.c > +++ b/unpack-trees.c > @@ -8,6 +8,7 @@ > #include "progress.h" > #include "refs.h" > #include "attr.h" > +#include "submodule.h" > > /* > * Error messages expected by scripts out of plumbing commands such as > @@ -1263,14 +1264,13 @@ static void invalidate_ce_path(const struct > cache_entry *ce, > /* > * Check that checking out ce->sha1 in subdir ce->name is not > * going to overwrite any working files. > - * > - * Currently, git does not checkout subprojects during a superproject > - * checkout, so it is not going to overwrite anything. > */ > static int verify_clean_submodule(const struct cache_entry *ce, > enum unpack_trees_error_type
Re: git:// protocol over SSL/TLS
Sergey Sharybin writes: > So guess we just need to recommend using https:// protocol instead of > git:// for our users? Given how easy it is to verify the integrity of a git repository out of band there isn't really much of added security by using TLS for transport. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
On Fri, 27 Dec 2013 19:58:19 +0600 Sergey Sharybin wrote: [...] > > Yes, but it will only be secure if you've managed to verify the > > server's certificate and do trust its issuer (or a CA higher up the > > cert's trust chain) -- people tend to confuse "encrypted" with > > "secure" which is not at all the same thing. > > We've got CA-signed certificate atm and it's about to be also > EV-signed for our server (git.blender.org). So this is not gonna to be > an issue. Cloning over https:// works fine, but we wanted to be sure > all the bits are secure. This setup sounds to be just the right thing. > So guess we just need to recommend using https:// protocol instead of > git:// for our users? I think yes. HTTP[S] once was dumb and slow but now it should be comparable in speed to git:// as essentially using this protocol (which became "smart" [1]) means spawning a git server process once per fetch/push session and making the client and server Git processes communicate all by themselves, so HTTP is there for request routing, authentication and session setup while data transfer is carried out by Git processes themselves [2]. 1. http://git-scm.com/blog/2010/03/04/smart-http.html 2. https://www.kernel.org/pub/software/scm/git/docs/git-http-backend.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Teach checkout to recursively checkout submodules
Am 26.12.2013 17:22, schrieb Jonathan Nieder: > From: Jens Lehmann > Date: Wed, 13 Jun 2012 18:50:10 +0200 > > Signed-off-by: Jens Lehmann > Signed-off-by: Jonathan Nieder > --- > This is the patch that actually introduces the --recurse-submodules > option, which makes the rest work. > > The tests indicate some future directions for improving this, but > it works reasonably well already. I think I'd be most comfortable > applying these if they were rearranged a little to the following > order: > > 1. First, introducing the --recurse-submodules option, perhaps > with no actual effect, with tests that it is parsed correctly, > the default works, etc. > > 2. Then, adding the desired behaviors one by one with corresponding > tests (handling submodule modifications, removals, additions). > > 3. Finally, any needed tweaks on top. > > That way, it is easy to play around with early patches without > worrying about the later ones at first, and the patches are easy > to review to confirm that they at least don't break anything in > the --no-recurse-submodules case. Makes tons of sense. The only feature I'm aware of that is currently missing is to read the path <-> name mappings from the correct .gitmodules blob, which is needed to populate submodules that appear. > That said, Debian experimental has these applied as is already, > and there haven't been any complaints yet. Great! I'm also using this code at $dayjob successfully for quite some time now. Additionally I also enable unconditional recursive checkout by putting a "return 1" in the first line of the submodule_needs_update() function (Which is a hack to emulate "autoupdate=true" while at the same time pretending to already having added "--recurse-submodules=config" to all call sites in git porcelain scripts that call plumbing themselves). Only in a few corner cases submodules aren't properly updated, but we'll weed those out while implementing the tests. And we need lots of those to cover all important combinations ... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
On Fri, 27 Dec 2013 15:12:07 +0100 Andreas Schwab wrote: > > So guess we just need to recommend using https:// protocol instead > > of git:// for our users? > > Given how easy it is to verify the integrity of a git repository out > of band there isn't really much of added security by using TLS for > transport. If the devs employ signed tags then yes but otherwise you'd have to have some reference repository to compare with. Sure they target for a more no-brainer setup. ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
Our sysadmns are mainly worried about possible MITM which might give users completely wrong repo. For sure users might simply compare hash of HEAD from https'ed site with repo browser with what they've got in the checkout. But that's an extra step which we'd like to avoid without security harm :) On Fri, Dec 27, 2013 at 8:12 PM, Andreas Schwab wrote: > Sergey Sharybin writes: > >> So guess we just need to recommend using https:// protocol instead of >> git:// for our users? > > Given how easy it is to verify the integrity of a git repository out of > band there isn't really much of added security by using TLS for > transport. > > Andreas. > > -- > Andreas Schwab, sch...@linux-m68k.org > GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 > "And now for something completely different." -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: git:// protocol over SSL/TLS
> -Original Message- > From: Andreas Schwab > Sent: Friday, December 27, 2013 9:12 AM > > Sergey Sharybin writes: > > > So guess we just need to recommend using https:// protocol instead of > > git:// for our users? > > Given how easy it is to verify the integrity of a git repository out of > band there isn't really much of added security by using TLS for > transport. I have to say, using encryption (TLS, etc) is not just for assurance of the communicating parties, but also to prevent a compromise of confidentiality. Jason Pyeron (please do not add me to the cc) smime.p7s Description: S/MIME cryptographic signature
Re: git:// protocol over SSL/TLS
Andreas Schwab writes: > Sergey Sharybin writes: > >> So guess we just need to recommend using https:// protocol instead of >> git:// for our users? > > Given how easy it is to verify the integrity of a git repository out of > band there isn't really much of added security by using TLS for > transport. You can verify integrity after the fact, but not guarantee confidentiality ... so it again depends on the definition of "security". -- 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: git:// protocol over SSL/TLS
Security in this case is about being sure everyone gets exactly the same repository as stored on the server, without any modifications to the sources cased by MITM. As for "smart" http, this seems pretty much cool.However, we're currently using lighthttpd, so it might be an issue. We'll check on whether "smart" http is used there, and if not guess it wouldn't be a big deal to switch to apache. On Fri, Dec 27, 2013 at 8:20 PM, Matthieu Moy wrote: > Andreas Schwab writes: > >> Sergey Sharybin writes: >> >>> So guess we just need to recommend using https:// protocol instead of >>> git:// for our users? >> >> Given how easy it is to verify the integrity of a git repository out of >> band there isn't really much of added security by using TLS for >> transport. > > You can verify integrity after the fact, but not guarantee > confidentiality ... so it again depends on the definition of "security". > > -- > Matthieu Moy > http://www-verimag.imag.fr/~moy/ -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
Matthieu Moy writes: > You can verify integrity after the fact, but not guarantee > confidentiality ... so it again depends on the definition of "security". Since the OP is talking about anonymous access there is no need for confidentiality in this case. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
On Fri, 27 Dec 2013 20:25:16 +0600 Sergey Sharybin wrote: > Security in this case is about being sure everyone gets exactly the > same repository as stored on the server, without any modifications to > the sources cased by MITM. > > As for "smart" http, this seems pretty much cool.However, we're > currently using lighthttpd, so it might be an issue. We'll check on > whether "smart" http is used there, and if not guess it wouldn't be a > big deal to switch to apache. The web server software has nothing to do with HTTP[S] used by Git being "smart", I think, it just has to be set up properly. As discussed in an earlier thread here, a good indication of the dumb version of the protocol being in use is no display of the fetching progress on the client while doing `git clone` because this information (like "compressing objects ..." etc) is sent by the server-side Git process which is only there if HTTP[S] "was smart". Otherwise the client just GETs packs of objects, traverses them, GETs more and so on, so batches of HTTP GET requests correlating to clone sessions in the web server logs should also be indicative of the problem. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
>> As for "smart" http, this seems pretty much cool.However, we're >> currently using lighthttpd, so it might be an issue. We'll check on >> whether "smart" http is used there, and if not guess it wouldn't be a >> big deal to switch to apache. > > The web server software has nothing to do with HTTP[S] used by Git being > "smart", I think, it just has to be set up properly. Misunderstood git doc then which says "it has to be Apache, currently - other CGI servers don't work, last I checked". > As discussed in an earlier thread here, a good indication of the > dumb version of the protocol being in use is no display of the > fetching progress on the client while doing `git clone` because this > information (like "compressing objects ..." etc) is sent by the > server-side Git process which is only there if HTTP[S] "was smart". > Otherwise the client just GETs packs of objects, traverses them, GETs > more and so on, so batches of HTTP GET requests correlating to clone > sessions in the web server logs should also be indicative of the > problem. Just to verify, if i see messages like "Receiving objects: 1% (7289/705777), 1.72 MiB | 340.00 KiB/s" it means server is "smart" ? -- With best regards, Sergey Sharybin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
On Fri, 27 Dec 2013 20:47:54 +0600 Sergey Sharybin wrote: [...] > > As discussed in an earlier thread here, a good indication of the > > dumb version of the protocol being in use is no display of the > > fetching progress on the client while doing `git clone` because this > > information (like "compressing objects ..." etc) is sent by the > > server-side Git process which is only there if HTTP[S] "was smart". > > Otherwise the client just GETs packs of objects, traverses them, > > GETs more and so on, so batches of HTTP GET requests correlating to > > clone sessions in the web server logs should also be indicative of > > the problem. > > Just to verify, if i see messages like "Receiving objects: 1% > (7289/705777), 1.72 MiB | 340.00 KiB/s" it means server is "smart" ? I would say yes, because your Git knows the precise number of objects to receive. Unfortunately, I won't swear by this as this was a long time ago I have seen cloning using the dumb protocol. By the way, here [1] is that discussion. 1. http://thread.gmane.org/gmane.comp.version-control.git/238933/focus=238946 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
* Sergey Sharybin [131227 15:25]: > Security in this case is about being sure everyone gets exactly the > same repository as stored on the server, without any modifications to > the sources cased by MITM. Note that ssl (and thus https) only helps here against a resource-less man-in-the-middle. Getting catch-all CA-signed certificates is said to no longer available to everyone as easily as it was some years ago, but unless you allow only one private CA (and even there clients often fail) you still should assume everyone resourceful enough to still be able to do MITM. Bernhard R. Link -- To unsubscribe from this list: send the line "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] git-svn: workaround for a bug in svn serf backend
Roman Kagan wrote: > Subversion serf backend in versions 1.8.5 and below has a bug that the > function creating the descriptor of a file change -- add_file() -- > doesn't make a copy of its third argument when storing it on the > returned descriptor. As a result, by the time this field is used (in > transactions of file copying or renaming) it may well be released, and > the memory reused. > > One of its possible manifestations is the svn assertion triggering on an > invalid path, with a message > > svn_fspath__skip_ancestor: Assertion `svn_fspath__is_canonical(child_fspath)' > failed. [...] Makes sense. Perhaps also worth mentioning that this is fixed by r1553376, but no need to reroll just for that. > Cc: Benjamin Pabst > Cc: Eric Wong > Cc: Jonathan Nieder No need for these lines --- the mail header already keeps track of who is being cc-ed. > Signed-off-by: Roman Kagan Reviewed-by: Jonathan Nieder 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] git-svn: workaround for a bug in svn serf backend
Jonathan Nieder wrote: > Roman Kagan wrote: > > > Subversion serf backend in versions 1.8.5 and below has a bug that the > > function creating the descriptor of a file change -- add_file() -- > > doesn't make a copy of its third argument when storing it on the > > returned descriptor. As a result, by the time this field is used (in > > transactions of file copying or renaming) it may well be released, and > > the memory reused. > > > > One of its possible manifestations is the svn assertion triggering on an > > invalid path, with a message > > > > svn_fspath__skip_ancestor: Assertion > > `svn_fspath__is_canonical(child_fspath)' failed. > [...] > > Makes sense. Perhaps also worth mentioning that this is fixed by > r1553376, but no need to reroll just for that. Thanks all, I noted this in an addendum to the commit: Subversion serf backend in versions 1.8.5 and below has a bug(*) that the ... * [ew: fixed in Subversion r1553376 as noted by Jonathan Nieder] > > Cc: Benjamin Pabst > > Cc: Eric Wong > > Cc: Jonathan Nieder > > No need for these lines --- the mail header already keeps track of who > is being cc-ed. I don't mind seeing it in history. At least I've gotten accustomed to it from the Linux kernel and tracking patch flow between dev -> stable trees. > > Signed-off-by: Roman Kagan > > Reviewed-by: Jonathan Nieder Signed-off-by: Eric Wong The following changes since commit 7794a680e63a2a11b73cb1194653662f2769a792: Sync with 1.8.5.2 (2013-12-17 14:12:17 -0800) are available in the git repository at: git://git.bogomips.org/git-svn.git master for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace: git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +) Roman Kagan (1): git-svn: workaround for a bug in svn serf backend perl/Git/SVN/Editor.pm | 10 -- 1 file changed, 8 insertions(+), 2 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
[PATCH] Remove the line length limit for graft files
Support for grafts predates Git's strbuf, and hence it is understandable that there was a hard-coded line length limit of 1023 characters (which was chosen a bit awkwardly, given that it is *exactly* one byte short of aligning with the 41 bytes occupied by a commit name and the following space or new-line character). While regular commit histories hardly win comprehensibility in general if they merge more than twenty-two branches in one go, it is not Git's business to limit grafts in such a way. In this particular developer's case, the use case that requires substantially longer graft lines to be supported is the visualization of the commits' order implied by their changes: commits are considered to have an implicit relationship iff exchanging them in an interactive rebase would result in merge conflicts. Thusly implied branches tend to be very shallow in general, and the resulting thicket of implied branches is usually very wide; It is actually quite common that *most* of the commits in a topic branch have not even one implied parent, so that a final merge commit has about as many implied parents as there are commits in said branch. Signed-off-by: Johannes Schindelin --- builtin/blame.c | 8 commit.c| 10 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 1407ae7..9047b6e 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb) static int read_ancestry(const char *graft_file) { FILE *fp = fopen(graft_file, "r"); - char buf[1024]; + struct strbuf buf = STRBUF_INIT; if (!fp) return -1; - while (fgets(buf, sizeof(buf), fp)) { + while (!strbuf_getwholeline(&buf, fp, '\n')) { /* The format is just "Commit Parent1 Parent2 ...\n" */ - int len = strlen(buf); - struct commit_graft *graft = read_graft_line(buf, len); + struct commit_graft *graft = read_graft_line(buf.buf, buf.len); if (graft) register_commit_graft(graft, 0); } fclose(fp); + strbuf_release(&buf); return 0; } diff --git a/commit.c b/commit.c index de16a3c..57ebea2 100644 --- a/commit.c +++ b/commit.c @@ -196,19 +196,19 @@ bad_graft_data: static int read_graft_file(const char *graft_file) { FILE *fp = fopen(graft_file, "r"); - char buf[1024]; + struct strbuf buf = STRBUF_INIT; if (!fp) return -1; - while (fgets(buf, sizeof(buf), fp)) { + while (!strbuf_getwholeline(&buf, fp, '\n')) { /* The format is just "Commit Parent1 Parent2 ...\n" */ - int len = strlen(buf); - struct commit_graft *graft = read_graft_line(buf, len); + struct commit_graft *graft = read_graft_line(buf.buf, buf.len); if (!graft) continue; if (register_commit_graft(graft, 1)) - error("duplicate graft data: %s", buf); + error("duplicate graft data: %s", buf.buf); } fclose(fp); + strbuf_release(&buf); return 0; } -- 1.8.4.msysgit.0.1109.g3c58b16 -- To unsubscribe from this list: send the line "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] Remove the line length limit for graft files
Hi, Johannes Schindelin wrote: > While regular commit histories hardly win comprehensibility in general > if they merge more than twenty-two branches in one go, it is not Git's > business to limit grafts in such a way. Fun. :) Makes sense. [...] > --- > builtin/blame.c | 8 > commit.c| 10 +- > 2 files changed, 9 insertions(+), 9 deletions(-) Is this easy to reproduce so some interested but lazy person could write a test? [...] > --- a/builtin/blame.c > +++ b/builtin/blame.c > @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb) > static int read_ancestry(const char *graft_file) > { > FILE *fp = fopen(graft_file, "r"); > - char buf[1024]; > + struct strbuf buf = STRBUF_INIT; > if (!fp) > return -1; > - while (fgets(buf, sizeof(buf), fp)) { > + while (!strbuf_getwholeline(&buf, fp, '\n')) { If there is no newline at EOF, this will skip the last line, while the old behavior was to pay attention to it. I haven't thought through whether that's a good or bad change. Maybe it should just be documented? [...] > --- a/commit.c > +++ b/commit.c > @@ -196,19 +196,19 @@ static int read_graft_file(const char *graft_file) [...] > - while (fgets(buf, sizeof(buf), fp)) { > + while (!strbuf_getwholeline(&buf, fp, '\n')) { Likewise. The rest of the patch looks good. Merry christmas, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove the line length limit for graft files
Hi, On Fri, 27 Dec 2013, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > [...] > > --- > > builtin/blame.c | 8 > > commit.c| 10 +- > > 2 files changed, 9 insertions(+), 9 deletions(-) > > Is this easy to reproduce so some interested but lazy person could > write a test? Yep. Make 25 orphan commits, add a graft line to make the first a merge of the rest. > > --- a/builtin/blame.c > > +++ b/builtin/blame.c > > @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb) > > static int read_ancestry(const char *graft_file) > > { > > FILE *fp = fopen(graft_file, "r"); > > - char buf[1024]; > > + struct strbuf buf = STRBUF_INIT; > > if (!fp) > > return -1; > > - while (fgets(buf, sizeof(buf), fp)) { > > + while (!strbuf_getwholeline(&buf, fp, '\n')) { > > If there is no newline at EOF, this will skip the last line, while the > old behavior was to pay attention to it. I haven't thought through > whether that's a good or bad change. Maybe it should just be > documented? The way I read int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term) { int ch; if (feof(fp)) return EOF; strbuf_reset(sb); while ((ch = fgetc(fp)) != EOF) { strbuf_grow(sb, 1); sb->buf[sb->len++] = ch; if (ch == term) break; } if (ch == EOF && sb->len == 0) return EOF; sb->buf[sb->len] = '\0'; return 0; } it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters have been read before hitting EOF. In other words, strbuf_getwholeline() -- despite requiring an explicit terminating character argument -- does not require the last line to end with that terminating character. A quick test (in my case, because I am lazy, modifying test-mergesort.c to output the lines that were read by strbuf_getwholeline()) also confirms my suspicion. Or maybe I missed something? Ciao, Dscho -- To unsubscribe from this list: send the line "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] Remove the line length limit for graft files
Johannes Schindelin writes: > Support for grafts predates Git's strbuf, and hence it is understandable > that there was a hard-coded line length limit of 1023 characters (which > was chosen a bit awkwardly, given that it is *exactly* one byte short of > aligning with the 41 bytes occupied by a commit name and the following > space or new-line character). > > While regular commit histories hardly win comprehensibility in general > if they merge more than twenty-two branches in one go, it is not Git's > business to limit grafts in such a way. > > In this particular developer's case, the use case that requires > substantially longer graft lines to be supported is the visualization of > the commits' order implied by their changes: commits are considered to > have an implicit relationship iff exchanging them in an interactive > rebase would result in merge conflicts. > > Thusly implied branches tend to be very shallow in general, and the > resulting thicket of implied branches is usually very wide; It is > actually quite common that *most* of the commits in a topic branch have > not even one implied parent, so that a final merge commit has about as > many implied parents as there are commits in said branch. > > Signed-off-by: Johannes Schindelin > --- > builtin/blame.c | 8 > commit.c| 10 +- > 2 files changed, 9 insertions(+), 9 deletions(-) Makes sense. It is in line with the spirit of ef98c5cafb3e (commit-tree: lift completely arbitrary limit of 16 parents, 2008-06-27), too ;-) Thanks, will queue. > diff --git a/builtin/blame.c b/builtin/blame.c > index 1407ae7..9047b6e 100644 > --- a/builtin/blame.c > +++ b/builtin/blame.c > @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb) > static int read_ancestry(const char *graft_file) > { > FILE *fp = fopen(graft_file, "r"); > - char buf[1024]; > + struct strbuf buf = STRBUF_INIT; > if (!fp) > return -1; > - while (fgets(buf, sizeof(buf), fp)) { > + while (!strbuf_getwholeline(&buf, fp, '\n')) { > /* The format is just "Commit Parent1 Parent2 ...\n" */ > - int len = strlen(buf); > - struct commit_graft *graft = read_graft_line(buf, len); > + struct commit_graft *graft = read_graft_line(buf.buf, buf.len); > if (graft) > register_commit_graft(graft, 0); > } > fclose(fp); > + strbuf_release(&buf); > return 0; > } > > diff --git a/commit.c b/commit.c > index de16a3c..57ebea2 100644 > --- a/commit.c > +++ b/commit.c > @@ -196,19 +196,19 @@ bad_graft_data: > static int read_graft_file(const char *graft_file) > { > FILE *fp = fopen(graft_file, "r"); > - char buf[1024]; > + struct strbuf buf = STRBUF_INIT; > if (!fp) > return -1; > - while (fgets(buf, sizeof(buf), fp)) { > + while (!strbuf_getwholeline(&buf, fp, '\n')) { > /* The format is just "Commit Parent1 Parent2 ...\n" */ > - int len = strlen(buf); > - struct commit_graft *graft = read_graft_line(buf, len); > + struct commit_graft *graft = read_graft_line(buf.buf, buf.len); > if (!graft) > continue; > if (register_commit_graft(graft, 1)) > - error("duplicate graft data: %s", buf); > + error("duplicate graft data: %s", buf.buf); > } > fclose(fp); > + strbuf_release(&buf); > return 0; > } > > -- > 1.8.4.msysgit.0.1109.g3c58b16 > > -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2013, #05; Thu, 26)
On Thu, Dec 26, 2013 at 4:08 PM, Junio C Hamano wrote: > Here are the topics that have been cooking. Commits prefixed with > '-' are only in 'pu' (proposed updates) while commits prefixed with > '+' are in 'next'. > > You can find the changes described here in the integration branches > of the repositories listed at > > http://git-blame.blogspot.com/p/git-public-repositories.html > > -- > [New Topics] Would $gmane/239575 [1] be of interest for "New Topics"? [1]: http://article.gmane.org/gmane.comp.version-control.git/239575/ -- To unsubscribe from this list: send the line "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] Remove the line length limit for graft files
Johannes Schindelin wrote: > it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters > have been read before hitting EOF. Yep. api-strbuf.txt even says so. Sorry for the nonsense. For what it's worth, Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2013, #05; Thu, 26)
Eric Sunshine writes: > On Thu, Dec 26, 2013 at 4:08 PM, Junio C Hamano wrote: >> Here are the topics that have been cooking. Commits prefixed with >> '-' are only in 'pu' (proposed updates) while commits prefixed with >> '+' are in 'next'. >> >> You can find the changes described here in the integration branches >> of the repositories listed at >> >> http://git-blame.blogspot.com/p/git-public-repositories.html >> >> -- >> [New Topics] > > Would $gmane/239575 [1] be of interest for "New Topics"? > > [1]: http://article.gmane.org/gmane.comp.version-control.git/239575/ Actually I was planning to scoop it up directly to master but forgot to do so. Running "git diff maint pu -- name-hash.c" shows that we have added a comment that mentions index_name_exists---that needs to be adjusted, too, by the way. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
Konstantin Khomoutov writes: > On Fri, 27 Dec 2013 18:59:00 +0600 > Sergey Sharybin wrote: > >> Quick question is, is it possible to use git:// protocol over >> SSL/TLS/other secure transport? > > The Git protocol does not implement it itself but you can channel it > over a TLS tunnel (via stunnel for instance). Unfortunately, this > means a specialized software and setup on both ends so if the question > was about a general client using stock Git then the answer is no, it's > impossible. Hmph, I somehow had an impression that you wouldn't need anything more complex than a simple helper that uses git-remote-ext on the client side. On the remote end, you'd need to have something that terminates the incoming SSL/TLS and plugs it to your git daemon. > >> Or the recommended way to do secure anonymous checkout is to simply >> use https:// ? > > Yes, but it will only be secure if you've managed to verify the > server's certificate and do trust its issuer (or a CA higher up the > cert's trust chain) -- people tend to confuse "encrypted" with > "secure" which is not at all the same thing. -- To unsubscribe from this list: send the line "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] git-svn: workaround for a bug in svn serf backend
Eric Wong writes: > Jonathan Nieder wrote: >> Roman Kagan wrote: >> >> > Subversion serf backend in versions 1.8.5 and below has a bug that the >> > function creating the descriptor of a file change -- add_file() -- >> > doesn't make a copy of its third argument when storing it on the >> > returned descriptor. As a result, by the time this field is used (in >> > transactions of file copying or renaming) it may well be released, and >> > the memory reused. >> > >> > One of its possible manifestations is the svn assertion triggering on an >> > invalid path, with a message >> > >> > svn_fspath__skip_ancestor: Assertion >> > `svn_fspath__is_canonical(child_fspath)' failed. >> [...] >> >> Makes sense. Perhaps also worth mentioning that this is fixed by >> r1553376, but no need to reroll just for that. > > Thanks all, I noted this in an addendum to the commit: > > Subversion serf backend in versions 1.8.5 and below has a bug(*) that the > > ... > > * [ew: fixed in Subversion r1553376 as noted by Jonathan Nieder] > >> > Cc: Benjamin Pabst >> > Cc: Eric Wong >> > Cc: Jonathan Nieder >> >> No need for these lines --- the mail header already keeps track of who >> is being cc-ed. > > I don't mind seeing it in history. At least I've gotten accustomed to > it from the Linux kernel and tracking patch flow between dev -> stable > trees. > >> > Signed-off-by: Roman Kagan >> >> Reviewed-by: Jonathan Nieder > > Signed-off-by: Eric Wong > > > The following changes since commit 7794a680e63a2a11b73cb1194653662f2769a792: > > Sync with 1.8.5.2 (2013-12-17 14:12:17 -0800) > > are available in the git repository at: > > > git://git.bogomips.org/git-svn.git master > > for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace: > > git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 > +) > > > Roman Kagan (1): > git-svn: workaround for a bug in svn serf backend > > perl/Git/SVN/Editor.pm | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) Thanks. I almost missed this pull-request, though. Will pull. -- To unsubscribe from this list: send the line "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] Remove the line length limit for graft files
Johannes Schindelin wrote: > On Fri, 27 Dec 2013, Jonathan Nieder wrote: >> Is this easy to reproduce so some interested but lazy person could >> write a test? > > Yep. Make 25 orphan commits, add a graft line to make the first a merge of > the rest. Thanks. Here's a pair of tests doing that. Signed-off-by: Jonathan Nieder --- t/annotate-tests.sh | 21 + t/t6101-rev-parse-parents.sh | 16 +++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index c9d105d..304c7b7 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -116,6 +116,27 @@ test_expect_success 'blame evil merge' ' check_count A 2 B 1 B1 2 B2 1 "A U Thor" 1 ' +test_expect_success 'blame huge graft' ' + test_when_finished "git checkout branch2" && + test_when_finished "rm -f .git/info/grafts" && + graft= && + for i in 0 1 2 + do + for j in 0 1 2 3 4 5 6 7 8 9 + do + git checkout --orphan "$i$j" && + printf "%s\n" "$i" "$j" >file && + test_tick && + GIT_AUTHOR_NAME=$i$j GIT_AUTHOR_EMAIL=$i$j...@test.git \ + git commit -a -m "$i$j" && + commit=$(git rev-parse --verify HEAD) && + graft="$graft$commit " + done + done && + printf "%s " $graft >.git/info/grafts && + check_count -h 00 01 1 10 1 +' + test_expect_success 'setup incomplete line' ' echo "incomplete" | tr -d "\\012" >>file && GIT_AUTHOR_NAME="C" GIT_AUTHOR_EMAIL="c...@test.git" \ diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh index 7ea14ce..10b1452 100755 --- a/t/t6101-rev-parse-parents.sh +++ b/t/t6101-rev-parse-parents.sh @@ -20,7 +20,17 @@ test_expect_success 'setup' ' test_commit start2 && git checkout master && git merge -m next start2 && - test_commit final + test_commit final && + + test_seq 40 | + while read i + do + git checkout --orphan "b$i" && + test_tick && + git commit --allow-empty -m "$i" && + commit=$(git rev-parse --verify HEAD) && + printf "$commit " >>.git/info/grafts + done ' test_expect_success 'start is valid' ' @@ -79,6 +89,10 @@ test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' ' test_cmp expect actual ' +test_expect_success 'large graft octopus' ' + test_cmp_rev_output b31 "git rev-parse --verify b1^30" +' + test_expect_success 'repack for next test' ' git repack -a -d ' -- 1.8.5.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: [PATCH] Remove the line length limit for graft files
Hi Jonathan, On Fri, 27 Dec 2013, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > > it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters > > have been read before hitting EOF. > > Yep. api-strbuf.txt even says so. I never bothered to look ;-) > Sorry for the nonsense. Nope, no nonsense at all. I actually had a look only after your review, and it definitely makes sense to double-check. > For what it's worth, > Reviewed-by: Jonathan Nieder It is worth a lot. Believe me, I know just how thankless a task reviewing is, and instead of getting praise for it, you even get abused by contributors who would rather self-review their code for fear of having a twist in their knockers pointed out publicly. Your review makes the code better, even if it does not result in code changes all the time: it increases the confidence that the code is good. Thank you, Jonathan! Dscho -- To unsubscribe from this list: send the line "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] Remove the line length limit for graft files
Hi Jonathan, On Fri, 27 Dec 2013, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > On Fri, 27 Dec 2013, Jonathan Nieder wrote: > > >> Is this easy to reproduce so some interested but lazy person could > >> write a test? > > > > Yep. Make 25 orphan commits, add a graft line to make the first a merge of > > the rest. > > Thanks. Here's a pair of tests doing that. Thank you very much! > + for i in 0 1 2 > + do > + for j in 0 1 2 3 4 5 6 7 8 9 > + do for the record, I usually prefer i=0 while test $i -t 30 do ... i=$(($i+1)) done but your code is just as good! Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Darlehen Angebot !
Wir bieten persönliche und geschäftliche Kredite ohne Sicherheiten (nur Identifikation) mit 3 % Zinssatz, von ? 10.000, ? 90,000,000 in 1 Jahr auf 20 Jahre Laufzeit. !!bewerben Sie sich jetzt! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
On Fri, Dec 27, 2013 at 08:25:16PM +0600, Sergey Sharybin wrote: > Security in this case is about being sure everyone gets exactly the > same repository as stored on the server, without any modifications to > the sources cased by MITM. Besides security, HTTPS is more likely to work across different firewalls and proxies, since "odd" ports like 9418 are often blocked and HTTPS usually isn't subject to the weirdness of proxies (since they can't inspect it or modify it). > As for "smart" http, this seems pretty much cool.However, we're > currently using lighthttpd, so it might be an issue. We'll check on > whether "smart" http is used there, and if not guess it wouldn't be a > big deal to switch to apache. You can use Lighttpd if you like. See Documentation/git-http-backend.txt (or git http-backend --help). -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] Remove the line length limit for graft files
Jonathan Nieder writes: > Johannes Schindelin wrote: >> On Fri, 27 Dec 2013, Jonathan Nieder wrote: > >>> Is this easy to reproduce so some interested but lazy person could >>> write a test? >> >> Yep. Make 25 orphan commits, add a graft line to make the first a merge of >> the rest. > > Thanks. Here's a pair of tests doing that. > > Signed-off-by: Jonathan Nieder > --- > t/annotate-tests.sh | 21 + > t/t6101-rev-parse-parents.sh | 16 +++- > 2 files changed, 36 insertions(+), 1 deletion(-) Makes sense. Thanks, both. Small lint-picking like this change to perfect the system, as opposed to earth-shattering new shinies, tend to often get neglected but are very much appreciated. > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh > index c9d105d..304c7b7 100644 > --- a/t/annotate-tests.sh > +++ b/t/annotate-tests.sh > @@ -116,6 +116,27 @@ test_expect_success 'blame evil merge' ' > check_count A 2 B 1 B1 2 B2 1 "A U Thor" 1 > ' > > +test_expect_success 'blame huge graft' ' > + test_when_finished "git checkout branch2" && > + test_when_finished "rm -f .git/info/grafts" && > + graft= && > + for i in 0 1 2 > + do > + for j in 0 1 2 3 4 5 6 7 8 9 > + do > + git checkout --orphan "$i$j" && > + printf "%s\n" "$i" "$j" >file && > + test_tick && > + GIT_AUTHOR_NAME=$i$j GIT_AUTHOR_EMAIL=$i$j...@test.git \ > + git commit -a -m "$i$j" && > + commit=$(git rev-parse --verify HEAD) && > + graft="$graft$commit " > + done > + done && > + printf "%s " $graft >.git/info/grafts && > + check_count -h 00 01 1 10 1 > +' > + > test_expect_success 'setup incomplete line' ' > echo "incomplete" | tr -d "\\012" >>file && > GIT_AUTHOR_NAME="C" GIT_AUTHOR_EMAIL="c...@test.git" \ > diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh > index 7ea14ce..10b1452 100755 > --- a/t/t6101-rev-parse-parents.sh > +++ b/t/t6101-rev-parse-parents.sh > @@ -20,7 +20,17 @@ test_expect_success 'setup' ' > test_commit start2 && > git checkout master && > git merge -m next start2 && > - test_commit final > + test_commit final && > + > + test_seq 40 | > + while read i > + do > + git checkout --orphan "b$i" && > + test_tick && > + git commit --allow-empty -m "$i" && > + commit=$(git rev-parse --verify HEAD) && > + printf "$commit " >>.git/info/grafts > + done > ' > > test_expect_success 'start is valid' ' > @@ -79,6 +89,10 @@ test_expect_success 'final^1^! = final^1 ^final^1^1 > ^final^1^2' ' > test_cmp expect actual > ' > > +test_expect_success 'large graft octopus' ' > + test_cmp_rev_output b31 "git rev-parse --verify b1^30" > +' > + > test_expect_success 'repack for next test' ' > git repack -a -d > ' > -- > 1.8.5.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: [PATCH v2 00/21] Support multiple worktrees
On Fri, Dec 27, 2013 at 12:12 AM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Sun, Dec 22, 2013 at 1:38 PM, Junio C Hamano wrote: >> >>> Do we even need to expose them as ref-like things as a part of the >>> external API/UI in the first place? For end-user scripts that want >>> to operate in a real or borrowing worktree, there should be no >>> reason to touch this "other" repository directly. Things like "if >>> one of the wortrees tries to check out a branch that is already >>> checked out elsewhere, error out" policy may need to consult the >>> other worktrees via $GIT_COMMON_DIR mechanism but at that level we >>> have all the control without contaminating end-user facing ref >>> namespace in a way main/FETCH_HEAD... do. >> >> No, external API/UI is just extra bonus. We need to (or should) do so >> in order to handle $GIT_COMMON_DIR/HEAD exactly like how we do normal >> refs. > > And that is what I consider a confusion-trigger, not a nice bonus. > > I do not think it is particularly a good idea to contaminate > end-user namespace for this kind of "peek another repository" > special operation. > > In order to handle your "multiple worktrees manipulating the same > branch" case sanely, you need to be aware of not just the real > repository your worktree is borrowing from, but also _all_ the other > worktrees that borrow from that same real repository, so a single > "main" virtual namespace will not cut it. You will be dealing with > an unbounded set of HEADs, one for each such worktree. Yes. My problem is, while all secondary worktrees are in $GIT_DIR/repos and their HEADs can be accessed there with "repos/xxx/HEAD", the first worktree's HEAD can't be accessed this way because "HEAD" in a linked checkouts means repos//HEAD. > Can't we do this by adding a "with this real repository" layer, > e.g. "resolve HEAD wrt that repository", somewhat similar to how we > peek into submodule namespaces? Hmm.. never thought of it like a "submodule". Thanks for the idea. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html