Re: commit-message attack for extracting sensitive data from rewritten Git history
Am 4/8/2013 23:54, schrieb Jeff King: Yeah, it would make sense for filter-branch to have a --map-commit-ids option or similar that does the update. At first I thought it might take two passes, but I don't think it is necessary, as long as we traverse the commits topologically (i.e., you cannot have mentioned X in a commit that is an ancestor of X, so you do not have to worry about mapping it until after it has been processed). Topological traversal is not sufficient. Consider this history: o--A--o-- / / --o--B--o If A mentions B (think of cherry-pick -x), then you must ensure that the branch containing B was traversed first. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Using a bit more decoration
On Mon, Apr 08, 2013 at 09:54:50PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: The tricky part is how to store the index for each commit (or object). I don't see a way around adding a new field to struct commit to do so. We could reuse the space indegree used to live at; that is a one-word space already, and I really do not need encoding. It was no more than while we are at it, we could add different things here demonstration. Here is what my proposal would look like, replacing indegree with the index field. In my timings of git rev-list --topo-order --all, I couldn't measure any difference. Now the next tricky step will actually be writing a paint_down_to_common that uses it. :) The auto-growing slab implementation below assumes a fixed width of one int per commit. I think we'd want something similar that stores a set of N bits per commit, where N is fixed at the start of the algorithm. --- commit.c | 59 +-- commit.h | 2 +- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/commit.c b/commit.c index 516a4ff..95f041a 100644 --- a/commit.c +++ b/commit.c @@ -14,6 +14,7 @@ const char *commit_type = commit; int save_commit_buffer = 1; const char *commit_type = commit; +static int commit_count; static struct commit *check_commit(struct object *obj, const unsigned char *sha1, @@ -58,8 +59,11 @@ struct commit *lookup_commit(const unsigned char *sha1) struct commit *lookup_commit(const unsigned char *sha1) { struct object *obj = lookup_object(sha1); - if (!obj) - return create_object(sha1, OBJ_COMMIT, alloc_commit_node()); + if (!obj) { + struct commit *c = alloc_commit_node(); + c-index = commit_count++; + return create_object(sha1, OBJ_COMMIT, c); + } if (!obj-type) obj-type = OBJ_COMMIT; return check_commit(obj, sha1, 0); @@ -506,6 +510,36 @@ struct commit *pop_commit(struct commit_list **stack) return item; } +struct commit_slab { + int *buf; + int alloc; +}; + +static void slab_init(struct commit_slab *s) +{ + memset(s, 0, sizeof(*s)); +} + +static void slab_clear(struct commit_slab *s) +{ + free(s-buf); + slab_init(s); +} + +static inline int *slab_at(struct commit_slab *s, const struct commit *c) +{ + if (s-alloc = c-index) { + int new_alloc = alloc_nr(s-alloc); + if (new_alloc = c-index) + new_alloc = c-index + 1; + + s-buf = xrealloc(s-buf, new_alloc * sizeof(*s-buf)); + memset(s-buf + s-alloc, 0, new_alloc - s-alloc); + s-alloc = new_alloc; + } + return s-buf + c-index; +} + /* * Performs an in-place topological sort on the list supplied. */ @@ -514,15 +548,18 @@ void sort_in_topological_order(struct commit_list ** list, int lifo) struct commit_list *next, *orig = *list; struct commit_list *work, **insert; struct commit_list **pptr; + struct commit_slab indegree; if (!orig) return; *list = NULL; + slab_init(indegree); + /* Mark them and clear the indegree */ for (next = orig; next; next = next-next) { struct commit *commit = next-item; - commit-indegree = 1; + *slab_at(indegree, commit) = 1; } /* update the indegree */ @@ -530,9 +567,10 @@ void sort_in_topological_order(struct commit_list ** list, int lifo) struct commit_list * parents = next-item-parents; while (parents) { struct commit *parent = parents-item; + int *pi = slab_at(indegree, parent); - if (parent-indegree) - parent-indegree++; + if (*pi) + (*pi)++; parents = parents-next; } } @@ -549,7 +587,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo) for (next = orig; next; next = next-next) { struct commit *commit = next-item; - if (commit-indegree == 1) + if (*slab_at(indegree, commit) == 1) insert = commit_list_insert(commit, insert)-next; } @@ -570,8 +608,9 @@ void sort_in_topological_order(struct commit_list ** list, int lifo) commit = work_item-item; for (parents = commit-parents; parents ; parents = parents-next) { struct commit *parent = parents-item; + int *pi = slab_at(indegree, parent); - if (!parent-indegree) + if (!*pi) continue; /* @@ -579,7 +618,7 @@ void
Re: RFC: Very useful script to SVG graph the git commits from a file orientated view
looking a little bit more into this, I was very suprised there seems to be little/no tools in the git ecosystem that studies the dependencies between commits based on the file they modified and/or the conflict they would cause. Is there any pre-existing tool to do that ? It can be done with git-log --name-only(the graph_git.pl is just a graphing layer above that command) but i'm suprised that I couldn't find anything else And that was at the file level, is there any tool to help find what commits can be reordered without causing conflicts ? I am not sure if there is an easy way to extract potential conflict information from git... Regards Jérémy Rosen fight key loggers : write some perl using vim - Mail original - Hi Jeremy, It would be great if you could send your email again to the list, so that other people can see that there is interest in my script :) Makes it easier for me to get it included. * The tooltips are very handy, but it would be nice if the tooltip would activate on the whole commit ellipsis, not just the text inside the ellipsis Yes, tooltips are a real pain. I don't know how to do that. * I would love to have tooltips on the arrows too. when trying to follow what arrow is what file it makes things really handi Actually they do have tooltips - it's just that the arrows are really thin so you have to be very accurate with your mouse pointing! It would be nice if there were a way to filter only some files in the output... there probably is with the git-log like syntax but i'm not a master of that... hints would be welcome Try just specifying the files: $ graph_git.pl -10 filename1 filename2 I haven't tested, but it should work is there a public repo for this script so I can point other people to it ? No - any suggestions as to where to put it are welcome :) John -- To unsubscribe from this list: send the line 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] t3700 (add): add failing test for add with submodules
Jeff King wrote: That is not actually a submodule, but rather just a repo that happens to be inside our working tree. I know the distinction is subtle, but according to the thread I linked to above, we may actually treat paths with gitlinked index entries separately already (I did not try it, though). Agreed. treat_gitlink() dies if there is a gitlink cache_entry matching any of the pathspecs; it does one thing well, and promises what it does: however, its core logic in check_path_for_gitlink() can easily be moved into lstat_cache_matchlen() as that is more generic (checks index and worktree). die_if_path_beyond_symlink() is the perfect example to replicate. Today, there is one more caller of die_if_path_beyond_symlink(): check-ignore, so we must patch that too. On a slightly related note treat_path() also contains the logic for checking for a git repository in the worktree. Unfortunately, the code cannot be reused because it checks for a '.git' in a dirent. On the wording issue, a submodule is a submodule whether in-index or otherwise. I would write two different tests: one for in-worktree submodule and another for in-index submodule, and name them appropriately. Does that make sense? -- To unsubscribe from this list: send the line 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 0/2] Fix git add with submodules
Hi, git add has a bug when operating on submodules. The following test fails: mkdir submodule_dir ( cd submodule_dir git init cat foo -\EOF Some content EOF git add foo git commit -a -m Add foo ) test_must_fail git add submodule_dir/foo [1/2] adds this failing test along with a passing related test. [2/2] fixes the failing test. Ramkumar Ramachandra (2): t3700 (add): add two tests for testing add with submodules add: refuse to add paths beyond repository boundaries builtin/add.c | 5 +++-- cache.h| 2 ++ pathspec.c | 12 pathspec.h | 1 + symlinks.c | 43 +-- t/t3700-add.sh | 32 6 files changed, 87 insertions(+), 8 deletions(-) -- 1.8.2.1.347.gdd82260.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t3700 (add): add two tests for testing add with submodules
The first test git add should not go past gitlink boundaries checks that paths within a submodule added with 'git submodule add' cannot be added to the index. It passes because of treat_gitlink() in builtin/add.c. The second test git add should not go past git repository boundaries checks that paths within a git repository in the worktree (not yet added with 'git submodule add') cannot be added to the index. It fails because there is no existing code to check this. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t3700-add.sh | 32 1 file changed, 32 insertions(+) diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 874b3a6..1ad2331 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -310,4 +310,36 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out test_i18ncmp expect.err actual.err ' +test_expect_success 'git add should not go past gitlink boundaries' ' + rm -rf submodule_dir + mkdir submodule_dir + ( + cd submodule_dir + git init + git config remote.origin.url quux + cat foo -\EOF + Some content + EOF + git add foo + git commit -a -m Add foo + ) + git submodule add ./submodule_dir + test_must_fail git add submodule_dir/foo +' + +test_expect_failure 'git add should not go past git repository boundaries' ' + rm -rf submodule_dir + mkdir submodule_dir + ( + cd submodule_dir + git init + cat foo -\EOF + Some content + EOF + git add foo + git commit -a -m Add foo + ) + test_must_fail git add submodule_dir/foo +' + test_done -- 1.8.2.1.347.gdd82260.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] add: refuse to add paths beyond repository boundaries
Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a git repository in the worktree still adds fine, and this is a bug. The logic for denying it is very similar to denying adding paths beyond symbolic links: die_if_path_beyond_symlink(). Follow its example and write a die_if_path_beyond_gitrepo() to fix this bug. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/add.c | 5 +++-- cache.h| 2 ++ pathspec.c | 12 pathspec.h | 1 + symlinks.c | 43 +-- t/t3700-add.sh | 2 +- 6 files changed, 56 insertions(+), 9 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index ab1c9e8..1538129 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -155,8 +155,8 @@ static void refresh(int verbose, const char **pathspec) /* * Normalizes argv relative to prefix, via get_pathspec(), and then - * runs die_if_path_beyond_symlink() on each path in the normalized - * list. + * runs die_if_path_beyond_symlink() and die_if_path_beyond_repository() + * on each path in the normalized list. */ static const char **validate_pathspec(const char **argv, const char *prefix) { @@ -166,6 +166,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix) const char **p; for (p = pathspec; *p; p++) { die_if_path_beyond_symlink(*p, prefix); + die_if_path_beyond_gitrepo(*p, prefix); } } diff --git a/cache.h b/cache.h index e1e8ce8..987d7f3 100644 --- a/cache.h +++ b/cache.h @@ -962,6 +962,8 @@ struct cache_def { extern int has_symlink_leading_path(const char *name, int len); extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int); +extern int has_gitrepo_leading_path(const char *name, int len); +extern int threaded_has_gitrepo_leading_path(struct cache_def *, const char *, int); extern int check_leading_path(const char *name, int len); extern int has_dirs_only_path(const char *name, int len, int prefix_len); extern void schedule_dir_for_removal(const char *name, int len); diff --git a/pathspec.c b/pathspec.c index 284f397..142631d 100644 --- a/pathspec.c +++ b/pathspec.c @@ -99,3 +99,15 @@ void die_if_path_beyond_symlink(const char *path, const char *prefix) die(_('%s' is beyond a symbolic link), path + len); } } + +/* + * Dies if the given path refers to a file inside a directory with a + * git repository in it. + */ +void die_if_path_beyond_gitrepo(const char *path, const char *prefix) +{ + if (has_gitrepo_leading_path(path, strlen(path))) { + int len = prefix ? strlen(prefix) : 0; + die(_('%s' is beyond a git repository), path + len); + } +} diff --git a/pathspec.h b/pathspec.h index db0184a..c201c7b 100644 --- a/pathspec.h +++ b/pathspec.h @@ -5,5 +5,6 @@ extern char *find_pathspecs_matching_against_index(const char **pathspec); extern void add_pathspec_matches_against_index(const char **pathspec, char *seen, int specs); extern const char *check_path_for_gitlink(const char *path); extern void die_if_path_beyond_symlink(const char *path, const char *prefix); +extern void die_if_path_beyond_gitrepo(const char *path, const char *prefix); #endif /* PATHSPEC_H */ diff --git a/symlinks.c b/symlinks.c index c2b41a8..e551dae 100644 --- a/symlinks.c +++ b/symlinks.c @@ -54,6 +54,7 @@ static inline void reset_lstat_cache(struct cache_def *cache) #define FL_LSTATERR (1 3) #define FL_ERR (1 4) #define FL_FULLPATH (1 5) +#define FL_GITREPO (1 6) /* * Check if name 'name' of length 'len' has a symlink leading @@ -142,8 +143,22 @@ static int lstat_cache_matchlen(struct cache_def *cache, if (errno == ENOENT) *ret_flags |= FL_NOENT; } else if (S_ISDIR(st.st_mode)) { - last_slash_dir = last_slash; - continue; + /* Check to see if the directory contains a + git repository */ + struct stat st; + struct strbuf dotgitentry = STRBUF_INIT; + strbuf_addf(dotgitentry, %s/.git, cache-path); + if (lstat(dotgitentry.buf, st) 0) { + if (errno == ENOENT) { + strbuf_release(dotgitentry); + last_slash_dir = last_slash; + continue; + } + *ret_flags = FL_LSTATERR; + } + else + *ret_flags = FL_GITREPO; +
***** SCSI 2013 ***** New Deadline for SCSI2013. Indexing in ISI, EI Compendex, SCOPUS, INSPEC (IET) etc... Publication of accepted papers in Journals or Chapters in Springer Verlag, World
*SCSI 2013* The 2013 International Conference on Systems, Control, Signal Processing and Informatics July 16-19, 2013, Rhodes (Rodos) Island, Greece http://www.europment.org/conf2013/scsi.htm PUBLICATIONS: Extended Versions of Selected papers (about 40%) will be published in Journals of NOVA Publishers or as Chapters in Books of Springer Verlag, World Scientific, Imperial College Press, IOS Press, IGI Global and others. These Books (hard copies) of Springer Verlag, World Scientific, Imperial College Press, IOS Press, IGI Global, etc will be sent to all authors after the conference. These publishers will take care of Indexing of these Books in all major indexes (including ISI). Samples from our previous work with SPRINGER: http://www.springer.com/?SGWID=0-102-24-0-0searchType=EASY_CDAqueryText=Mastorakis Samples from our previous work with WORLDSCIENTIFIC http://www.worldscientific.com/action/doSearch?searchType=normalsearchText=MastorakispublicationFilterSearch=all Samples from our previous work with NOVA Publishers https://www.novapublishers.com/catalog/advanced_search_result.php?keywords=MastorakisosCsid=85c084d2f14555a3366fac07495f9b98x=0y=0 The Proceedings of the Conferences (all the accepted and registered papers; not only the selected ones to be sent to the previous publishers) with all the accepted and registered papers of the conferences will be sent for indexing to: ISI (Thomson Reuters), ELSEVIER, SCOPUS, ACM - Association for Computing Machinery, Zentralblatt MATH, British Library, EBSCO, SWETS, EMBASE, CAS - American Chemical Society, EI Compendex, Engineering Village, DoPP, GEOBASE, Biobase, TIB|UB - German National Library of Science and Technology, American Mathematical Society (AMS), Inspec - The IET, Ulrich's International Periodicals Directory. The conferences will feature tutorials, technical paper presentations, workshops and distinguished keynote speeches Organizing Committee General Chairs - Professor Charles A. Long Professor Emeritus University of Wisconsin Stevens Point, Wisconsin, USA Professor Nikos E. Mastorakis Military Institutes of University Education (ASEI) Hellenic Naval Academy Sector of Electrical Engineering and Computer Science Piraeus, Greece -also with- Technical University of Sofia 1000 Sofia, Bulgaria Professor Valeri Mladenov Technical University of Sofia 1000 Sofia, Bulgaria Senior Program Chair Professor Philippe Dondon ENSEIRB Rue A Schweitzer 33400 Talence France Program Chairs - Professor Filippo Neri Dipartimento di Informatica e Sistemistica University of Naples Federico II Naples, Italy Professor Hamid Reza Karimi Department of Engineering Faculty of Engineering and Science University of Agder, N-4898 Grimstad Norway Professor Sandra Sendra Instituto de Inv. para la Gestión Integrada de Zonas Costeras (IGIC) Universidad Politécnica de Valencia, Spain Tutorials Chair -- Professor Pradip Majumdar Department of Mechanical Engineering Northern Illinois University Dekalb, Illinois, USA Special Session Chair Professor Pavel Varacha Tomas Bata University in Zlin Faculty of Applied Informatics Department of Informatics and Artificial Intelligence Zlin, Czech Republic Workshops Chair -- Professor Ryszard S. Choras Institute of Telecommunications University of Technology Life Sciences Bydgoszcz, Poland Local Organizing Chair - Assistant Professor Klimis Ntalianis, Technological Educat. Inst. of Athens (TEI), Athens, Greece Publication Chair Professor Gen Qi Xu Department of Mathematics Tianjin University Tianjin, China Publicity Committee -- Professor Reinhard Neck Department of Economics Klagenfurt University Klagenfurt, Austria Professor Myriam Lazard Institut Superieur d' Ingenierie de la Conception Saint Die, France International Liaisons, France Professor Ka-Lok Ng Department of Bioinformatics Asia University Taichung, Taiwan Professor Olga Martin Applied Sciences Faculty Politehnica University of Bucharest Romania Professor Vincenzo Niola Departement of Mechanical Engineering for Energetics University of Naples Federico II Naples, Italy Professor Eduardo Mario Dias Electrical Energy and Automation Engineering Department Escola Politecnica da Universidade de Sao Paulo Brazil Steering Committee --- Professor Aida Bulucea, University of Craiova, Romania Professor Dana Simian, Univ. of Sibiu, Sibiu, Romania Professor Zoran Bojkovic, Univ. of Belgrade, Serbia Professor Metin Demiralp, Istanbul Technical University, Turkey Professor F. V. Topalis, Nat. Tech. Univ. of Athens, Greece Professor Imre Rudas, Obuda University, Budapest, Hungary Program Committee Prof. Lotfi Zadeh (IEEE Fellow,University of Berkeley, USA) Prof. Leon Chua (IEEE Fellow,University of Berkeley, USA) Prof. Michio Sugeno (RIKEN Brain Science Institute
Re: [PATCH 1/4] gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, commitdiff_plain, and patch
On 08.04.2013, Junio C Hamano wrote: j...@blackdown.de (Jürgen Kreileder) writes: Fixes the encoding for several _plain actions and for text/* and */*+xml blobs. Signed-off-by: Jürgen Kreileder j...@blackdown.de I see that this patch does (or tries to do) two _independent_ things, and should be split into at least two separate commits: --- Thanks, will queue but not hold until I hear something from Jakub. gitweb/gitweb.perl |8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 1309196..9cfe5b5 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -3823,7 +3823,7 @@ sub blob_contenttype { my ($fd, $file_name, $type) = @_; $type ||= blob_mimetype($fd, $file_name); -if ($type eq 'text/plain' defined $default_text_plain_charset) { +if (($type =~ m!^text/\w[-\w]*$! || $type =~ m!^\w[-\w]*/\w[-\w]*\+xml$!) defined $default_text_plain_charset) { $type .= ; charset=$default_text_plain_charset; } First, it extends adding ; charset=$default_text_plain_charset to other mimetypes for 'blob_plain' view to all 'text/*' and '*/*+xml' mimetypes (without changing name of variable... though this is more complicated as it is configuration variable and we would want to preserve backward compatibility, but at least a comment would be, I think, needed). Originally it applied only to 'text/plain' files, which can be displayed inline by web browser, and which need charset in 'Content-Type:' HTTP header to be displayed correctly, as they do not include such information inside the file. 'text/html' and 'application/xhtml+xml' can include such information inside them (meta http-equiv for 'text/html' and ?xml ... for 'application/xhtml+xml'). I don't know what browser does when there is conflicting information about charset, i.e. which one wins, but it is certainly something to consider. It might be a good change; I don't know what web browser do when serving 'text/css', 'text/javascript', 'text/xml' to client to view without media type known. BTW I have noticed that we do $prevent_xss dance outside blob_contenttype(), in it's only caller i.e. git_blob_plain()... which for example means that 'text/html' converted to 'text/plain' don't get '; charset=...' added. I guess that it *might* be what prompted this part of change... but if it is so, it needs to be fixed at source, e.g. by moving $prevent_xss to blob_contenttype() subroutine. About implementation: +if (($type =~ m!^text/\w[-\w]*$! || $type =~ m!^\w[-\w]*/\w[-\w]*\+xml$!) defined $default_text_plain_charset) { $type .= ; charset=$default_text_plain_charset; would be better with line split +if (($type =~ m!^text/\w[-\w]*$! || $type =~ m!^\w[-\w]*/\w[-\w]*\+xml$!) +defined $default_text_plain_charset) { $type .= ; charset=$default_text_plain_charset; Also: do we need to be strict with '\w[-\w]*', or would '.*' suffice? @@ -7637,7 +7637,9 @@ sub git_blobdiff { last if $line =~ m!^\+\+\+!; } local $/ = undef; +binmode STDOUT, ':raw'; print $fd; +binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi close $fd; } } @@ -7884,12 +7886,16 @@ sub git_commitdiff { } elsif ($format eq 'plain') { local $/ = undef; +binmode STDOUT, ':raw'; print $fd; +binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi close $fd or print Reading git-diff-tree failed\n; } elsif ($format eq 'patch') { local $/ = undef; +binmode STDOUT, ':raw'; print $fd; +binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi close $fd or print Reading git-format-patch failed\n; } Second it changes 'blobdiff_plain', 'commitdiff_plain' (which I think that should be abandoned in favor of 'patch' view; but that is a separate issue) and 'patch' views so they use binary-safe output. Note that in all cases (I think) we use $cgi-header( -type = 'text/plain', -charset = 'utf-8', ... ); promising web browser that output is as whole in 'utf-8' encoding. It is not explained in the commit message what is the reason for this change. Is it better handing of a situation where files being diff-ed being in other encoding (like for example in commit that changes encoding of files from legacy encoding such like e.g. iso-8859-2 or cp1250 to utf-8)? But what about -charset = 'utf-8' then? About implementation: I think after this change common code crosses threshold for refactoring it into subroutine, for example: sub dump_fh_raw { my $fh = shift; local $/ = undef; binmode STDOUT, ':raw'; print $fh; binmode STDOUT,
Re: [PATCH] t3700 (add): add failing test for add with submodules
W dniu 08.04.2013 23:30, Jeff King pisze: On Mon, Apr 08, 2013 at 03:56:49PM +0530, Ramkumar Ramachandra wrote: git add currently goes past submodule boundaries. Document this bug. It's not just submodules, but we should not recurse into any sub-repository. If I have an unrelated Meta/ repository, I should not be able to git add Meta/foo, whether I have used git submodule or not. This topic came about 2 years ago, and I had a quick-and-dirty patch: http://thread.gmane.org/gmane.comp.version-control.git/170937/focus=171040 I do not recall anything about the patch at this point (i.e., whether it was the right thing), but maybe it is a good starting point for somebody to look into it. Hmmm... I used to do (and still do) such not-recommended thing, i.e. keeping git/gitweb/TODO etc. in git/gitweb/.git repository, while having git/gitweb/gitweb.perl in git/.git repository. So my (admittedly strange) setup will stop working? -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Junio C Hamano wrote: Ramkumar Ramachandra wrote: 2. If we want to make git-submodule a part of git-core (which I think everyone agrees with), we will need to make the information in .gitmodules available more easily to the rest of git-core. Care to define more easily which is another subjective word? The .gitmodules file uses the bog-standard configuration format that can be easily read with the config.c infrastructure. It is a separate matter that git_config() API is cumbersome to use, but improving it would help not just .gitmodules but also the regular non-submodule users of Git. There is a topic in the works to read data in that ^^ format from core Heiko is working on. ^ BTW. this is something that I was missing to implement better submodule support in gitweb (and thus git-instaweb) than just marking it as submodule in 'tree' view. -- Jakub Narębski -- To unsubscribe from this list: send the line 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] t3700 (add): add failing test for add with submodules
Jakub Narębski wrote: Hmmm... I used to do (and still do) such not-recommended thing, i.e. keeping git/gitweb/TODO etc. in git/gitweb/.git repository, while having git/gitweb/gitweb.perl in git/.git repository. Why don't you put the gitweb/TODO in a different branch in the git.git repository? Why do you feel the need to have two different repositories tracking different files in the same path? Just out of curiosity, how does stuff work with your setup? Does the worktree gitweb/ belong to your gitweb.git repository or git.git repository? How do half the git commands work? For example, won't git clean -dfx remove the files tracked by your other repository? Will a conflicting checkout not stomp files tracked by the other repository? How are worktree-rules like .gitignore applied? So my (admittedly strange) setup will stop working? Yes. I would persuade you not to use such a gross setup; this is not what git was intended for 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
GnuPG commit signature verification ignores GPG's status-fd and status-file options
Starting with the newest git version 1.8.2.1, the signature checking code somehow ignores GPG's status-fd and status-file options, which are THE way to machine parse GPG's output (seee [1]) How to reproduce: 1. Put the following line in your ~/.gnupg/gpg.conf file: status-fd 1 2. Produce a singed commit: git commit -aS -m signed test commit 3. Let git check the signature: git log -n 1 --show-signature | cat Output: Commit 104cffded653aasdd gpg: Signature made Tue 09 Apr 2013 01:09:09 PM CEST using RSA key ID 12345678 gpg: Good signature from User Author: user usermail Missing from output is the machine parsable GPG information: [GNUPG:] SIG_ID sorvifhoerui/asidunb 2013-04-09 23947273 [GNUPG:] GOODSIG 4338111324 User usermail [GNUPG:] VALIDSIG ddfsjidjfv 2013-04-09 aoidfjidh0 0 4 0 1 2 00 oshidvoo44d [GNUPG:] TRUST_ULTIMATE Note: The git-log format specifiers %GG, %G?, %GK, ... do not provide enough information, as they, for example, do not provide the information, that the signature is valid, if the key is untrusted (which will simply leed to a bad signature). [1] http://www.gnupg.org/documentation/manuals/gnupg-devel/Unattended-Usage.html#Unattended-Usage -- To unsubscribe from this list: send the line 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-http-backend: anonymous read, authenticated write
On 09.04.2013, Magnus Therning wrote: I've been trying to set up git-http-backend+lighttpd. I've managed to set up anonymous read-only access, and I then successfully configured authentication for both read and write. Then I get stuck. The man-page for git-http-backend says that the following snippet can be used for Apache 2.x: LocationMatch ^/git/.*/git-receive-pack$ AuthType Basic AuthName Git Access Require group committers ... /LocationMatch However, when I put in this match on location in my lighty config and try to push I'm not asked for a password, instead I'm greeted with % git push error: The requested URL returned error: 403 Forbidden while accessing http://magnus@tracsrv.local/git/foo.git/info/refs?service=git-receive-pack AFAICS this means the man-page is wrong, and that I instead ought to match on the service=git-receive-pack part. Is that a correct conclusion? Yes, it is. I have tried to do the same anonymous read and authenticated write in smart HTTP access in Apache. There are some proposals[1], all I think which use mod_rewrite (as LocationMatch doesn't take query string into account, unfortunately), but I haven't been able to make it work. The problem is that both POST *and GET* (to get refs) must be authethicated. Nb. I thought that it was corrected... which git version do you use? [1]: http://paperlined.org/apps/git/SmartHTTP_Ubuntu.html In the end I have worked around this by allowing all registered users to read with require valid-user (which in my situation might be even more correct solution; the case being repositories for Computer Science class lab work), and restricting write via pre-receive hook which checks REMOTE_USER. -- Jakub Narębski -- To unsubscribe from this list: send the line 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] t/README: --immediate skips cleanup commands for failed tests
On Sun, Apr 07, 2013 at 03:32:00PM -0700, Jonathan Nieder wrote: I'm not sure if it's better to use test_when_finished with rm or just rm -rf tmp at the end of the test in case someone wants to look at the output. test_when_finished is better here, since it means later tests can run and provide useful information about how bad a regression is. Cleanup commands requested using test_when_finished are not run when a test being run with --immediate fails, so you can still inspect output after a failed test. Hello Jonathan, Thanks for the explanation. I couldn't find this documented in t/README, the following patch adds it. -- 8 -- Subject: [PATCH] t/README: --immediate skips cleanup commands for failed tests --- t/README | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/README b/t/README index 9b41fe7..e5e7d37 100644 --- a/t/README +++ b/t/README @@ -86,7 +86,8 @@ appropriately before running make. --immediate:: This causes the test to immediately exit upon the first - failed test. + failed test. Cleanup commands requested with + test_when_finished are not executed if the test failed. --long-tests:: This causes additional long-running tests to be run (where -- 1.8.2.481.g0d034d4 -- 8 -- Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- To unsubscribe from this list: send the line 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] t3700 (add): add failing test for add with submodules
On 09.04.2013, Ramkumar Ramachandra wrote: Jakub Narębski wrote: Hmmm... I used to do (and still do) such not-recommended thing, i.e. keeping git/gitweb/TODO etc. in git/gitweb/.git repository, while having git/gitweb/gitweb.perl in git/.git repository. Why don't you put the gitweb/TODO in a different branch in the git.git repository? Why do you feel the need to have two different repositories tracking different files in the same path? It is not only gitweb/TODO. If it was only that file, I could have used 'gitweb/todo' branch for it, or something. Though I would be missing having it beside gitweb.perl, and having easy access to it during work on gitweb.perl I want to have various files that I use when working on gitweb.perl (but should not and would not be in gitweb subsystem in git.git repository) to be under version control, and be side-by-side near gitweb.perl. These are: * gitweb/TODO - TODO file for gitweb (personal, not often updated), and similar gitweb/gitwebs-whats-cooking.txt Might be put into 'gitweb/todo/TODO' file and 'gitweb/todo/.git' private repository (and perhaps 'gitweb/todo' branch of my clone of git.git repository). * various *_test.perl files, where I test features to be possibly put into gitweb, like e.g. chop_str_test.perl or test_find_forks.perl (or similar benchmark_find_forks.perl) * private personal configuration files for testing its output, like gitweb/gitweb_config.perl and gitweb/magic.txt !!! Those are very much required to reside beside gitweb/gitweb.perl because of default GITWEB_CONFIG value. With those I can simply run current gitweb/gitweb.perl (sic!) from its directory while I am working on it. Just out of curiosity, how does stuff work with your setup? Does the worktree gitweb/ belong to your gitweb.git repository or git.git repository? The 'git/gitweb/' worktree belong to both repositories (it is 'gitweb/' in git.git clone i.e. git/.git, and it is top dir of git/gitweb/.git repository). How do half the git commands work? For example, won't git clean -dfx remove the files tracked by your other repository? They work, somewhat and with some care. I don't use git clean for example. Will a conflicting checkout not stomp files tracked by the other repository? How are worktree-rules like .gitignore applied? 'git/gitweb/.gitignore' belong to 'git/gitweb/.git' repository and is used to ignore 'git/.git' files (with the intent of marking them untracked *precious*). I could have used 'info/exclude' here because this repository is for the time being private. As gitweb subsystem in git.git is quite stable, untracked but existing 'gitweb/.gitignore' doesn't usually matter, as all files in 'gitweb/' are already tracked. Besides I can always use git add -f for adding to git.git if necessary (e.g. when splitting gitweb.js etc.). So my (admittedly strange) setup will stop working? Yes. I would persuade you not to use such a gross setup; this is not what git was intended for at all. Why not? -- Jakub Narębski -- To unsubscribe from this list: send the line 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/6] send-email: make annotate configurable
W dniu 07.04.2013 19:46, Felipe Contreras pisze: @@ -212,7 +212,8 @@ my %config_bool_settings = ( signedoffbycc = [\$signed_off_by_cc, undef], signedoffcc = [\$signed_off_by_cc, undef], # Deprecated validate = [\$validate, 1], -multiedit = [\$multiedit, undef] +multiedit = [\$multiedit, undef], +annotate = [\$annotate, undef] ); Why not leave hanging , to make it easier on future changes, i.e.: -multiedit = [\$multiedit, undef] +multiedit = [\$multiedit, undef], +annotate = [\$annotate, undef], -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Very useful script to SVG graph the git commits from a file orientated view
Hi, On Tue, Apr 9, 2013 at 10:55 AM, Jeremy Rosen jeremy.ro...@openwide.fr wrote: looking a little bit more into this, I was very suprised there seems to be little/no tools in the git ecosystem that studies the dependencies between commits based on the file they modified and/or the conflict they would cause. Is there any pre-existing tool to do that ? It can be done with git-log --name-only(the graph_git.pl is just a graphing layer above that command) but i'm suprised that I couldn't find anything else And that was at the file level, is there any tool to help find what commits can be reordered without causing conflicts ? I am not sure if there is an easy way to extract potential conflict information from git... It looks like this tool will do Proactive Merge Conflicts Detection: http://commitq.com/ But it's true that it would be nice if there was something in git itself. Thanks, Christian. -- To unsubscribe from this list: send the line 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: Teaching rerere about existing merges
Jeremy Rosen jeremy.ro...@openwide.fr writes: is there a way to teach rerere about existing merge commits, or do I have to re-solve all the existing merge manually once ? See src/contrib/rerere-train.sh and Junio's blog. http://git-blame.blogspot.com/2012/04/rebuilding-git-integration-environment.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: Teaching rerere about existing merges
thanks a lot, that solves my problem. I'm a bit suprised that it's not part of the git-rerere command but that's good enough for me... Cordialement Jérémy Rosen fight key loggers : write some perl using vim - Mail original - Jeremy Rosen jeremy.ro...@openwide.fr writes: is there a way to teach rerere about existing merge commits, or do I have to re-solve all the existing merge manually once ? See src/contrib/rerere-train.sh and Junio's blog. http://git-blame.blogspot.com/2012/04/rebuilding-git-integration-environment.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: GnuPG commit signature verification ignores GPG's status-fd and status-file options
Not Sure kue...@googlemail.com writes: Starting with the newest git version 1.8.2.1, the signature checking [...] Missing from output is the machine parsable GPG information: [GNUPG:] SIG_ID sorvifhoerui/asidunb 2013-04-09 23947273 [GNUPG:] GOODSIG 4338111324 User usermail [GNUPG:] VALIDSIG ddfsjidjfv 2013-04-09 aoidfjidh0 0 4 0 1 2 00 oshidvoo44d [GNUPG:] TRUST_ULTIMATE I suspect that's because since b60b756 (gpg-interface: check good signature in a reliable way, 2013-02-14), Git parses the above output. Also, in e290c4b (pretty printing: extend %G? to include 'N' and 'U', 2013-03-31) which is currently in master, this was fixed: Note: The git-log format specifiers %GG, %G?, %GK, ... do not provide enough information Is anything else missing? -- 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 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages
W dniu 08.04.2013 22:10, Jürgen Kreileder pisze: Fix broken blob action parameters on blobdiff and commitdiff pages by explicitly passing variables instead of relying on global ones. Do I understand it correctly that those variables (e.g. $hash variable in git_patchset_body in second chunk below, after this change passed as parameter to format_diff_cc_simplified()) can be filled in then, or at least adjusted correctly? (The broken parameters on blob links lead to blob pages which show the blob but with a hash instead of a commit message and have broken blob_plain (404 - Cannot find file) and tree links (404 - Reading tree failed)) Signed-off-by: Jürgen Kreileder j...@blackdown.de I wonder how we missed this. Does this happen always, or in some specific conditions? Anyway, this change is a good change. Internal subroutines should not use global variables. I hope that in the future we would get rid of most global variables... Acked-by: Jakub Narebski jna...@gmail.com [not tested] # create note for patch simplified by combined diff sub format_diff_cc_simplified { - my ($diffinfo, @parents) = @_; + my ($diffinfo, $hash, @parents) = @_; my $result = ''; $result .= div class=\diff header\ . [...] @@ -5404,7 +5405,7 @@ sub git_patchset_body { # generate anchor for patch links in difftree / whatchanged part print div class=\patch\ id=\patch. ($patch_idx+1) .\\n . - format_diff_cc_simplified($diffinfo, @hash_parents) . + format_diff_cc_simplified($diffinfo, $hash, @hash_parents) . /div\n; # class=patch $patch_number++; -- Jakub Narębski -- To unsubscribe from this list: send the line 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: Teaching rerere about existing merges
Jeremy Rosen jeremy.ro...@openwide.fr writes: is there a way to teach rerere about existing merge commits, or do I have to re-solve all the existing merge manually once ? There is a tool that does the re-solve manually for you. $ git ls-files | grep rerere-train contrib/rerere-train.sh It uses your working tree to do its work, so you should first commit or stash whatever you are in the middle of doing before using it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t3700 (add): add failing test for add with submodules
On Tue, Apr 09, 2013 at 02:49:24PM +0530, Ramkumar Ramachandra wrote: On the wording issue, a submodule is a submodule whether in-index or otherwise. I would write two different tests: one for in-worktree submodule and another for in-index submodule, and name them appropriately. Does that make sense? Yeah, that makes perfect sense to me. -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 2/2] clone: Allow repo using gitfile as a reference
At 17:24 -0700 08 Apr 2013, Jonathan Nieder jrnie...@gmail.com wrote: +test_expect_success 'clone using repo with gitfile as a reference' ' + git clone --separate-git-dir=L A M + git clone --reference=M A N What should happen if I pass --reference=M/.git? That isn't supported and I wouldn't expect it to work. The --reference option is documented as taking the location of a repository as the argument and I wouldn't consider a .git file to be a repository. I also can't think of a reason that it would be very useful since it should be simple to just refer to the directory containing the .git file. But if others disagree, I could be convinced to add support for that. I also wouldn't consider it breakage if that use would start working, so I don't see a point in adding a test to check that that usage fails. + echo $base_dir/L/objects expected The usual style in tests is to include no space after redirection operators. Fixed for the next version, pending further comments. -- To unsubscribe from this list: send the line 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] clone: Allow repo using gitfile as a reference
Aaron Schrab aa...@schrab.com writes: At 17:24 -0700 08 Apr 2013, Jonathan Nieder jrnie...@gmail.com wrote: +test_expect_success 'clone using repo with gitfile as a reference' ' + git clone --separate-git-dir=L A M + git clone --reference=M A N What should happen if I pass --reference=M/.git? That isn't supported and I wouldn't expect it to work. The --reference option is documented as taking the location of a repository as the argument and I wouldn't consider a .git file to be a repository. I also can't think of a reason that it would be very useful since it should be simple to just refer to the directory containing the .git file. But if others disagree, I could be convinced to add support for that. If M/.git weren't a gitfile that points elsewhere, that request ought to work, no? A gitfile is the moral equilvalent of a symbolic link, meant to help people on platforms and filesystems that lack symbolic links, so in that sense, not supporting the case goes against the whole reason why we have added support for gitfile in the first place, I think. -- To unsubscribe from this list: send the line 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] clone: Allow repo using gitfile as a reference
At 09:47 -0700 09 Apr 2013, Junio C Hamano ju...@pobox.com wrote: Aaron Schrab aa...@schrab.com writes: But if others disagree, I could be convinced to add support for that. If M/.git weren't a gitfile that points elsewhere, that request ought to work, no? A gitfile is the moral equilvalent of a symbolic link, meant to help people on platforms and filesystems that lack symbolic links, so in that sense, not supporting the case goes against the whole reason why we have added support for gitfile in the first place, I think. OK, I'm convinced. I'll modify it to support that as well. -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
On Tue, Apr 09, 2013 at 02:51:37PM +0530, Ramkumar Ramachandra wrote: Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a git repository in the worktree still adds fine, and this is a bug. The logic for denying it is very similar to denying adding paths beyond symbolic links: die_if_path_beyond_symlink(). Follow its example and write a die_if_path_beyond_gitrepo() to fix this bug. Thanks for working on this. I think the direction is a good one. It does disallow Jakub's crazy shared-directory setup. I am not too sad to see that disallowed, at least by default, because there are so many ways to screw yourself while using it if you are not careful (I tried something similar once, and gave up because I kept running into problematic cases). I am not opposed to having an escape hatch to operate in that mode, but it should be triggered explicitly so it doesn't catch users unaware. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/add.c | 5 +++-- cache.h| 2 ++ pathspec.c | 12 pathspec.h | 1 + symlinks.c | 43 +-- t/t3700-add.sh | 2 +- 6 files changed, 56 insertions(+), 9 deletions(-) I am not super-familiar with this part of the code, but having worked on it once two years ago for the same problem, your solution looks like the right thing. @@ -142,8 +143,22 @@ static int lstat_cache_matchlen(struct cache_def *cache, if (errno == ENOENT) *ret_flags |= FL_NOENT; } else if (S_ISDIR(st.st_mode)) { - last_slash_dir = last_slash; - continue; + /* Check to see if the directory contains a +git repository */ + struct stat st; + struct strbuf dotgitentry = STRBUF_INIT; + strbuf_addf(dotgitentry, %s/.git, cache-path); Can we use mkpath here to avoid an allocation? Or even better, cache-path is PATH_MAX+1 bytes, and we munge it earlier in the function. Can we just check the length and stick /.git on the end? + if (lstat(dotgitentry.buf, st) 0) { + if (errno == ENOENT) { + strbuf_release(dotgitentry); + last_slash_dir = last_slash; + continue; + } + *ret_flags = FL_LSTATERR; + } + else + *ret_flags = FL_GITREPO; + strbuf_release(dotgitentry); In my original patch long ago, Junio asked if we should be checking is_git_directory() when we find a .git entry, to make sure it is not a false positive. I don't have a strong opinion either way, but if we do that, we would possibly want to update treat_path to do the same thing for consistency. -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: commit-message attack for extracting sensitive data from rewritten Git history
On Tue, Apr 09, 2013 at 08:03:24AM +0200, Johannes Sixt wrote: Am 4/8/2013 23:54, schrieb Jeff King: Yeah, it would make sense for filter-branch to have a --map-commit-ids option or similar that does the update. At first I thought it might take two passes, but I don't think it is necessary, as long as we traverse the commits topologically (i.e., you cannot have mentioned X in a commit that is an ancestor of X, so you do not have to worry about mapping it until after it has been processed). Topological traversal is not sufficient. Consider this history: o--A--o-- / / --o--B--o If A mentions B (think of cherry-pick -x), then you must ensure that the branch containing B was traversed first. Yeah, you're right. Multiple passes are necessary to get it completely right. And because each pass may change more commit id's, you have to recurse to pick up those changes, and keep going until you have a pass with no changes. But I haven't thought that hard about it. There might be a clever optimization where you can prune out parts of the history (e.g., if you know that all changes to consider are in descendants of a commit, you do not have to care about rewriting the commit or its ancestors). -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
gitweb commitdiff page - binary files with ampersands in filename?
Change a binary file whose filename contains an ampersand, then view the commitdiff page in gitweb. Git outputs a message like Binary files a/bw.dll and b/bw.dll differ Gitweb format_diff_from_to_header() doesn't notice anything in that output which needs escaping, and writes it directly to the XHTML 1.0 Strict output. Then gitweb's output is invalid XML, meaning that browsers such as Firefox will refuse to display the page. (tested with v 1.7.9.5, but I can't see anything in latest https://github.com/git/git/blob/master/gitweb/gitweb.perl#CL2158 which is looking for text like Binary files ... differ) -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Ramkumar Ramachandra artag...@gmail.com writes: Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a git repository in the worktree still adds fine, and this is a bug. The logic for denying it is very similar to denying adding paths beyond symbolic links: die_if_path_beyond_symlink(). Follow its example and write a die_if_path_beyond_gitrepo() to fix this bug. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- @@ -166,6 +166,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix) const char **p; for (p = pathspec; *p; p++) { die_if_path_beyond_symlink(*p, prefix); + die_if_path_beyond_gitrepo(*p, prefix); } } diff --git a/cache.h b/cache.h index e1e8ce8..987d7f3 100644 --- a/cache.h +++ b/cache.h @@ -962,6 +962,8 @@ struct cache_def { extern int has_symlink_leading_path(const char *name, int len); +extern int has_gitrepo_leading_path(const char *name, int len); I looked at the output from grep has_symlink_leading_path and also for die_if_path_beyond; all of these places are checking I have this multi-level path; I want to know if the path does not (should not) be part of the current project, I think. Certainly the one in the update-index is about the same operation as git add you are patching. Isn't it a better approach to _rename_ the existing function not to single out symlink-ness of the path first ? A symlink in the middle of such a multi-level path that leads to a place outside the project is _not_ the only way to step out of our project boundary. A directory in the middle of a multi-level path that is the top-level of the working tree of a foreign project is another way to step out of our project boundary. Perhaps die_if_path_outside_our_project() path_outside_our_project() And then update the implementation of path_outside_our_project(), which only took symlink in the middle into account so far, and teach it that such a top-level of the working tree of a foreign project is also stepping out of our project? That way, you do not have to settle on fixing the bug only in git add and keep the bug in git update-index, I think. I think the hit in builtin/apply.c deals with the same beyond symlink is outside our project check and can be updated like so. I didn't look at the ones in diff-lib.c and dir.c so you may want to double check on what they use it for. -- To unsubscribe from this list: send the line 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-http-backend: anonymous read, authenticated write
On Tue, Apr 09, 2013 at 07:45:53AM +0200, Magnus Therning wrote: I've been trying to set up git-http-backend+lighttpd. I've managed to set up anonymous read-only access, and I then successfully configured authentication for both read and write. Then I get stuck. The man-page for git-http-backend says that the following snippet can be used for Apache 2.x: LocationMatch ^/git/.*/git-receive-pack$ AuthType Basic AuthName Git Access Require group committers ... /LocationMatch However, when I put in this match on location in my lighty config and try to push I'm not asked for a password, instead I'm greeted with % git push error: The requested URL returned error: 403 Forbidden while accessing http://magnus@tracsrv.local/git/foo.git/info/refs?service=git-receive-pack Something in your config is blocking access to info/refs there. It should not be the block shown above, which handles only the actual POST of the data. The sequence of http requests made is: 1. GET $repo/info/refs?service=git-receive-pack This makes initial contact and gets the ref information which push uses to decide what it is going to push. So it is read-only, and in an anonymous-read setup, does not need to be protected. 2. POST $repo/git-receive-pack This actually pushes up the objects and updates the refs, and must be protected. The setup listed above does work with apache; it is tested as part of our test suite (you can see the actual config in t/lib-httpd/apache.conf). So what in lighttpd is giving us the 403? Can you share your whole config? AFAICS this means the man-page is wrong, and that I instead ought to match on the service=git-receive-pack part. Is that a correct conclusion? No. It is not a bad idea to _also_ match on info/refs, but I think it's a little trickier (you need to reliably match the query string to differentiate it from a fetch, which IIRC is a little hard in apache, at least). But if you drop the protections on /git-receive-pack$, then an attacker can just POST whatever they want into your repository. -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: segfault in git-remote-http
On Tue, Apr 09, 2013 at 08:47:18AM -0700, rh wrote: The symptoms that this patch addresses look similar: http://article.gmane.org/gmane.mail.postfix.user/217790 Quote from that thread: This behavior is actually documented (SSL_set_fd() destroys a BIO already on the SSL handle, and creates a new BIO). Maybe someone used to looking at git-remote-http code can say anything about this. git-remote-http does not touch the openssl code itself at all. We only talk to curl, which handles all of the ssl (and may even be built on gnutls). So if that is the problem, then I think it may be a libcurl bug, not a git one. -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: segfault in git-remote-http
On Tue, 9 Apr 2013, Jeff King wrote: git-remote-http does not touch the openssl code itself at all. We only talk to curl, which handles all of the ssl (and may even be built on gnutls). So if that is the problem, then I think it may be a libcurl bug, not a git one. ... and if/when you do make it a libcurl bug, please include more details that includes how to repeat the problem and what versions of libcurl and OpenSSL you're using. -- / daniel.haxx.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: [ITCH] Specify refspec without remote
Ramkumar Ramachandra artag...@gmail.com writes: Jeff King wrote: So you would need some heuristics based on whether something was a valid refspec, or could be a valid remote name or URL. All refspecs conform to a very simple format: quux +quux quux:baz +quux:baz All of them fail at git_connect(). The third and fourth are equivalent and take a little longer to fail than the first two, because ssh tries to resolve the hostname quux or +quux. host:foo/bar (take my host branch, push it to their foo/bar branch) could be tricky, no? It could be trying to go over the ssh to host and access repository at $HOME/foo/bar. The git_connect() call may even succeed and you cannot use the failure as a hint to disambiguate. Also the request may genuinely be to access foo/bar repository at the host, but the network layer had trouble connecting temporarily to the host. After disambiguating incorrectly to push to the origin, mapping our host branch to their foo/bar branch, that push might even succeed. -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Junio C Hamano gits...@pobox.com writes: Ramkumar Ramachandra artag...@gmail.com writes: Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a git repository in the worktree still adds fine, and this is a bug. The logic for denying it is very similar to denying adding paths beyond symbolic links: die_if_path_beyond_symlink(). Follow its example and write a die_if_path_beyond_gitrepo() to fix this bug. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- @@ -166,6 +166,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix) const char **p; for (p = pathspec; *p; p++) { die_if_path_beyond_symlink(*p, prefix); +die_if_path_beyond_gitrepo(*p, prefix); } } diff --git a/cache.h b/cache.h index e1e8ce8..987d7f3 100644 --- a/cache.h +++ b/cache.h @@ -962,6 +962,8 @@ struct cache_def { extern int has_symlink_leading_path(const char *name, int len); +extern int has_gitrepo_leading_path(const char *name, int len); I looked at the output from grep has_symlink_leading_path and also for die_if_path_beyond; all of these places are checking I have this multi-level path; I want to know if the path does not (should not) be part of the current project, I think. Certainly the one in the update-index is about the same operation as git add you are patching. Isn't it a better approach to _rename_ the existing function not to single out symlink-ness of the path first ? A symlink in the middle of such a multi-level path that leads to a place outside the project is _not_ the only way to step out of our project boundary. A directory in the middle of a multi-level path that is the top-level of the working tree of a foreign project is another way to step out of our project boundary. Perhaps die_if_path_outside_our_project() path_outside_our_project() And then update the implementation of path_outside_our_project(), which only took symlink in the middle into account so far, and teach it that such a top-level of the working tree of a foreign project is also stepping out of our project? That way, you do not have to settle on fixing the bug only in git add and keep the bug in git update-index, I think. I think the hit in builtin/apply.c deals with the same beyond symlink is outside our project check and can be updated like so. I didn't look at the ones in diff-lib.c and dir.c so you may want to double check on what they use it for. The first step (renaming and adjusting comments) would look like this. builtin/add.c | 6 +++--- builtin/apply.c| 8 ++-- builtin/check-ignore.c | 2 +- builtin/update-index.c | 4 ++-- cache.h| 4 ++-- diff-lib.c | 2 +- dir.c | 2 +- pathspec.c | 6 +++--- pathspec.h | 2 +- preload-index.c| 2 +- symlinks.c | 10 +- t/t0008-ignores.sh | 2 +- 12 files changed, 27 insertions(+), 23 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index ab1c9e8..7cb80ef 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -155,8 +155,8 @@ static void refresh(int verbose, const char **pathspec) /* * Normalizes argv relative to prefix, via get_pathspec(), and then - * runs die_if_path_beyond_symlink() on each path in the normalized - * list. + * runs die_if_path_outside_our_project() on each path in the + * normalized list. */ static const char **validate_pathspec(const char **argv, const char *prefix) { @@ -165,7 +165,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix) if (pathspec) { const char **p; for (p = pathspec; *p; p++) { - die_if_path_beyond_symlink(*p, prefix); + die_if_path_outside_our_project(*p, prefix); } } diff --git a/builtin/apply.c b/builtin/apply.c index 5b882d0..d0b408e 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -3469,10 +3469,14 @@ static int check_to_create(const char *new_name, int ok_if_exists) * A leading component of new_name might be a symlink * that is going to be removed with this patch, but * still pointing at somewhere that has the path. -* In such a case, path new_name does not exist as +* Or it could be the top-level of a working tree of +* a different project that is embedded in our working +* tree. +* +* In such cases, path new_name does not exist as * far as git is concerned. */ - if (has_symlink_leading_path(new_name, strlen(new_name)))
Re: [ITCH] Specify refspec without remote
Junio C Hamano wrote: host:foo/bar (take my host branch, push it to their foo/bar branch) could be tricky, no? It could be trying to go over the ssh to host and access repository at $HOME/foo/bar. The git_connect() call may even succeed and you cannot use the failure as a hint to disambiguate. Also the request may genuinely be to access foo/bar repository at the host, but the network layer had trouble connecting temporarily to the host. After disambiguating incorrectly to push to the origin, mapping our host branch to their foo/bar branch, that push might even succeed. Oh, ouch. I didn't think of that. What do you suggest we do? Go with Duy's simple '-' solution, or try some heuristics that may lead to confusing behavior in edge cases? -- To unsubscribe from this list: send the line 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] gitweb: Make feed title valid utf8
Jakub Narębski jna...@gmail.com writes: Jürgen Kreileder wrote: Properly encode site and project names for RSS and Atom feeds. Signed-off-by: Jürgen Kreileder j...@blackdown.de --- gitweb/gitweb.perl |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 9cfe5b5..09294eb 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -8056,7 +8056,7 @@ sub git_feed { return if ($cgi-request_method() eq 'HEAD'); # header variables -my $title = $site_name - $project/$action; +my $title = to_utf8($site_name) . - . to_utf8($project) . /$action; my $feed_type = 'log'; if (defined $hash) { $title .= - '$hash'; Was this patch triggered by some bug? Yes, I actually see broken encoding with the old code, e.g on https://git.blackdown.de/old.cgi?p=contactalbum.git;a=rss my first name is messed up in the title tag. New version: https://git.blackdown.de/?p=contactalbum.git;a=rss Because the above is not necessary, as git_feed() has $title = esc_html($title); a bit later, which does to_utf8() internally. Good point. But it doesn't fix the string in question: It looks like to_utf8($a $b) != (to_utf8($a) . . to_utf($b)). Juergen -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Junio C Hamano gits...@pobox.com writes: I looked at the output from grep has_symlink_leading_path and also for die_if_path_beyond; all of these places are checking I have this multi-level path; I want to know if the path does not (should not) be part of the current project, I think. Certainly the one in the update-index is about the same operation as git add you are patching. Isn't it a better approach to _rename_ the existing function not to single out symlink-ness of the path first ? A symlink in the middle of such a multi-level path that leads to a place outside the project is _not_ the only way to step out of our project boundary. A directory in the middle of a multi-level path that is the top-level of the working tree of a foreign project is another way to step out of our project boundary. Perhaps die_if_path_outside_our_project() path_outside_our_project() And then update the implementation of path_outside_our_project(), which only took symlink in the middle into account so far, and teach it that such a top-level of the working tree of a foreign project is also stepping out of our project? That way, you do not have to settle on fixing the bug only in git add and keep the bug in git update-index, I think. I think the hit in builtin/apply.c deals with the same beyond symlink is outside our project check and can be updated like so. I didn't look at the ones in diff-lib.c and dir.c so you may want to double check on what they use it for. The first step (renaming and adjusting comments) would look like this. Actually, there is another function check_leading_path() you may want also adjust. /* * Return zero if path 'name' has a leading symlink component or * if some leading path component does not exists. * * Return -1 if leading path exists and is a directory. * * Return path length if leading path exists and is neither a * directory nor a symlink. */ int check_leading_path(const char *name, int len) { return threaded_check_leading_path(default_cache, name, len); } I think what the callers of this function care about is if the name is a path that should not be added to our index (i.e. points outside the repository). If you had a symlink d that points at e when our project does have a subdirectory e with file f, check_leading_path(d/f) wants to say bad, even though the real file pointed at, i.e. e/f is inside our working tree, so outside our working tree is not quite correct in the strict sense (this applies equally to has_symlink_leading_path), but I think we should treat the case where d (and d/f) belongs to the working tree of a repository for a separate project, that is embedded in our working tree the same 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: [PATCH 2/2] add: refuse to add paths beyond repository boundaries
Junio C Hamano wrote: The first step (renaming and adjusting comments) would look like this. Thanks for this! I like the name die_if_path_outside_our_project(). I'll take care of the rest.` -- To unsubscribe from this list: send the line 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: segfault in git-remote-http
On Tue, Apr 09, 2013 at 10:41:34AM -0700, rh wrote: git-remote-http does not touch the openssl code itself at all. We only talk to curl, which handles all of the ssl (and may even be built on gnutls). So if that is the problem, then I think it may be a libcurl bug, not a git one. Thanks, I see git-remote-{http,https,ftp,ftps} are the same size. Minor nitpick but shouldn't the error thrown say git-remote-https? They are all hardlinks to the same program (or copies if your platform does not support hardlinks or symlinks). But I'm not sure which error you are talking about. We can figure out inside the program which program was invoked by checking argv, but I do not see us printing remote-http anywhere. -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 2/2] add: refuse to add paths beyond repository boundaries
Sorry for repeated rerolls. I had missed another instance in t0008 and also the explanation was lacking. -- 8 -- Subject: [PATCH] symlinks: rename has_symlink_leading_path() to path_outside_our_project() The purpose of the function is to prevent a path from getting added to our project when its path component steps outside our working tree, like this: ln -s /etc myetc git add myetc/passwd We do not want to end up with myetc/passwd in our index. To make sure an attempt to add such a path is caught, the implementation checks if there is any leading symbolic link in the given path (adding myetc itself as a symbolic link to our project is accepted). But there are other cases to attempt to add a path that do not belong to our project, which do not have to involve a symbolic link in the leading path. Rename the function, and die_if_path_beyond_symlink() function, to clarify what they are really checking, not how they are checking. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/add.c | 6 +++--- builtin/apply.c| 8 ++-- builtin/check-ignore.c | 2 +- builtin/update-index.c | 4 ++-- cache.h| 4 ++-- diff-lib.c | 2 +- dir.c | 2 +- pathspec.c | 6 +++--- pathspec.h | 2 +- preload-index.c| 2 +- symlinks.c | 10 +- t/t0008-ignores.sh | 4 ++-- 12 files changed, 28 insertions(+), 24 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index ab1c9e8..7cb80ef 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -155,8 +155,8 @@ static void refresh(int verbose, const char **pathspec) /* * Normalizes argv relative to prefix, via get_pathspec(), and then - * runs die_if_path_beyond_symlink() on each path in the normalized - * list. + * runs die_if_path_outside_our_project() on each path in the + * normalized list. */ static const char **validate_pathspec(const char **argv, const char *prefix) { @@ -165,7 +165,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix) if (pathspec) { const char **p; for (p = pathspec; *p; p++) { - die_if_path_beyond_symlink(*p, prefix); + die_if_path_outside_our_project(*p, prefix); } } diff --git a/builtin/apply.c b/builtin/apply.c index 5b882d0..d0b408e 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -3469,10 +3469,14 @@ static int check_to_create(const char *new_name, int ok_if_exists) * A leading component of new_name might be a symlink * that is going to be removed with this patch, but * still pointing at somewhere that has the path. -* In such a case, path new_name does not exist as +* Or it could be the top-level of a working tree of +* a different project that is embedded in our working +* tree. +* +* In such cases, path new_name does not exist as * far as git is concerned. */ - if (has_symlink_leading_path(new_name, strlen(new_name))) + if (path_outside_our_project(new_name, strlen(new_name))) return 0; return EXISTS_IN_WORKTREE; diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c index 0240f99..bce378d 100644 --- a/builtin/check-ignore.c +++ b/builtin/check-ignore.c @@ -88,7 +88,7 @@ static int check_ignore(const char *prefix, const char **pathspec) full_path = prefix_path(prefix, prefix ? strlen(prefix) : 0, path); full_path = check_path_for_gitlink(full_path); - die_if_path_beyond_symlink(full_path, prefix); + die_if_path_outside_our_project(full_path, prefix); if (!seen[i]) { exclude = last_exclude_matching_path(check, full_path, -1, dtype); diff --git a/builtin/update-index.c b/builtin/update-index.c index 5c7762e..7c47fa2 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -186,8 +186,8 @@ static int process_path(const char *path) struct cache_entry *ce; len = strlen(path); - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if (path_outside_our_project(path, len)) + return error('%s' is outside our working tree, path); pos = cache_name_pos(path, len); ce = pos 0 ? NULL : active_cache[pos]; diff --git a/cache.h b/cache.h index e1e8ce8..f6359b5 100644 --- a/cache.h +++ b/cache.h @@ -960,8 +960,8 @@ struct cache_def { int prefix_len_stat_func; }; -extern int has_symlink_leading_path(const char *name, int len); -extern int
Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries
Junio C Hamano wrote: I think what the callers of this function care about is if the name is a path that should not be added to our index (i.e. points outside the repository). If you had a symlink d that points at e when our project does have a subdirectory e with file f, check_leading_path(d/f) wants to say bad, even though the real file pointed at, i.e. e/f is inside our working tree, so outside our working tree is not quite correct in the strict sense (this applies equally to has_symlink_leading_path), but Actually, you introduced one naming regression: has_symlink_leading_path() is a good name for what the function does, as opposed to die_if_path_outside_our_tree(), which is misleading. What about die_if_path_contains_links() to encapsulate gitlinks and symlinks? I think we should treat the case where d (and d/f) belongs to the working tree of a repository for a separate project, that is embedded in our working tree the same way. I'm not too sure about this. It means that I can have symlinks to files in various parts of my worktree, but not to directories. Isn't this an absurd limitation to impose? I'm not saying that it's particularly useful to have a symlink at / pointing to a directory deeply nested in your repository, but that limitations must have some concrete rationale. Anyway, since we're not introducing any regressions (as has_symlink_leading_path imposes the same absurd limitation), we don't have to fix this now. But it's certainly something worth fixing in the future, I think. -- To unsubscribe from this list: send the line 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: [ITCH] Specify refspec without remote
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: host:foo/bar (take my host branch, push it to their foo/bar branch) could be tricky, no? It could be trying to go over the ssh to host and access repository at $HOME/foo/bar. The git_connect() call may even succeed and you cannot use the failure as a hint to disambiguate. Also the request may genuinely be to access foo/bar repository at the host, but the network layer had trouble connecting temporarily to the host. After disambiguating incorrectly to push to the origin, mapping our host branch to their foo/bar branch, that push might even succeed. Oh, ouch. I didn't think of that. What do you suggest we do? Go with Duy's simple '-' solution, or try some heuristics that may lead to confusing behavior in edge cases? What is bad about saying push origin ...the rest...? It is beyond me why people would want to invent unintuitive line noise like '-' that others need to read the manual from cover to cover to find and memorize for something small like this. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] gitweb: Don't append ';js=(0|1)' to external links
W dniu 09.04.2013 19:54, Jürgen Kreileder napisał: Jakub Narębski jna...@gmail.com writes: Jürgen Kreileder wrote: Don't add js parameters to links outside of gitweb itself. Hmmm... this limits adding ';js=(0|1)' to only links which begin with $my_url, i.e. absolute links beginning with gitweb's base URL. Wouldn't that mean that most internal gitweb-generated links wouldn't get ';js=(0|1)'? href(..., -full = 1) is not the default... No, link.href is always absolute in JavaScript - even if the emitted URL was relative. Thanks, I didn't know that. So, with that explanation: Acked-by: Jakub Narebski jna...@gmail.com Old: https://git.blackdown.de/old.cgi?p=contactalbum.git;a=summary;js=1 New: https://git.blackdown.de/?p=contactalbum.git;a=summary;js=1 With the old version the external links in the description got ';js=1' appended. With the new version, ';js=1' isn't on those links. Other links are the same in both versions. Thanks. This is a nice solution for a problem which I didn't know how to solve (I was thinking about using a class=internal ..., but your solution is better). -- Jakub Narębski -- To unsubscribe from this list: send the line 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: [ITCH] Specify refspec without remote
Junio C Hamano wrote: What is bad about saying push origin ...the rest...? I don't know which remote to push to: all I know is that the remote to push to is configured somewhere in the web of branch.remote, remote.pushdefault, and branch.name.pushremote, and I don't want to have to figure that out by hand. Ever since we got triangular workflows, I've been missing the implicit beauty when I have to force-push some refs. -- To unsubscribe from this list: send the line 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: commit-message attack for extracting sensitive data from rewritten Git history
On 9 April 2013 18:01, Jeff King p...@peff.net wrote: On Tue, Apr 09, 2013 at 08:03:24AM +0200, Johannes Sixt wrote: If A mentions B (think of cherry-pick -x), then you must ensure that the branch containing B was traversed first. Yeah, you're right. Multiple passes are necessary to get it completely right. And because each pass may change more commit id's, you have to recurse to pick up those changes, and keep going until you have a pass with no changes. Just to give some context on how the BFG handles this (without doing multiple passes): The BFG makes a design choice (based on it's intended use-case of annihilating unwanted data) that a specific tree or blob will always be cleaned in exactly the same way - because when you're trying to get rid of large blobs or private data, you most likely /don't care/ where it is, what commit it belongs to, how old it is. The id for a cleaned tree or blob is always the same no matter where it came from, and so the BFG maintains a in-memory mapping of 'dirty' to 'clean' object ids while cleaning a repo - whenever an object (commit, tag, tree, blob) is cleaned, these values are stored in the map: dirty-id - clean-id clean-id - clean-id (in terms of memory overhead, this amounts to only ~ 128MB for even quite a large repo like the linux kernel, so I don't spend much time worrying about it) The map memoises the cleaning functions on all objects, so an object (particularly a tree) never gets cleaned more than once, which is one of the things that makes the BFG fast. Having these memoised functions makes cleaning commit messages fairly easy - the message is grepped for hex strings more than a few characters in length, and if a matched string resolves uniquely to an object id in the repo, the clean() method is called on it to get the cleaned id - which will either return immediately with a previously calculated result, or if the id came from a different branch, trigger a cascade of more cleaning, eventually returning the required cleaned id. In the case of git-filter-branch, the user has a lot more freedom to change the tree-structure of commits on a commit-by-commit basis, so memoising tree-cleaning is out of the question, but I guess it might be possible to do memoisation of just the commit ids to short-cut the multiple-pass problem. - Roberto Tyley -- To unsubscribe from this list: send the line 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: [ITCH] Specify refspec without remote
Ramkumar Ramachandra wrote: [...] Let's not do anything too complex, and just aim for a more pleasant experience for the simple case of force-pushing some refs without the :dst counterpart. Then, all we have to do is verify that what is specified is not a valid remote, and is not a valid local path (to a git repository, but we don't need to check that), correct? -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Junio C Hamano wrote: But there are other cases to attempt to add a path that do not belong to our project, which do not have to involve a symbolic link in the leading path. The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Perhaps clarify with: s/there are cases/there may be cases/ and append One such case that we currently don't handle yet is a path inside another git repository in our worktree, as demonstrated by test t.X. -- To unsubscribe from this list: send the line 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] gitweb: Make feed title valid utf8
W dniu 09.04.2013 19:40, Jürgen Kreileder napisał: Jakub Narębski jna...@gmail.com writes: Jürgen Kreileder wrote: Properly encode site and project names for RSS and Atom feeds. - my $title = $site_name - $project/$action; + my $title = to_utf8($site_name) . - . to_utf8($project) . /$action; Was this patch triggered by some bug? Yes, I actually see broken encoding with the old code, e.g on https://git.blackdown.de/old.cgi?p=contactalbum.git;a=rss my first name is messed up in the title tag. New version: https://git.blackdown.de/?p=contactalbum.git;a=rss Because the above is not necessary, as git_feed() has $title = esc_html($title); a bit later, which does to_utf8() internally. Good point. But it doesn't fix the string in question: It looks like to_utf8($a $b) != (to_utf8($a) . . to_utf8($b)). Strange. I wonder if the bug is in our to_utf8() implementation, or in Encode, or in Perl... and whether this bug can be triggered anywhere else in gitweb. What Perl version and Encode module version do you use? -- Jakub Narębski -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
W dniu 09.04.2013 19:34, Junio C Hamano pisze: - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if (path_outside_our_project(path, len)) + return error('%s' is outside our working tree, path); Don't we lose important information here? Or we shouldn't care? -- Jakub Narębski -- To unsubscribe from this list: send the line 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-am doesn't apply the rest of the email after a partial patch fail?
On 8 April 2013 10:49, Junio C Hamano gits...@pobox.com wrote: Constantine A. Murenin muren...@gmail.com writes: However, what I've faced with, is that when a conflict happens, and I resolve, and do `git add`, and `git am --resolved`, then the rest of the `format-patch` email where the conflict has occurred is discarded, That is unusual. Are you using any other options when running git am? You said `git add`, but what did you add? By default, its patch application is all-or-none, so when it stops, saying I cannot apply this patch---please help me with it, it expects that all the changes the email wanted to give you has been applied by you to your working tree, perhaps using GNU patch or git apply --reject, followed by manual editing, and to your index using git add, when you run git am --resolved. Not just the file (or hunk) it detected issues with. Well, I now know this, but it wasn't clear from the documentation that that was the behaviour. Also, I've now noticed that --reject doesn't automatically do `git add` of any new files that were added, so, once you resolve the conflicts, and add those files that used to result in a conflict, you're still missing out. C. -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: I think what the callers of this function care about is if the name is a path that should not be added to our index (i.e. points outside the repository). If you had a symlink d that points at e when our project does have a subdirectory e with file f, check_leading_path(d/f) wants to say bad, even though the real file pointed at, i.e. e/f is inside our working tree, so outside our working tree is not quite correct in the strict sense (this applies equally to has_symlink_leading_path), but Actually, you introduced one naming regression: has_symlink_leading_path() is a good name for what the function does, as opposed to die_if_path_outside_our_tree(), which is misleading. What about die_if_path_contains_links() to encapsulate gitlinks and symlinks? The cases we know that $d/f (where $d is a path that is one or more levels, e.g. dir, d/i, or d/i/r) is bad are when - $d is a symlink, because what you could add to the index is $d and nothing underneath it; or - $d is a directory that is the top level of the working tree that is controled by $d/.git, because what you could add to the index is $d and nothing underneath it. If $d were added to our index, the former will make 12 entry and the latter will make 16 entry. But the user may not want to add $d ever to our project, so in that case, neither will give us a symlink or a gitlink. We should find a word that makes it clear that this path is beyond something we _could_ add. I do not think link is a good word for it. It shares the same mistake that led to the original misnomer, i.e. the case we happened to notice was when we have symlink so let's name it with 'symlink' somewhere in it. I think we should treat the case where d (and d/f) belongs to the working tree of a repository for a separate project, that is embedded in our working tree the same way. I'm not too sure about this. It means that I can have symlinks to files in various parts of my worktree, but not to directories. It does not mean that. It is valid to do ln -s myetc /etc git add myetc It is NOT valid to do git add myetc/passwd One can have symlinks to anywhere all one wants. We track symlinks. It is the same for the top-level of the working tree of a separate project, be it a submodule or not. It is valid to do mkdir foo (cd foo git init file) git add foo It is NOT valid to do git add foo/file -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Ramkumar Ramachandra artag...@gmail.com writes: The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Because it wasn't written back then? Perhaps clarify with: s/there are cases/there may be cases/ and append One such case that we currently don't handle yet is a path inside another git repository in our worktree, as demonstrated by test t.X. I think we currently don't handle is a misstatement. It is not a bug that we don't. -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:34, Junio C Hamano pisze: -if (has_symlink_leading_path(path, len)) -return error('%s' is beyond a symbolic link, path); +if (path_outside_our_project(path, len)) +return error('%s' is outside our working tree, path); Don't we lose important information here? Or we shouldn't care? What important information is it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-am doesn't apply the rest of the email after a partial patch fail?
Constantine A. Murenin muren...@gmail.com writes: Well, I now know this, but it wasn't clear from the documentation that that was the behaviour. Yes, the message after you _resolved_, please tell me you are now done is too fuzzy. What it wants to say is: I punted, because the patch does not apply, and it is stored here. Update the index to hold contents that the sender of the patch would have liked to see if the patch were to apply cleanly. Tell me when you are done, with am --resolved. Then I'll commit that content in the index for you with the authorship and log message I learned from the e-mail and continue. -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Junio C Hamano wrote: Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:34, Junio C Hamano pisze: - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if (path_outside_our_project(path, len)) + return error('%s' is outside our working tree, path); Don't we lose important information here? Or we shouldn't care? What important information is it? That the cause is symbolic link (or other git repository, in the future). -- Jakub Narębski -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Junio C Hamano gits...@pobox.com writes: Ramkumar Ramachandra artag...@gmail.com writes: The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Because it wasn't written back then? Perhaps clarify with: s/there are cases/there may be cases/ and append One such case that we currently don't handle yet is a path inside another git repository in our worktree, as demonstrated by test t.X. I think we currently don't handle is a misstatement. It is not a bug that we don't. We can think of it this way. In your working tree, there is an upper bound for the paths you can include in your commit. When you are at the top-level of your working tree, you do not say git add ../f or git add ../d/f. The root-level of your working tree is an upper bound and you do not cross that boundary. It turns out that there are lower bounds for the paths as well. When we say Git tracks symbolic links, anything that appears beyond a symbolic link is beyond that boundary. If we track a symbolic link l, we can of course add l. When l leads to a directory somewhere else, the filesystem gives you an illusion that there are things under l (e.g. l points at /etc and there is l/passwd there), but that is beyond the boundary. You do not add l/passwd. Otherwise git add l would become meaningless. Does it add the symbolic link itself, or all the files in there, pretending l is actually a directory? We have chosen to say it is the former, and apply that rule consistently. It is the same for Git tracks submodules, which defines that the top-level of the submodule working tree as such a lower boundary. -- To unsubscribe from this list: send the line 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] add: refuse to add paths beyond repository boundaries
Jakub Narębski jna...@gmail.com writes: Junio C Hamano wrote: Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:34, Junio C Hamano pisze: - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if (path_outside_our_project(path, len)) + return error('%s' is outside our working tree, path); Don't we lose important information here? Or we shouldn't care? What important information is it? That the cause is symbolic link (or other git repository, in the future). And in what way is it important? -- To unsubscribe from this list: send the line 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] t/README: --immediate skips cleanup commands for failed tests
Simon Ruderich si...@ruderich.org writes: On Sun, Apr 07, 2013 at 03:32:00PM -0700, Jonathan Nieder wrote: I'm not sure if it's better to use test_when_finished with rm or just rm -rf tmp at the end of the test in case someone wants to look at the output. test_when_finished is better here, since it means later tests can run and provide useful information about how bad a regression is. Cleanup commands requested using test_when_finished are not run when a test being run with --immediate fails, so you can still inspect output after a failed test. Hello Jonathan, Thanks for the explanation. I couldn't find this documented in t/README, the following patch adds it. -- 8 -- Subject: [PATCH] t/README: --immediate skips cleanup commands for failed tests --- Sign-off? t/README | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/README b/t/README index 9b41fe7..e5e7d37 100644 --- a/t/README +++ b/t/README @@ -86,7 +86,8 @@ appropriately before running make. --immediate:: This causes the test to immediately exit upon the first - failed test. + failed test. Cleanup commands requested with + test_when_finished are not executed if the test failed. Perhaps adding ... to keep the state for inspection by the tester to diagnose the bug or something is in order? --long-tests:: This causes additional long-running tests to be run (where -- 1.8.2.481.g0d034d4 -- 8 -- Regards Simon -- To unsubscribe from this list: send the line 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] gitweb: Make feed title valid utf8
Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:40, Jürgen Kreileder napisał: Jakub Narębski jna...@gmail.com writes: Jürgen Kreileder wrote: Properly encode site and project names for RSS and Atom feeds. - my $title = $site_name - $project/$action; + my $title = to_utf8($site_name) . - . to_utf8($project) . /$action; Was this patch triggered by some bug? Yes, I actually see broken encoding with the old code, e.g on https://git.blackdown.de/old.cgi?p=contactalbum.git;a=rss my first name is messed up in the title tag. New version: https://git.blackdown.de/?p=contactalbum.git;a=rss Because the above is not necessary, as git_feed() has $title = esc_html($title); a bit later, which does to_utf8() internally. Good point. But it doesn't fix the string in question: It looks like to_utf8($a $b) != (to_utf8($a) . . to_utf8($b)). Strange. I wonder if the bug is in our to_utf8() implementation, or in Encode, or in Perl... and whether this bug can be triggered anywhere else in gitweb. I don't think it's a bug, more like a consequence of concatenating utf8 and non-utf8 strings: my $a = ü; my $b = ü; my $c = $a - $b; print $c - . to_utf8($c) . : . (utf8::is_utf8($c) ? utf8 : not utf8) . \n; # GOOD $b = to_utf8($b); $c = $a - $b; print $c - . to_utf8($c) . : . (utf8::is_utf8($c) ? utf8 : not utf8) . \n; # GOOD yields (hopefully the broken encoding shows up correctly here): ü - ü - ü - ü: not utf8 ü - ü - ü - ü: utf8 In gitweb we have the bad case: my $title = $site_name - $project/$action; $project and $action are apparently utf8 already but $site_name isn't. The resulting string is marked as utf8 - although the encoding of $site_name was never fixed. The to_utf8() in esc_html() returns the string without fixing anything because of that. What Perl version and Encode module version do you use? 5.14.2 and 2.42_01 on Ubuntu. Same results with 5.12.4 and 2.39 on OS X. Juergen -- To unsubscribe from this list: send the line 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: [ITCH] Specify refspec without remote
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: What is bad about saying push origin ...the rest...? I don't know which remote to push to: all I know is that the remote to push to is configured somewhere in the web of ... Ahh, and then the recent triangular stuff makes it even worse. I can see why we might want some token that says I am pushing where I would push normally if this were a 'git push' that does not say 'to where' and 'push what' to help users. Some background to explain why I was hesitant to the change. There must be a reason why the user wants to use a custom set of refspecs, not the configured ones, for this particular push only. There must be something special for this particular push that makes it different from the configured 'git push' default. I was wondering if it is sensible to assume that the user is likely to still want to push to the same repository where she usually pushes to, when she has that special reason. The underlying assumption for it to be sensible is that 'to where' (destination repository) is more sticky than 'push what' (the refspecs). And I think now I agree that indeed is a sensible assumption. I am not sure '-' is a good token for that, but I do not offhand think of a reason why '-' would be a _bad_ token for that, either. -- To unsubscribe from this list: send the line 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: tar on Mac does not like empty tar from git archive
Am 08.04.2013 23:05, schrieb Jeff King: On Mon, Apr 08, 2013 at 02:36:05PM -0400, BJ Hargrave wrote: Git 1.8.2.1 includes commit bd54cf17 - archive: handle commits with an empty tree Test 2 of t5004-archive-corner-cases, tar archive of empty tree is empty, fails on Mac OS X 10.8.3 (with XCode 4.6.1) since the tar command exits with return code 1 on the generated tar file. Hmm. So I guess the question is: do we need to work around this in the test for platforms that do not like empty tar files, or are the empty tarfiles we are making wrong somehow? tar --version bsdtar 2.8.3 - libarchive 2.8.3 It appears that bsdtar does not like the empty tar files created by git archive. An empty tar file created by bsdtar is accepted. tar cT /dev/null | tar t; echo $? 0 That makes me think the latter (we are wrong). I don't have my OS X box handy; can you provide a sample empty tarfile that it creates? libarchive (on which bsdtar is based) doesn't like extended pax headers at the end of archives. Here's the relevant source file: https://github.com/libarchive/libarchive/blob/master/libarchive/archive_read_support_format_tar.c tar_read_header() calls header_pax_global() to handle a global pax header, which in turn calls tar_read_header() again to fetch the next header. If it reaches the end of the archive then err is set to ARCHIVE_EOF and Damaged tar archive is reported at the end of this function. I tried come up with a small patch that convinces it to ignore such a condition, but it's apparently not as easy as it looks -- I just made bsdtar report even more obscure errors. Will look deeper into it later this week. René -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Locating merge that dropped a change
This morning, I was struggling (not for the first time) to produce a Git command that would identify a merge commit that dropped a change. I could see where it was added, but couldn't automate finding out why it wasn't any longer in HEAD. All the permutations of --full-history, -m, -S, -G on git log I could think of did not get me anywhere. As long as I had --full-history, they could find the original commit that had added the change, but not the merge commit that had dropped it by taking the other parent. So, how to automatically find a merge that ignored a known change? And then for visualisation purposes, how do you persuade gitk's diff display to actually show that that merge commit removed the change from one of its parents? Again, -m didn't seem to work. Help appreciated! Kevin -- To unsubscribe from this list: send the line 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] gitweb: Make feed title valid utf8
W dniu 09.04.2013 21:22, Jürgen Kreileder napisał: Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:40, Jürgen Kreileder napisał: Jakub Narębski jna...@gmail.com writes: Jürgen Kreileder wrote: Properly encode site and project names for RSS and Atom feeds. Good point. But it doesn't fix the string in question: It looks like to_utf8($a $b) != (to_utf8($a) . . to_utf8($b)). Strange. I wonder if the bug is in our to_utf8() implementation, or in Encode, or in Perl... and whether this bug can be triggered anywhere else in gitweb. I don't think it's a bug, more like a consequence of concatenating utf8 and non-utf8 strings: my $a = ü; my $b = ü; my $c = $a - $b; print $c - . to_utf8($c) . : . (utf8::is_utf8($c) ? utf8 : not utf8) . \n; # GOOD $b = to_utf8($b); $c = $a - $b; print $c - . to_utf8($c) . : . (utf8::is_utf8($c) ? utf8 : not utf8) . \n; # GOOD yields (hopefully the broken encoding shows up correctly here): ü - ü - ü - ü: not utf8 ü - ü - ü - ü: utf8 Ah, so it looks like it is misfeature of the way Perl handles Unicode; concatenating adds 'UTF8' flag if either of concatenates strings has it to the result. [Which I have checked using Devel::Peek with perl -MDevel::Peek -E ' my $a = ż; my $b = \x{17c}; Dump $a; Dump $b; Dump $b - $a' ] In gitweb we have the bad case: my $title = $site_name - $project/$action; $project and $action are apparently utf8 already but $site_name isn't. $project and $action are taken from URL, and we have to run decode_utf8 (at least for query params) for gitweb to work correctly. $site_name is usually taken from config file, and gitweb doesn't have use utf8 pragma. The resulting string is marked as utf8 - although the encoding of $site_name was never fixed. The to_utf8() in esc_html() returns the string without fixing anything because of that. O.K. _Maybe_ it would be worth adding explanation of this to commit message (and I see I should audit gitweb for similar problems elsewhere), but anyway Acked-by: Jakub Narebski jna...@gmail.com -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Using a bit more decoration
Jeff King p...@peff.net writes: +static inline int *slab_at(struct commit_slab *s, const struct commit *c) +{ + if (s-alloc = c-index) { + int new_alloc = alloc_nr(s-alloc); + if (new_alloc = c-index) + new_alloc = c-index + 1; + + s-buf = xrealloc(s-buf, new_alloc * sizeof(*s-buf)); + memset(s-buf + s-alloc, 0, new_alloc - s-alloc); + s-alloc = new_alloc; + } + return s-buf + c-index; +} This will hurt more as the number of objects we deal with grows (our ALLOC_GROW() shares the same). I wonder if it might be a good idea to do struct commit_slab { int piece_size; int piece_count; struct commit_slab_piece { int *buf; } *piece; }; and then make the look-up logic like this: int nth_piece, nth_slot; nth_piece = c-index / s-piece_size; nth_slot = c-index % s-piece_size; if (s-piece_count = nth_piece) { /* xrealloc s-piece to grow, update s-piece_count */ } if (!s-piece[nth_piece]) { /* xcalloc s-piece[nth_piece] to allocate */ } return s-piece[nth_piece]-buf + nth_slot; Other than that, looks like a good technology demonstration. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages
Jakub Narębski jna...@gmail.com writes: W dniu 08.04.2013 22:10, Jürgen Kreileder pisze: Fix broken blob action parameters on blobdiff and commitdiff pages by explicitly passing variables instead of relying on global ones. Do I understand it correctly that those variables (e.g. $hash variable in git_patchset_body in second chunk below, after this change passed as parameter to format_diff_cc_simplified()) can be filled in then, or at least adjusted correctly? Yep. (The broken parameters on blob links lead to blob pages which show the blob but with a hash instead of a commit message and have broken blob_plain (404 - Cannot find file) and tree links (404 - Reading tree failed)) I wonder how we missed this. Does this happen always, or in some specific conditions? Just having a hash in the diff header and missing navigation links for lhs links in diffs happen pretty much all the time: E.g. follow any a/... link on http://repo.or.cz/w/alt-git.git/commitdiff/5bda18c186e455f8e65f976d3bf333ab1f4b5b53 (Btw, on http://repo.or.cz/w/alt-git.git/blobdiff/6466fbbeef67ec398f6f5d5b5da5d6f107a647b2..5bda18c186e455f8e65f976d3bf333ab1f4b5b53:/GIT-VERSION-GEN both the a/ und b/ link are broken. No idea if my patch fixes that, I don't use pathinfo because I've had similar problems with it when I tested it a year ago). Broken a/ and b/ + broken tree + broken blob_plain links are more rare. I see them on https://git.blackdown.de/old.cgi?p=contactalbum.git;a=blobdiff;f=Classes/ContactAlbum.h;h=8f2b6a772ba0a4530e0c441db53ae5bbc0f2e277;hp=03c43ea01a3a88bbce3c73df9bfde64ee7a13717;hb=cc4eaa64c2b399dd9bdbf1f67f6d621aa24df5f8;hpb=43ff97d2c247517f43d83314aaa215e9f61d5d0c;js=1 for example: When following either a/ or b/, all links in [contactalbum.git] / Classes / ContactAlbum.h are broken. With my patch I don't see these problems: https://git.blackdown.de/?p=contactalbum.git;a=blobdiff;f=Classes/ContactAlbum.h;h=8f2b6a772ba0a4530e0c441db53ae5bbc0f2e277;hp=03c43ea01a3a88bbce3c73df9bfde64ee7a13717;hb=cc4eaa64c2b399dd9bdbf1f67f6d621aa24df5f8;hpb=43ff97d2c247517f43d83314aaa215e9f61d5d0c;js=1 Juergen -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] submodule: drop the top-level requirement
Since version 1, patch 1 has been completely re-written using the approach proposed by Jonathan and Junio. Also, there's now a documentation update and some tests. John Keeping (2): rev-parse: add --filename-prefix option submodule: drop the top-level requirement Documentation/git-rev-parse.txt | 16 builtin/rev-parse.c | 24 --- git-submodule.sh| 7 t/t1513-rev-parse-prefix.sh | 90 + t/t7400-submodule-basic.sh | 26 t/t7401-submodule-summary.sh| 9 + 6 files changed, 167 insertions(+), 5 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh -- 1.8.2.694.ga76e9c3.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] rev-parse: add --filename-prefix option
This adds a prefix string to any filename arguments encountered after it has been specified. Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/git-rev-parse.txt | 16 builtin/rev-parse.c | 24 --- t/t1513-rev-parse-prefix.sh | 90 + 3 files changed, 125 insertions(+), 5 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt index 1f9ed6c..0ab2b77 100644 --- a/Documentation/git-rev-parse.txt +++ b/Documentation/git-rev-parse.txt @@ -59,6 +59,22 @@ OPTIONS If there is no parameter given by the user, use `arg` instead. +--prefix arg:: + Behave as if 'git rev-parse' was invoked from the `arg` + subdirectory of the working tree. Any relative filenames are + resolved as if they are prefixed by `arg` and will be printed + in that form. ++ +This can be used to convert arguments to a command run in a subdirectory +so that they can still be used after moving to the top-level of the +repository. For example: ++ + +prefix=$(git rev-parse --show-prefix) +cd $(git rev-parse --show-toplevel) +eval set -- $(git rev-parse --sq --prefix $prefix $@) + + --verify:: Verify that exactly one parameter is provided, and that it can be turned into a raw 20-byte SHA-1 that can be used to diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index f267a1d..de894c7 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr) show(buffer); } -static int show_file(const char *arg) +static int show_file(const char *arg, int output_prefix) { show_default(); if ((filter (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) { - show(arg); + if (output_prefix) { + const char *prefix = startup_info-prefix; + show(prefix_filename(prefix, +prefix ? strlen(prefix) : 0, +arg)); + } else + show(arg); return 1; } return 0; @@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n int cmd_rev_parse(int argc, const char **argv, const char *prefix) { int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0; + int output_prefix = 0; unsigned char sha1[20]; const char *name = NULL; @@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) const char *arg = argv[i]; if (as_is) { - if (show_file(arg) as_is 2) + if (show_file(arg, output_prefix) as_is 2) verify_filename(prefix, arg, 0); continue; } @@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) as_is = 2; /* Pass on the -- if we show anything but files.. */ if (filter (DO_FLAGS | DO_REVS)) - show_file(arg); + show_file(arg, 0); continue; } if (!strcmp(arg, --default)) { @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) i++; continue; } + if (!strcmp(arg, --prefix)) { + prefix = argv[i+1]; + startup_info-prefix = prefix; + output_prefix = 1; + i++; + continue; + } if (!strcmp(arg, --revs-only)) { filter = ~DO_NOREV; continue; @@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (verify) die_no_single_rev(quiet); as_is = 1; - if (!show_file(arg)) + if (!show_file(arg, output_prefix)) continue; verify_filename(prefix, arg, 1); } diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh new file mode 100755 index 000..5ef48d2 --- /dev/null +++ b/t/t1513-rev-parse-prefix.sh @@ -0,0 +1,90 @@ +#!/bin/sh + +test_description='Tests for rev-parse --prefix' + +. ./test-lib.sh + +test_expect_success 'setup' ' + mkdir -p sub1/sub2 + echo top top + echo file1 sub1/file1 + echo file2 sub1/sub2/file2 +
[PATCH v2 2/2] submodule: drop the top-level requirement
Use the new rev-parse --prefix option to process all paths given to the submodule command, dropping the requirement that it be run from the top-level of the repository. Signed-off-by: John Keeping j...@keeping.me.uk --- git-submodule.sh | 7 +++ t/t7400-submodule-basic.sh | 26 ++ t/t7401-submodule-summary.sh | 9 + 3 files changed, 42 insertions(+) diff --git a/git-submodule.sh b/git-submodule.sh index 79bfaac..bbf7983 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -14,10 +14,13 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference re or: $dashless [--quiet] foreach [--recursive] command or: $dashless [--quiet] sync [--recursive] [--] [path...] OPTIONS_SPEC= +SUBDIRECTORY_OK=Yes . git-sh-setup . git-sh-i18n . git-parse-remote require_work_tree +wt_prefix=$(git rev-parse --show-prefix) +cd_to_toplevel command= branch= @@ -112,6 +115,7 @@ resolve_relative_url () # module_list() { + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) ( git ls-files --error-unmatch --stage -- $@ || echo unmatched pathspec exists @@ -335,6 +339,8 @@ cmd_add() usage fi + sm_path=$wt_prefix$sm_path + # assure repo is absolute or relative to parent case $repo in ./*|../*) @@ -942,6 +948,7 @@ cmd_summary() { fi cd_to_toplevel + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) # Get modified modules cared by user modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- $@ | sane_egrep '^:([0-7]* )?16' | diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index ff26535..7795f21 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // in path' ' test_cmp empty untracked ' +test_expect_success 'submodule add in subdir' ' + echo refs/heads/master expect + empty + ( + mkdir addtest/sub + cd addtest/sub + git submodule add $submodurl ../realsubmod3 + git submodule init + ) + + rm -f heads head untracked + inspect addtest/realsubmod3 ../.. + test_cmp expect heads + test_cmp expect head + test_cmp empty untracked +' + test_expect_success 'setup - add an example entry to .gitmodules' ' GIT_CONFIG=.gitmodules \ git config submodule.example.url git://example.com/init.git @@ -319,6 +336,15 @@ test_expect_success 'status should be up-to-date after update' ' grep ^ $rev1 list ' +test_expect_success 'status works correctly from a subdirectory' ' + mkdir sub + ( + cd sub + git submodule status ../list + ) + grep ^ $rev1 list +' + test_expect_success 'status should be modified after submodule commit' ' ( cd init diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 30b429e..8f5c1e0 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -45,6 +45,15 @@ EOF test_cmp expected actual +test_expect_success 'run summary from subdir' ' + mkdir sub + ( + cd sub + git submodule summary ../actual + ) + test_cmp expected actual +' + commit_file sm1 head2=$(add_file sm1 foo3) -- 1.8.2.694.ga76e9c3.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: But there are other cases to attempt to add a path that do not belong to our project, which do not have to involve a symbolic link in the leading path. The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Perhaps clarify with: s/there are cases/there may be cases/ and append One such case that we currently don't handle yet is a path inside another git repository in our worktree, as demonstrated by test t.X. I _think_ I misread what you meant to say in the above. We can go either way between are cases or may be cases. I meant it as an immediate predecessor ([PATCH 1/n]) to the patch you were working on ([PATCH 2/n] and later), so in that context, it does not matter. [PATCH 2/n] will start as Now the naming is saner, let's start noticing when the user gives a path is beyond our project boundary because it is under control of another repository by adding necessary logic to that function. And I also misread we currently don't handle above as but we really should allow adding d/f when d is at the top of the working tree of another project, but that was not what you meant to say. Instead, We do not notice such a bad case in today's code yet was what you meant. But if we are to use there are cases in [1/n] and start [2/n] with Now we have renamed, let's do this, then we do not have to bother saying anything in [1/n] about the upcoming change in [2/n], especially the patches come back-to-back in a single series. -- To unsubscribe from this list: send the line 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-archive --worktree-attributes (1.8.2)
Hi, In the help for git-archive it states: --worktree-attributes Look for attributes in .gitattributes in working directory too. This doesn't seem to be the case. I have a use case where I need to archive a remote I don't have write access too (via --remote=), and I wish to ignore certain paths. When I chdir to a temp dir, create a .gitattributes file, and call git archive --worktree-attributes, the bevaviour I see is that it is ignoring .gitattributes. I've tried setting GIT_WORK_TREE to my temp dir, without success. Is the documentation wrong about 'working directory'? Did it mean to say 'working tree'? Looking at the source for archive.c, it seems to assume a GIT_WORK_TREE. Would you accept a patch that either adds an option to explicitly set the .gitattributes file or would let --worktree-attributes look in $PWD. Thanks, Amit -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option
John Keeping j...@keeping.me.uk writes: This adds a prefix string to any filename arguments encountered after it has been specified. Signed-off-by: John Keeping j...@keeping.me.uk --- Stale subject? +--prefix arg:: + Behave as if 'git rev-parse' was invoked from the `arg` + subdirectory of the working tree. Any relative filenames are + resolved as if they are prefixed by `arg` and will be printed + in that form. ++ +This can be used to convert arguments to a command run in a subdirectory +so that they can still be used after moving to the top-level of the +repository. For example: ++ + +prefix=$(git rev-parse --show-prefix) +cd $(git rev-parse --show-toplevel) +eval set -- $(git rev-parse --sq --prefix $prefix $@) I think you should tighten rev-parse parameter to reject options and revisions, especially as an example to teach how to use it. When the user said git mylog -U20 master..next -- README inside Documentation/ directory, git-mylog script would want to see README prefixed with Documentation/ but want to see -U20 or master..next untouched. Historically, rev-parse was a way to sift options and args meant for rev-list from those mant for diff-tree so that a variant of git rev-list $(git rev-parse --revs) $@ | git diff-tree --stdin $(git rev-parse --no-revs) can be used to implement such git mylog script. I think --no-revs --no-flags is how you ask it to give you only the paths, but I am writing from memory so please double check. Having said all that. Existing scripts (e.g. git am) do this kind of things without such an option added to rev-parse. They first do: prefix=$(git rev-parse --show-prefix) and refer to $prefix/$1, $prefix/$2, etc., I think. Is this option really necessary to update git submodule? Don't we have a much better idea which parameter holds user-supplied path in the script than having rev-parse make a guess on the entire $@? -- To unsubscribe from this list: send the line 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] submodule: drop the top-level requirement
John Keeping j...@keeping.me.uk writes: + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) This may be handier than having to do the for arg loop git-am uses yourself. ( git ls-files --error-unmatch --stage -- $@ || echo unmatched pathspec exists @@ -335,6 +339,8 @@ cmd_add() usage fi + sm_path=$wt_prefix$sm_path But this is doing fine without rev-parse --prefix 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: git-archive --worktree-attributes (1.8.2)
Amit Bakshi ambak...@gmail.com writes: Hi, In the help for git-archive it states: --worktree-attributes Look for attributes in .gitattributes in working directory too. ... The worktree-attributes should read from the worktree. It should not pay any attention to where you are in the directory hierarchy, i.e. your current working directory (aka $cwd). Would you accept a patch that either adds an option to explicitly set the .gitattributes file or would let --worktree-attributes look in $PWD. The former, possibly, if accompanied with a good justification and implemented cleanly. The latter, definitely not. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option
On Tue, Apr 09, 2013 at 01:57:21PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: This adds a prefix string to any filename arguments encountered after it has been specified. Signed-off-by: John Keeping j...@keeping.me.uk --- Stale subject? Yep. Sorry. +--prefix arg:: + Behave as if 'git rev-parse' was invoked from the `arg` + subdirectory of the working tree. Any relative filenames are + resolved as if they are prefixed by `arg` and will be printed + in that form. ++ +This can be used to convert arguments to a command run in a subdirectory +so that they can still be used after moving to the top-level of the +repository. For example: ++ + +prefix=$(git rev-parse --show-prefix) +cd $(git rev-parse --show-toplevel) +eval set -- $(git rev-parse --sq --prefix $prefix $@) I think you should tighten rev-parse parameter to reject options and revisions, especially as an example to teach how to use it. When the user said git mylog -U20 master..next -- README inside Documentation/ directory, git-mylog script would want to see README prefixed with Documentation/ but want to see -U20 or master..next untouched. And it will if it runs: git rev-parse --prefix Documentation/ -U20 master..next -- README Which gives: -U20 f131fb6eb2a1e09f7ce53d148e21ce6960f42422 ^fa7285dc3dce8bd01fd8c665b032603ed55348e5 -- Documentation/README Historically, rev-parse was a way to sift options and args meant for rev-list from those mant for diff-tree so that a variant of git rev-list $(git rev-parse --revs) $@ | git diff-tree --stdin $(git rev-parse --no-revs) can be used to implement such git mylog script. I think --no-revs --no-flags is how you ask it to give you only the paths, but I am writing from memory so please double check. Having said all that. Existing scripts (e.g. git am) do this kind of things without such an option added to rev-parse. They first do: prefix=$(git rev-parse --show-prefix) and refer to $prefix/$1, $prefix/$2, etc., I think. Is this option really necessary to update git submodule? Don't we have a much better idea which parameter holds user-supplied path in the script than having rev-parse make a guess on the entire $@? It's not guessing on all of $@ in git-submodule - we know that everything left is a path. I've looked at git-am and hadn't thought of doing that, just thought this was a reasonably elegant way of processing the arguments. I'm happy to try another approach if that's going to be more acceptable. -- To unsubscribe from this list: send the line 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] submodule: drop the top-level requirement
On Tue, Apr 09, 2013 at 02:00:52PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) This may be handier than having to do the for arg loop git-am uses yourself. ( git ls-files --error-unmatch --stage -- $@ || echo unmatched pathspec exists @@ -335,6 +339,8 @@ cmd_add() usage fi + sm_path=$wt_prefix$sm_path But this is doing fine without rev-parse --prefix at all. In this case we only have a single argument (and it must have a value). In the cases using rev-parse --prefix we can have any number of arguments (including zero). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option
John Keeping j...@keeping.me.uk writes: It's not guessing on all of $@ in git-submodule - we know that everything left is a path. OK, 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 v4] transport-helper: report errors properly
Felipe Contreras felipe.contre...@gmail.com writes: If a push fails because the remote-helper died (with fast-export), the user won't see any error message. So let's add one. At the same time lets add tests to ensure this error is reported, and while we are at it, check the error from fast-import [...] +# We sleep to give fast-export a chance to catch the SIGPIPE +test_expect_success 'proper failure checks for pushing' ' + (GIT_REMOTE_TESTGIT_FAILURE=1 + export GIT_REMOTE_TESTGIT_FAILURE + cd local + test_must_fail git push --all 2 error + cat error + grep -q Reading from remote helper failed error + ) +' There appears to be a race in the version that is in today's pu (5eb25f737b). I reproduced with this: cd git/t i=1 while ./t5801-remote-helpers.sh --root=/dev/shm --valgrind do i=$(($i+1)) done Two out of six of these loops quit within 1 and 2 iterations, respectively, both with an error along the lines of: expecting success: (GIT_REMOTE_TESTGIT_FAILURE=1 export GIT_REMOTE_TESTGIT_FAILURE cd local test_must_fail git push --all 2 error cat error grep -q Reading from remote helper failed error ) error: fast-export died of signal 13 fatal: Error while running fast-export not ok 21 - proper failure checks for pushing I haven't been able to reproduce outside of valgrind tests. Is this an expected issue, caused by overrunning the sleep somehow? If so, can you increase the sleep delay under valgrind so as to not cause intermittent failures in the test suite? -- 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 v2] t/README: --immediate skips cleanup commands for failed tests
Signed-off-by: Simon Ruderich si...@ruderich.org --- On Tue, Apr 09, 2013 at 12:16:56PM -0700, Junio C Hamano wrote: Sign-off? Sorry, forgot it. Perhaps adding ... to keep the state for inspection by the tester to diagnose the bug or something is in order? Good idea. Revised patch is attached. Regards Simon t/README | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/README b/t/README index 9b41fe7..e6c060e 100644 --- a/t/README +++ b/t/README @@ -86,7 +86,10 @@ appropriately before running make. --immediate:: This causes the test to immediately exit upon the first - failed test. + failed test. Cleanup commands requested with + test_when_finished are not executed if the test failed to + keep the state for inspection by the tester to diagnose + the bug. --long-tests:: This causes additional long-running tests to be run (where -- 1.8.2.481.g0d034d4 -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- To unsubscribe from this list: send the line 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] transport-helper: report errors properly
On Tue, Apr 09, 2013 at 11:38:05PM +0200, Thomas Rast wrote: Two out of six of these loops quit within 1 and 2 iterations, respectively, both with an error along the lines of: expecting success: (GIT_REMOTE_TESTGIT_FAILURE=1 export GIT_REMOTE_TESTGIT_FAILURE cd local test_must_fail git push --all 2 error cat error grep -q Reading from remote helper failed error ) error: fast-export died of signal 13 fatal: Error while running fast-export not ok 21 - proper failure checks for pushing I haven't been able to reproduce outside of valgrind tests. Is this an expected issue, caused by overrunning the sleep somehow? If so, can you increase the sleep delay under valgrind so as to not cause intermittent failures in the test suite? Yeah, I am not too surprised. The failing helper sleeps before exiting so that fast-export puts all of its data into the pipe buffer before the helper dies, and does not get SIGPIPE. But obviously the sleep is just delaying the problem if your fast-export runs really slowly (which, if you are running under valgrind, is a possibility). The helper should instead just consume all of fast-export's input before exiting, which accomplishes the same thing, finishes sooner in the normal case, and doesn't race. And I think it also simulates a reasonable real-world setup (a helper reads and converts the data, but then dies while writing the output to disk, the network, or whatever). I posted review comments, including that, and I'm assuming that Felipe is going to re-roll at some point. -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/4] gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, commitdiff_plain, and patch
Jakub Narębski jna...@gmail.com writes: On 08.04.2013, Junio C Hamano wrote: j...@blackdown.de (Jürgen Kreileder) writes: Fixes the encoding for several _plain actions and for text/* and */*+xml blobs. Signed-off-by: Jürgen Kreileder j...@blackdown.de I see that this patch does (or tries to do) two _independent_ things, and should be split into at least two separate commits: Agreed. --- Thanks, will queue but not hold until I hear something from Jakub. gitweb/gitweb.perl |8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 1309196..9cfe5b5 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -3823,7 +3823,7 @@ sub blob_contenttype { my ($fd, $file_name, $type) = @_; $type ||= blob_mimetype($fd, $file_name); - if ($type eq 'text/plain' defined $default_text_plain_charset) { + if (($type =~ m!^text/\w[-\w]*$! || $type =~ m!^\w[-\w]*/\w[-\w]*\+xml$!) defined $default_text_plain_charset) { $type .= ; charset=$default_text_plain_charset; } First, it extends adding ; charset=$default_text_plain_charset to other mimetypes for 'blob_plain' view to all 'text/*' and '*/*+xml' mimetypes (without changing name of variable... though this is more complicated as it is configuration variable and we would want to preserve backward compatibility, but at least a comment would be, I think, needed). Originally it applied only to 'text/plain' files, which can be displayed inline by web browser, and which need charset in 'Content-Type:' HTTP header to be displayed correctly, as they do not include such information inside the file. What prompted the change is that some browsers (Chrome and Safari) also display other file types inline: text/x-chdr, text/x-java, text/x-objc, text/x-sh, ... At least on my system these files do get served with charset set! (ISO-8859-1 even though Apache has AddDefaultCharset UTF-8...) $ curl -I https://git.blackdown.de/old.cgi?p=contactalbum.git;a=blob_plain;f=Classes/ContactAlbum.h;hb=HEAD; HTTP/1.1 200 OK Date: Tue, 09 Apr 2013 21:47:30 GMT Server: Apache Content-disposition: inline; filename=Classes/ContactAlbum.h X-Frame-Options: SAMEORIGIN Vary: User-Agent X-UA-Compatible: IE=edge Strict-Transport-Security: max-age=31536000 Content-Type: text/x-chdr; charset=ISO-8859-1 'text/html' and 'application/xhtml+xml' can include such information inside them (meta http-equiv for 'text/html' and ?xml ... for 'application/xhtml+xml'). I don't know what browser does when there is conflicting information about charset, i.e. which one wins, but it is certainly something to consider. As shown above, even without my patch, this already can happen! It might be a good change; I don't know what web browser do when serving 'text/css', 'text/javascript', 'text/xml' to client to view without media type known. BTW I have noticed that we do $prevent_xss dance outside blob_contenttype(), in it's only caller i.e. git_blob_plain()... which for example means that 'text/html' converted to 'text/plain' don't get '; charset=...' added. I guess that it *might* be what prompted this part of change... but if it is so, it needs to be fixed at source, e.g. by moving $prevent_xss to blob_contenttype() subroutine. Jep. [...] Second it changes 'blobdiff_plain', 'commitdiff_plain' (which I think that should be abandoned in favor of 'patch' view; but that is a separate issue) and 'patch' views so they use binary-safe output. Note that in all cases (I think) we use $cgi-header( -type = 'text/plain', -charset = 'utf-8', ... ); promising web browser that output is as whole in 'utf-8' encoding. Yes. It is not explained in the commit message what is the reason for this change. Is it better handing of a situation where files being diff-ed being in other encoding (like for example in commit that changes encoding of files from legacy encoding such like e.g. iso-8859-2 or cp1250 to utf-8)? I do see encoding problems when comparing utf8 to utf8 files (i.e. no encoding change). For instance: https://git.blackdown.de/old.cgi?p=contactalbum.git;a=commitdiff_plain;h=cc4eaa64c2b399dd9bdbf1f67f6d621aa24df5f8 I don't claim to be an expert in Perl's utf8 handling but I guess this because Perl's internal utf8 form differs from the normal utf8 out from git commands. Switching to :raw fixes that: We write plain utf8 (and, as noted above, charset is set to utf8 already) With the patch: https://git.blackdown.de/?p=contactalbum.git;a=commitdiff_plain;h=cc4eaa64c2b399dd9bdbf1f67f6d621aa24df5f8 But what about -charset = 'utf-8' then? That's a good question. I think I never tried git with anything besides ISO-8859-1 (rarely), UTF8 (mostly), and UTF16 (some Xcode files). (UTF16 definitely causes problems for gitweb.) About implementation: I think after this change common code
Re: [PATCH] http-backend: respect GIT_NAMESPACE with dumb clients
On Mon, Apr 8, 2013 at 2:45 PM, Jeff King p...@peff.net wrote: On Mon, Apr 08, 2013 at 11:25:39PM +0200, Thomas Rast wrote: At the risk of repeating something that's been said already -- I only skimmed the thread -- this test breaks in today's pu on my machine. I get: [...] --- expect2013-04-08 21:24:36.571874540 + +++ actual2013-04-08 21:24:36.579874619 + @@ -1,3 +1,2 @@ -453190505bf07f7513bed9839da875eb3610f807 HEAD 453190505bf07f7513bed9839da875eb3610f807 refs/heads/master 453190505bf07f7513bed9839da875eb3610f807 refs/namespaces/ns/refs/heads/master not ok 14 - backend respects namespaces I think what is in pu is not yet reflecting the latest discussion. HEAD should not be included in the simulated info/refs, but should be generated, respecting namespaces, whenever a client retrieves the HEAD file directly. It looks like the thread was left here; http://article.gmane.org/gmane.comp.version-control.git/220237 and we are just waiting for John's re-roll. I should be able to get this done either later today or later this week. Life/$DAYJOB has been taking up the time slot I was spending on this the past few days. John -- To unsubscribe from this list: send the line 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 0/2] Using gitfile repository with clone --reference
Here's the third version of my series for dealing with gitfiles in clone --reference. The first patch is unchanged from the previous version except for the addition of a Reviewed-by line. The second patch has been modified so that it now supports having a .git file supplied as the argument to the option directly rather than only dealing with that if the containing directory was supplied. This makes the first patch from the series more important, since it would make even less sense to complain that the path isn't a directory when a non-directory is acceptable. I've also fixed the minor style issue in the test script from the previous versions. -- To unsubscribe from this list: send the line 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 1/2] clone: Fix error message for reference repository
Do not report that an argument to clone's --reference option is not a local directory. Nothing checks for the existence or type of the path as supplied by the user; checks are only done for particular contents of the supposed directory, so we have no way to know the status of the supplied path. Telling the user that a directory doesn't exist when that isn't actually known may lead him or her on the wrong path to finding the problem. Instead just state that the entered path is not a local repository which is really all that is known about it. It could be more helpful to state the actual paths which were checked, but I believe that giving a good description of that would be too verbose for a simple error message and would be too dependent on implementation details. Signed-off-by: Aaron Schrab aa...@schrab.com Reviewed-by: Jonathan Nieder jrnie...@gmail.com diff --git a/builtin/clone.c b/builtin/clone.c index f9c380e..0a1e0bf 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) free(ref_git); ref_git = ref_git_git; } else if (!is_directory(mkpath(%s/objects, ref_git))) - die(_(reference repository '%s' is not a local directory.), + die(_(reference repository '%s' is not a local repository.), item-string); strbuf_addf(alternate, %s/objects, ref_git); -- 1.8.2.677.g9202ef0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] clone: Allow repo using gitfile as a reference
Try reading gitfile files when processing --reference options to clone. This will allow, among other things, using a submodule checked out with a recent version of git as a reference repository without requiring the user to have internal knowledge of submodule layout. Signed-off-by: Aaron Schrab aa...@schrab.com diff --git a/builtin/clone.c b/builtin/clone.c index 0a1e0bf..58fee98 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -232,11 +232,21 @@ static void strip_trailing_slashes(char *dir) static int add_one_reference(struct string_list_item *item, void *cb_data) { char *ref_git; + const char *repo; struct strbuf alternate = STRBUF_INIT; - /* Beware: real_path() and mkpath() return static buffer */ + /* Beware: read_gitfile(), real_path() and mkpath() return static buffer */ ref_git = xstrdup(real_path(item-string)); - if (is_directory(mkpath(%s/.git/objects, ref_git))) { + + repo = read_gitfile(ref_git); + if (!repo) + repo = read_gitfile(mkpath(%s/.git, ref_git)); + if (repo) { + free(ref_git); + ref_git = xstrdup(repo); + } + + if (!repo is_directory(mkpath(%s/.git/objects, ref_git))) { char *ref_git_git = mkpathdup(%s/.git, ref_git); free(ref_git); ref_git = ref_git_git; diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh index 2a7b78b..719d778 100755 --- a/t/t5700-clone-reference.sh +++ b/t/t5700-clone-reference.sh @@ -185,4 +185,17 @@ test_expect_success 'fetch with incomplete alternates' ' ! grep want $tag_object $U.K ' +test_expect_success 'clone using repo with gitfile as a reference' ' + git clone --separate-git-dir=L A M + git clone --reference=M A N + echo $base_dir/L/objects expected + test_cmp expected $base_dir/N/.git/objects/info/alternates +' + +test_expect_success 'clone using repo pointed at by gitfile as reference' ' + git clone --reference=M/.git A O + echo $base_dir/L/objects expected + test_cmp expected $base_dir/O/.git/objects/info/alternates +' + test_done -- 1.8.2.677.g9202ef0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
write() _will_ fail on Mac OS X/XNU if nbytes INT_MAX
Hi all, While “git svn fetch”ing a subversion repository (private, sorry), I've encoutered a bug that appears in several git versions (always with the same symptoms): git from master (from 2013-04-08) git version 1.8.2.1 (compiled from homebrew) git version 1.7.12.4 (Apple Git-37) The only symptom is git blowing up with the error: fatal: write error: Invalid argument Problems showed up when this happened (in the SVN repo): rev A: File F with 110K is replaced with a 9G file (don't ask…) intermediate revs: files got added and changed, not touching file F rev B: File F finally got reverted to the state before rev A I can git svn fetch up to rev B-1, but svn fetching rev B will throw the previously mentioned error. I traced it down to write() returning 0 and setting errno to EINVAL if nbytes INT_MAX: http://fxr.watson.org/fxr/source/bsd/kern/sys_generic.c?v=xnu-2050.18.24#L573 (the write() will eventually call dofilewrite, which has that check). Testing with dd bs=INT_MAX+1 count=1 also gets me an “Invalid argument” error, while bs=INT_MAX will do what's expected. I have a preliminary patch that fixes it, but it may not be the preferred way. The code is not ifdef'ed out and I'm doing the fix in write_in_full(), while it may be preferred to do the fix in xwrite(). A radar bug has been submitted to Apple about this, but I think git could tolerate the bug while it isn't fixed, by working around it. Thank you, Filipe git-darwin-bigwrites.patch Description: Binary data
Re: [PATCH v3 0/2] Using gitfile repository with clone --reference
Aaron Schrab aa...@schrab.com writes: Here's the third version of my series for dealing with gitfiles in clone --reference. The first patch is unchanged from the previous version except for the addition of a Reviewed-by line. The second patch has been modified so that it now supports having a .git file supplied as the argument to the option directly rather than only dealing with that if the containing directory was supplied. This makes the first patch from the series more important, since it would make even less sense to complain that the path isn't a directory when a non-directory is acceptable. I've also fixed the minor style issue in the test script from the previous versions. Thanks, will queue. -- To unsubscribe from this list: send the line 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: write() _will_ fail on Mac OS X/XNU if nbytes INT_MAX
Filipe Cabecinhas fil...@gmail.com writes: Testing with dd bs=INT_MAX+1 count=1 also gets me an “Invalid argument” error, while bs=INT_MAX will do what's expected. I have a preliminary patch that fixes it, but it may not be the preferred way. The code is not ifdef'ed out and I'm doing the fix in write_in_full(), while it may be preferred to do the fix in xwrite(). A radar bug has been submitted to Apple about this, but I think git could tolerate the bug while it isn't fixed, by working around it. Thank you, Filipe diff --git a/wrapper.c b/wrapper.c index bac59d2..474d760 100644 --- a/wrapper.c +++ b/wrapper.c @@ -187,7 +187,12 @@ ssize_t write_in_full(int fd, const void *buf, size_t count) ssize_t total = 0; while (count 0) { - ssize_t written = xwrite(fd, p, count); + ssize_t written = 0; +if (count = INT_MAX) + written = xwrite(fd, p, INT_MAX-1); +else + written = xwrite(fd, p, count); I think it is better to fix it in much lower level of the stack, as other codepaths would call write(2), either thru xwrite() or directly. Ideally the fix should go to the lowest level, i.e. the write(2). I do not care if it is done in the kernel or in the libc wrapping code; the above does not belong to our code (in an ideal world, that is). Otherwise you would have to patch everything in /usr/bin, no? But you do not live in an ideal world and neither do we. I think the least ugly way may be to add compat/clipped-write.c that implements a loop like the above in a helper function: ssize_t clipped_write(int, const void *, size_t); and have a #ifdef NEED_CLIPPED_WRITE #define write(x,y,z) clipped_write((x),(y),(z)) #endif in git-compat-util.h, or something. Makefile needs to get adjusted to link with compat/clipped-write.o when NEED_CLIPPED_WRITE is defined as well. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/3] Teach mv to move submodules using a gitfile
Jens Lehmann jens.lehm...@web.de writes: diff --git a/submodule.c b/submodule.c index 975bc87..eba9b42 100644 --- a/submodule.c +++ b/submodule.c @@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path, ... + if (!fp) + die(_(Could not create git link %s), gitfile_name.buf); + fprintf(fp, gitfile_content.buf); Perhaps. fprintf(fp, %s, gitfile_content.buf); -- To unsubscribe from this list: send the line 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: [ITCH] Specify refspec without remote
Junio C Hamano wrote: And I think now I agree that indeed is a sensible assumption. I am not sure '-' is a good token for that, but I do not offhand think of a reason why '-' would be a _bad_ token for that, either. Random idea: today you can do git push origin master; # push branch master to remote origin git push --multiple origin korg; # push default refspec to 2 remotes How about: git push origin korg -- master; # push master to 2 remotes git push -- master next; # push two refs to default remote git push origin -- master; # push master to origin, more explicitly git push origin korg --; # push default refspec to 2 remotes, again git push host:some/path; # ambiguous argument. Please disambiguate. git push host:some/path --; # push default refspec over SSH git push -- host:some/path; # push specified refspec to default remote git push origin; # is a remote name and not a refname. Good. git push master; # is a ref name and not a remote name. Good. What do you think? 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: [ITCH] Specify refspec without remote
Jonathan Nieder wrote: today you can do git push origin master; # push branch master to remote origin git push --multiple origin korg; # push default refspec to 2 remotes Pretend I said fetch. ;-) -- To unsubscribe from this list: send the line 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] http-backend: respect GIT_NAMESPACE with dumb clients
Filter the list of refs returned via the dumb HTTP protocol according to the active namespace, consistent with other clients of the upload-pack service. Signed-off-by: John Koleszar jkoles...@google.com --- Updates to generate HEAD. Drops my original tests, since they were under the flawed assumption that both the dumb and smart protocols produced the same ref advertisement at /info/refs. http-backend.c | 38 ++ t/lib-httpd/apache.conf |5 + t/t5551-http-fetch.sh | 24 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/http-backend.c b/http-backend.c index f50e77f..4f35a31 100644 --- a/http-backend.c +++ b/http-backend.c @@ -361,17 +361,19 @@ static void run_service(const char **argv) static int show_text_ref(const char *name, const unsigned char *sha1, int flag, void *cb_data) { + const char *name_nons = strip_namespace(name); struct strbuf *buf = cb_data; struct object *o = parse_object(sha1); if (!o) return 0; - strbuf_addf(buf, %s\t%s\n, sha1_to_hex(sha1), name); + strbuf_addf(buf, %s\t%s\n, sha1_to_hex(sha1), name_nons); if (o-type == OBJ_TAG) { o = deref_tag(o, name, 0); if (!o) return 0; - strbuf_addf(buf, %s\t%s^{}\n, sha1_to_hex(o-sha1), name); + strbuf_addf(buf, %s\t%s^{}\n, sha1_to_hex(o-sha1), + name_nons); } return 0; } @@ -402,12 +404,40 @@ static void get_info_refs(char *arg) } else { select_getanyfile(); - for_each_ref(show_text_ref, buf); + for_each_namespaced_ref(show_text_ref, buf); send_strbuf(text/plain, buf); } strbuf_release(buf); } +static int show_head_ref(const char *name, const unsigned char *sha1, + int flag, void *cb_data) +{ + struct strbuf *buf = cb_data; + + if (flag REF_ISSYMREF) { + unsigned char sha1[20]; + const char *target = resolve_ref_unsafe(name, sha1, 1, NULL); + const char *target_nons = strip_namespace(target); + + strbuf_addf(buf, ref: %s\n, target_nons); + } else { + strbuf_addf(buf, %s\n, sha1_to_hex(sha1)); + } + + return 0; +} + +static void get_head(char *arg) +{ + struct strbuf buf = STRBUF_INIT; + + select_getanyfile(); + head_ref_namespaced(show_head_ref, buf); + send_strbuf(text/plain, buf); + strbuf_release(buf); +} + static void get_info_packs(char *arg) { size_t objdirlen = strlen(get_object_directory()); @@ -520,7 +550,7 @@ static struct service_cmd { const char *pattern; void (*imp)(char *); } services[] = { - {GET, /HEAD$, get_text_file}, + {GET, /HEAD$, get_head}, {GET, /info/refs$, get_info_refs}, {GET, /objects/info/alternates$, get_text_file}, {GET, /objects/info/http-alternates$, get_text_file}, diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 938b4cf..ad85537 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -61,6 +61,11 @@ Alias /auth/dumb/ www/auth/dumb/ SetEnv GIT_COMMITTER_NAME Custom User SetEnv GIT_COMMITTER_EMAIL cus...@example.com /LocationMatch +LocationMatch /smart_namespace/ + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL + SetEnv GIT_NAMESPACE ns +/LocationMatch ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1 ScriptAlias /broken_smart/ broken-smart-http.sh/ Directory ${GIT_EXEC_PATH} diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index 47eb769..b31019f 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -162,6 +162,30 @@ test_expect_success 'invalid Content-Type rejected' ' grep not valid: actual ' +test_expect_success 'create namespaced refs' ' + test_commit namespaced + git push public HEAD:refs/namespaces/ns/refs/heads/master + git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/repo.git \ + symbolic-ref refs/namespaces/ns/HEAD refs/namespaces/ns/refs/heads/master +' + +test_expect_success 'smart clone respects namespace' ' + git clone $HTTPD_URL/smart_namespace/repo.git ns-smart + echo namespaced expect + git --git-dir=ns-smart/.git log -1 --format=%s actual + test_cmp expect actual +' + +test_expect_success 'dumb clone via http-backend respects namespace' ' + git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/repo.git \ + config http.getanyfile true + GIT_SMART_HTTP=0 git clone \ + $HTTPD_URL/smart_namespace/repo.git ns-dumb + echo namespaced expect + git --git-dir=ns-dumb/.git log -1 --format=%s actual + test_cmp expect actual +' + test -n $GIT_TEST_LONG test_set_prereq EXPENSIVE
Re: [PATCH v4] http-backend: respect GIT_NAMESPACE with dumb clients
Thanks; will replace the previous one that has been in 'pu'. -- To unsubscribe from this list: send the line 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: [ITCH] Specify refspec without remote
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: And I think now I agree that indeed is a sensible assumption. I am not sure '-' is a good token for that, but I do not offhand think of a reason why '-' would be a _bad_ token for that, either. Random idea: today you can do git push origin master; # push branch master to remote origin git push --multiple origin korg; # push default refspec to 2 remotes How about: git push origin korg -- master; # push master to 2 remotes For this to be any useful, origin and korg has to have the same or at least similar ref structure, such that pushing my 'master' to their 'master' makes sense for both sites. I am not sure how common it would be. If people commonly do so, the above looks like a reasonably useful feature. git push -- master next; # push two refs to default remote ... or default push remote if there is one, I presume? As you are giving what to push, I am assuming that branch.$name.remote would not come into play in this case. git push origin -- master; # push master to origin, more explicitly git push origin korg --; # push default refspec to 2 remotes, again As you are _not_ saying what to push, I would expect branch.$name.remote may have to come into the picture, but because you are saying where to push, that is not the case. What does default refspec mean in this context? What git push origin (no refspecs) would push by default will be sent to origin, and what git push korg (no refspecs) would push by default will be sent to korg? All of the above sounds a bit too complicated to explain to end users, but I think they are internally consistent. git push host:some/path; # ambiguous argument. Please disambiguate. git push host:some/path --; # push default refspec over SSH git push -- host:some/path; # push specified refspec to default remote OK. git push origin; # is a remote name and not a refname. Good. git push master; # is a ref name and not a remote name. Good. Hmm, I dunno. What do you think? 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: [ITCH] Specify refspec without remote
On Tue, Apr 09, 2013 at 04:13:32PM -0700, Jonathan Nieder wrote: Random idea: today you can do git push origin master; # push branch master to remote origin git push --multiple origin korg; # push default refspec to 2 remotes Can we do git push --multiple today? My git does not seem to know about that (and I don't remember any patches in the area). Am I missing something? How about: git push origin korg -- master; # push master to 2 remotes git push -- master next; # push two refs to default remote git push origin -- master; # push master to origin, more explicitly git push origin korg --; # push default refspec to 2 remotes, again I like that _way_ better than the - proposal. Rather than introducing a magic token for the default ref, it fixes the actual syntax problem (that we have two lists of arguments to the command, and we do not know where one begins and the other ends). And it does it the same way as other parts of git. git push host:some/path; # ambiguous argument. Please disambiguate. This is a regression. I thought the point of this exercise was to leave that working. We could just as easily switch to: git push --remote=host:some/path if we are willing to break the existing syntax. Though your proposal does have the benefit of breaking only one particular syntax which is (I'm guessing) less frequently used. But we'd still need the usual deprecation period, I think. git push host:some/path --; # push default refspec over SSH git push -- host:some/path; # push specified refspec to default remote I like this. git push origin; # is a remote name and not a refname. Good. git push master; # is a ref name and not a remote name. Good. And this is sensible, too. -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: [ITCH] Specify refspec without remote
On Tue, Apr 09, 2013 at 06:19:01PM -0700, Junio C Hamano wrote: git push -- master next; # push two refs to default remote ... or default push remote if there is one, I presume? As you are giving what to push, I am assuming that branch.$name.remote would not come into play in this case. I would have assumed the opposite. We feed push two items: where to push (dst), and what refspecs to push (refs). We may or may not have each item. Here's what's possible today: dst=present, refs=present: (case 1) push to $dst, using $refs dst=present, refs=missing: (case 2) push to $dst, using refspecs from remote.$dst.push or push.default dst=missing, refs=missing: (case 3) push to remote.pushDefault, branch.*.remote, or origin; use refspecs from remote.$x.push or push.default, where $x is the remote we decide to use. The missing case 4 is obviously: dst=missing, refs=present And I would expect it to select the remote in the same way as case 3. In other words, the where and the what are orthogonal, and the presence or absence of one does not affect how the other is calculated (with the exception that _if_ we have a configured remote, whether it was specified by the user or calculated, we may use its config if no refspecs are specified). Do you want to explain your thinking? I'm guessing it has to do with the fact that choosing branch.*.remote is about trying to push to the configured upstream (even though we traditionally do _not_ take into account branch.*.merge when doing so). git push origin korg --; # push default refspec to 2 remotes, again As you are _not_ saying what to push, I would expect branch.$name.remote may have to come into the picture, but because you are saying where to push, that is not the case. What does default refspec mean in this context? What git push origin (no refspecs) would push by default will be sent to origin, and what git push korg (no refspecs) would push by default will be sent to korg? Yeah, I would expect that each uses its own default refspec. That is, the above command is exactly equivalent to: git push origin git push korg (possibly without the short-circuit behavior of , but definitely taking both into account in the exit code). I'd also be fine if we punted on the multiple remotes thing for now. You can accomplish it easily in the shell, as I showed above (and it is not any less efficient, since we have to make two network connections anyway). Jonathan's syntax allows for 0 to N remotes alongside 0 to N refspecs. The interesting new case (to me, anyway) is the 0 remotes, 0 refspecs case. But we don't have to handle 1 remotes now; once we have the syntax in place, we can make it work later when we've decided on the semantics. -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 v4] http-backend: respect GIT_NAMESPACE with dumb clients
On Tue, Apr 09, 2013 at 05:55:08PM -0700, John Koleszar wrote: Filter the list of refs returned via the dumb HTTP protocol according to the active namespace, consistent with other clients of the upload-pack service. Thanks, this version looks good to me. Updates to generate HEAD. Drops my original tests, since they were under the flawed assumption that both the dumb and smart protocols produced the same ref advertisement at /info/refs. Yeah, your new tests look good, and I think exercise the feature well. -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: segfault in git-remote-http
On Tue, Apr 09, 2013 at 12:40:44PM -0700, rh wrote: does not support hardlinks or symlinks). But I'm not sure which error you are talking about. We can figure out inside the program which program was invoked by checking argv, but I do not see us printing remote-http anywhere. I wasn't clear in my initial reportand may have omitted a significant fact. The git clone returned right away and I saw no error. The error shows up in dmesg via syslog, something like git-remote-http[1234]: segfault at blah blah in libcrypto That message is not generated by git, but rather by the kernel, which pulls the name from argv[0], presumably. E.g., try: echo 'int main() { *(int *)0=0; }' foo.c gcc foo.c ln a.out alternate ./a.out; ./alternate dmesg | tail -n 2 which should show both program names. Git invokes git-remote-* based on the URL you fed it. So if you are seeing a segfault in git-remote-http, presumably you fed it an http URL (which may still execute SSL code if it redirects to an https URL). -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
regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon
Greetings, I use git-daemon as the keeper of all source (love it). git is a normal user, running as git:daemon, with all repositories living in ~git. git-daemon is started like so: /usr/lib/git/git-daemon --syslog --detach --reuseaddr --user=git --group=daemon --pid-file=/var/run/git-daemon.pid --export-all --user-path --enable=receive-pack Try to pull as root or normal user results in: [pid 26786] access(/root/.config/git/config, R_OK) = -1 EACCES (Permission denied) [pid 26786] write(2, fatal: unable to access '/root/, 70) = 70 [pid 26785] ... read resumed fatal: unable to access '/root/, 4096) = 70 [pid 26786] exit_group(128) Bisection fingered this commit, though it looks like it's really due to not forgetting who it was at birth. It's not root, so has no business rummaging around in /root. It used to not care, but this commit made go away while looking for non-existent config file terminal. -Mike -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html