* Re: [PATCH v5 2/5] gitweb: add project_filter to limit project list to a subdirectory
From: Junio C Hamano @ 2012-01-30 21:05 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Bernhard R. Link, git
In-Reply-To: <201201302148.03909.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
>> - my @list = git_get_projects_list((my $filter = $project) =~ s/\.git$//);
>> + my ($filter = $project) =~ s/\.git$//;
>
> This doesn't work: it is syntax error:
>
> Can't declare scalar assignment in "my"
>
> It has to be either
>
> + (my $filter = $project) =~ s/\.git$//;
Sorry, that is what I meant.
^ permalink raw reply
* Re: [PATCH v5 2/5] gitweb: add project_filter to limit project list to a subdirectory
From: Junio C Hamano @ 2012-01-30 21:08 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Bernhard R. Link, git
In-Reply-To: <201201302148.03909.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> On Mon, 30 Jan 2012, Junio C Hamano wrote:
>> "Bernhard R. Link" <brl@mail.brlink.eu> writes:
>>
>> > I'll resend the series as replies to this mail.
>>
>> Thanks; I'll queue them in 'pu' for now (if Jakub wants to Ack the pieces,
>> I'll amend them).
>
> You can add Ack from me for the whole series.
Ok, amended and queued (but not pushed out yet).
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Phil Hord @ 2012-01-30 21:15 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <4EE7BEF5.6050205@web.de>
I lost my grip on this thread over the holidays...
On Tue, Dec 13, 2011 at 4:09 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 13.12.2011 15:17, schrieb Phil Hord:
>> On Mon, Dec 12, 2011 at 2:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> [...]
>>> Distro package dependency tracking is a poor analogy for many reasons, but
>>> I'll only touch a few.
>> [...]
>>> Naively, one might think that two branches, branch-1.0 and branch-2.0, can
>>> be defined in the repository of L, tell somebody (like "superproject that
>>> covers all the packages in the distro") that A wants branch-1.0 and B
>>> wants branch-2.0 of L respectively, to emulate this, but if one thinks
>>> further, one would realize that it is insufficient. For one thing, it is
>>> unclear what should happen when both A and B are asked to be checked out,
>>> but more importantly, in dependency requirements on real distro packaging,
>>> the application C could say "I want v1.0 API but v1.4 is broken and not
>>> compatible with me", which won't fit on the two-branches model. A
>>> workaround to add more branches to L could be devised but any workaround
>>> cannot be a good solution that allows a random application C among 47
>>> others to dictate how the branch structure of L project should look like.
>>>
>>> Fortunately, the dependency management is a solved problem by distro
>>> package management and build systems, and they do so without using
>>> anything from submodules. There is no point reinventing these logic in git
>>> submodules and emulating poorly.
>>>
>>> The only remotely plausible analogy around distro packaging would be a
>>> superproject full of all the packages in a distro as its submodules, and
>>> records exact versions of each and every package that goes on a release
>>> CD (or DVD). In that case, you do want to have a central registry that
>>> records what exact version of each package is used to cut the CD and the
>>> mother of all modules superproject could be one way to implement it. But
>>> that is not an example of floating, but is a direct opposite.
>>>
>>> This exchange convinced me further that anybody who wishes to use
>>> "floating" is better off either by doing one or both of the following:
>>>
>>> - using "exact" but not updating religiously, as the interdepency
>>> requirement in their project is not strict; or
>>>
>>> - not using submodules at all, but merely keeping these unrelated A, B, C
>>> and L as standalone repositories next to each other in the directory
>>> structure.
>>
>> My interdependency requirements are not so cut-and-dry. We use
>> submodules to isolate controlled regions of code. We may need to
>> share our project with a contractor who is allowed to see code
>> pertaining to "vendorA" but not that for "vendorB" or "VendorN". But
>> our in-house developers want to have all the vendor code in one place
>> for convenient integration. Submodules do this nicely for us. We can
>> give the contractor just the main modules and the VendorA modules and
>> he'll be fine. In-house devs get all the submodules (using the
>> vendor-ALL superproject).
>>
>> But this necessarily means there is too much coupling for comfort
>> between our submodules. For example, when an API changes in the main
>> submodule, each of the vendor submodules is affected because they each
>> implement that API in a custom method. Some of those vendor modules
>> belong to different people. Submodule synchronization becomes a real
>> chore.
>
> Hmm, maybe having vendor-specific branches in the superproject would
> help here. But that is hard to tell without knowing more details about
> your setup. But I suspect your vendor-ALL superproject is exactly the
> right spot to deal with these kind of problems (and if that isn't easy
> that might be a result of the difficulty of the problem you are trying
> to solve here, keeping different vendors in sync with your API ;-).
>
>> Floating would help, I think. Instead I do this:
>>
>> git pull origin topic_foo && git submodule foreach 'git pull origin topic_foo'
>>
>> git submodule foreach 'git push origin topic_foo' && git push origin topic_foo
>
> This sounds to me like you would need the "--recurse-submodules" option
> implemented for "git pull" and "git push", no?
Only if I have nested submodules, but yes, we do use --recurs* in our scripts.
> And I miss to see how
> floating would help when the tips of some submodules are not ready to
> work with other submodules tips ...
By project policy, for any branch, all submodules' tips of the
same-named branch should be interoperable. The CI server looks after
this, as much as he can.
I think of branch names as sticky notes (extra-lightweight tags,
sometimes). We have linear history in many of our vendor submodules,
but multiple "branches" indicate where each superproject branch has
presumably finished integration.
>> But not all my developers are git-gurus yet, and they sometimes mess
>> up their ad hoc scripts or miss important changes they forgot to push
>> in one submodule or another.
>
> Sure, even though current git should help you some by showing changes
> in the submodules.
Real newbies may not even remember to use 'git status' strategically.
>> Or worse, their pull or push fails and
>> they can't see the problem for all the noise. So they email it to me.
>
> We circumvent that by not pulling, but fetching and merging in the
> submodule first and after that in the superproject. You have much more
> control about what is going wrong where (and can have more
> git-experienced people help with - or even do - the merges).
I do that, too, and I wish I didn't have to. I wish I could safely
and sanely recover from a conflicted "git pull --recurse-submodules"
pull from the superproject. That is, I wish doing so were as
straightforward as recovering from the same condition would be if all
my code were in one repository instead of in submodules.
Which is the gist -- I wish submodules did not make git more
complicated than it already is.
Phil
^ permalink raw reply
* Re: [PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true
From: Ævar Arnfjörð Bjarmason @ 2012-01-30 21:18 UTC (permalink / raw)
To: Dan Johnson; +Cc: git, Eric Wong
In-Reply-To: <CAPBPrntdWAM056C_iZDD1XZy6KZ=5rKvH98Owgc-J8ZbBwrErg@mail.gmail.com>
On Mon, Jan 30, 2012 at 21:41, Dan Johnson <computerdruid@gmail.com> wrote:
> On Sat, Jan 28, 2012 at 9:11 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Change git-svn to parse --no-abbrev-commit --no-decorate to git-log
> Did you mean _pass_ --no-abbrev-commit here?
Yup.
I'l submit another patch fixing this and using rev-list.
^ permalink raw reply
* Re: i18n: Avoid sentence puzzles
From: Ævar Arnfjörð Bjarmason @ 2012-01-30 21:12 UTC (permalink / raw)
To: Frederik Schwarzer; +Cc: git
In-Reply-To: <201201301231.21090.schwarzerf@gmail.com>
On Mon, Jan 30, 2012 at 12:31, Frederik Schwarzer <schwarzerf@gmail.com> wrote:
> in order to enable translators to prepare proper translations,
> sentence puzzles have to be avoided. While it makes perfect sense for
> English, some languages may have to separate those words to sound or
> even be correct.
>
> The attached patch demonstrates a change to achive that. I did not
> test it because its purpose is only to raise awareness and start a
> discussion about this topic. After all the question is, how important
> translations are for a tool like Git. I have started a German
> translation but many things are really hard to translate.
I added the i18n support you're having problems with, and I completely
agree that this is the sort of thing we need to do.
Unfortunately when I added the i18n support I didn't have time to get
rid of all these sentence puzzles (or, to put it another way "lego"
sentences). You should never force translators to translate partial
sentences, you should always provide them with full sentences that
they can translate completely, even if that means that there's some
duplication for some languages.
Problem reports like this one are exactly what we need at this point
for i18n, we need people spotting these issues, and then we can fix
them.
^ permalink raw reply
* Re: [PATCH v2 3/4] completion: cleanup __gitcomp*
From: Junio C Hamano @ 2012-01-30 21:25 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jonathan Nieder, git, Felipe Contreras, Ted Pavlic,
SZEDER Gábor, Shawn O. Pearce
In-Reply-To: <7vpqe1au7f.fsf@alter.siamese.dyndns.org>
I managed to pick up 1 & 2:
1. be nicer with zsh (aka avoid default value assignment on : true
command);
2. simplify __git_remotes (aka use ls -1 instead of rolling a loop to do
that ourselves).
But I do not see your patch for 3 & 4 either on gmane archive nor in my
mailbox (via vger, not direct delivery from you to me). The threads for
these two patches both begin with Jonathan's review for me.
Care to resend 3 & 4?
Thanks.
^ permalink raw reply
* Re: [PATCH] merge: add instructions to the commit message when editing
From: Thomas Rast @ 2012-01-30 21:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git
In-Reply-To: <7vy5soaons.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@student.ethz.ch> writes:
>
>> The sentence about justification is one of the few things about
>> standard git that are not agnostic to the workflow that the user
>> chose.
>
> We try to be agnostic at plumbing level, but I do not think we ever made
> such a promise at the Porcelain level like "git merge". On the contrary,
> we try to encourage good workflows by coding behaviours to support BCP to
> Porcelain commands. Am I misreading what you were trying to say here?
Oh, I was just trying to preempt a possible argument why this is wrong.
Maybe I was a bit over-eager in doing so ;-)
>> +static const char merge_editor_comment[] =
>> +N_("Please enter the commit message for your merge commit. You should\n"
>> +"justify it especially if it merges an updated upstream into a topic\n"
>> +"branch.\n"
>> +"\n"
>> +"Lines starting with '#' will be ignored, and an empty message aborts\n"
>> +"the commit.\n");
>
> I am tempted to rewrite this a bit, perhaps something like ...
>
> Please enter the commit message for your merge commit. Explain
> why the merge is necessary, especially if it merges an updated
> upstream into a topic branch.
>
> ... because people who need to be told to "justify it" would probably be
> helped by a more explicit "explain _why_ it is needed".
Why not. The "explain..." might be construed as a bit too coercive, but
I cannot come up with a way to defuse it (well, except again tacking on
"you should") and yours is certainly much clearer.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Jeff King @ 2012-01-30 21:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git
In-Reply-To: <7v39axc9gp.fsf@alter.siamese.dyndns.org>
On Mon, Jan 30, 2012 at 10:48:54AM -0800, Junio C Hamano wrote:
> Note that I am not saying that we shouldn't add support for special cases
> with special case codepaths.
>
> Perhaps we would need to sprinkle more special case magic like this (this
> is for the special case that arises from the same cause)?
I like your patch better than trying to pass around "0{40}", but:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 7095718..0997e75 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -640,6 +640,13 @@ static int edit_branch_description(const char *branch_name)
> struct strbuf buf = STRBUF_INIT;
> struct strbuf name = STRBUF_INIT;
>
> + strbuf_addf(&name, "refs/heads/%s", branch_name);
> + if (!ref_exists(name.buf)) {
> + strbuf_reset(&name);
> + return error("No such branch '%s'.", branch_name);
> + }
> + strbuf_reset(&name);
> +
I wonder if this conditional should have:
unsigned char sha1[20];
const char *head_points_at = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (!head_points_at || strcmp(head_points_at, name.buf))
return error("No such branch '%s'.", branch_name);
to special-case unborn branches that we are actually pointing to.
IOW, the problem with the current code is that it allows typos and other
arbitrary bogus names to be silently described, even though doing so is
probably an error. But since this branch is already in use (even though
its ref does not technically exist yet), it's probably not an error.
As an aside, the strbuf_reset inside the conditional should be
strbuf_release, no? Otherwise we are leaking. And probably the one
outside, too. Even though we release the memory later, there are error
code-paths that do not. (And yes, I know this was a quick sketch and not
a real patch, but I wanted to point it out in case it turns into a real
one).
-Peff
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Jeff King @ 2012-01-30 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git
In-Reply-To: <7vaa55ar4v.fsf@alter.siamese.dyndns.org>
On Mon, Jan 30, 2012 at 12:10:08PM -0800, Junio C Hamano wrote:
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index f1984d9..195c40b 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -922,6 +922,19 @@ static int parse_branchname_arg(int argc, const char **argv,
> return argcount;
> }
>
> +static int switch_unborn_to_new_branch(struct checkout_opts *opts, const char *old_ref)
> +{
> + int status;
> + struct strbuf branch_ref = STRBUF_INIT;
> +
> + strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
> + warning(_("Leaving the unborn branch '%s' behind..."),
> + skip_prefix(old_ref, "refs/heads/"));
> + status = create_symref("HEAD", branch_ref.buf, "checkout -b");
> + strbuf_reset(&branch_ref);
> + return status;
> +}
Is it really worth warning? After all, by definition you are not leaving
any commits or useful work behind.
Also, this has the same strbuf reset/release leak as the last patch. :)
-Peff
^ permalink raw reply
* Re: [PATCH] merge: add instructions to the commit message when editing
From: Junio C Hamano @ 2012-01-30 21:52 UTC (permalink / raw)
To: Thomas Rast; +Cc: Thomas Rast, git
In-Reply-To: <878vkoamu3.fsf@thomas.inf.ethz.ch>
Thomas Rast <trast@inf.ethz.ch> writes:
>> Please enter the commit message for your merge commit. Explain
>> why the merge is necessary, especially if it merges an updated
>> upstream into a topic branch.
>>
>> ... because people who need to be told to "justify it" would probably be
>> helped by a more explicit "explain _why_ it is needed".
>
> Why not. The "explain..." might be construed as a bit too coercive, but
> I cannot come up with a way to defuse it (well, except again tacking on
> "you should") ...
Would "Please enter the commit message for your merge commit, to explain
why ..." work?
^ permalink raw reply
* Re: [PATCH] find_pack_entry(): do not keep packed_git pointer locally
From: Junio C Hamano @ 2012-01-30 23:26 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1327922750-12106-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Commit f7c22cc (always start looking up objects in the last used pack
> first - 2007-05-30) introduces a static packed_git* pointer as an
> optimization. The kept pointer however may become invalid if
> free_pack_by_name() happens to free that particular pack.
>
> Current code base does not access packs after calling
> free_pack_by_name() so it should not be a problem. Anyway, move the
> pointer out so that free_pack_by_name() can reset it to avoid running
> into troubles in future.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
Thanks. Two curiosities:
- Why is there no hunk to actually clear the pointer in
free_pack_by_name() in this patch?
- Could we make the magic (void *)1 value a #define'd constant? Perhaps
we could even use NULL for that purpose?
> diff --git a/sha1_file.c b/sha1_file.c
> index 88f2151..4ecc953 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -54,6 +54,8 @@ static struct cached_object empty_tree = {
> 0
> };
>
> +static struct packed_git *find_pack_entry_last_found = (void *)1;
> +
> static struct cached_object *find_cached_object(const unsigned char *sha1)
> {
> int i;
> @@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
> close_pack_index(p);
> free(p->bad_object_sha1);
> *pp = p->next;
> + if (find_pack_entry_last_found == p)
> + find_pack_entry_last_found = (void*)1;
> free(p);
> return;
> }
> @@ -2012,14 +2016,13 @@ int is_pack_valid(struct packed_git *p)
>
> static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> {
> - static struct packed_git *last_found = (void *)1;
> struct packed_git *p;
> off_t offset;
>
> prepare_packed_git();
> if (!packed_git)
> return 0;
> - p = (last_found == (void *)1) ? packed_git : last_found;
> + p = (find_pack_entry_last_found == (void *)1) ? packed_git : find_pack_entry_last_found;
>
> do {
> if (p->num_bad_objects) {
> @@ -2046,16 +2049,16 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> e->offset = offset;
> e->p = p;
> hashcpy(e->sha1, sha1);
> - last_found = p;
> + find_pack_entry_last_found = p;
> return 1;
> }
>
> next:
> - if (p == last_found)
> + if (p == find_pack_entry_last_found)
> p = packed_git;
> else
> p = p->next;
> - if (p == last_found)
> + if (p == find_pack_entry_last_found)
> p = p->next;
> } while (p);
> return 0;
^ permalink raw reply
* [BUG] git clean -X skips a directory containing only ignored files
From: Paul Berry @ 2012-01-30 23:36 UTC (permalink / raw)
To: git
I am trying to use "git clean -X" to remove object files (which
are gitignored) from my source tree, but it appears to miss
object files that are in a subdirectory without any git-tracked
files:
$ git init test
Initialized empty Git repository in /home/pberry/tmp/test/.git/
$ cd test
$ mkdir foo
$ touch foo/bar.o
$ echo '*.o' > .gitignore
$ git add .gitignore
$ git commit -mgitignore
[master (root-commit) 6b5ffcb] gitignore
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 .gitignore
$ git status
# On branch master
nothing to commit (working directory clean)
$ git clean -d -X -f
$ ls foo
bar.o
It seems to me that bar.o should have been removed, because
according to the git-clean docs, -X means "Remove only files
ignored by git", and bar.o is definitely being ignored by git.
It looks like a very similar bug was reported back in 2010, but
not fixed:
http://git.661346.n2.nabble.com/BUG-git-clean-X-behaviour-when-gitignore-has-sub-directory-entries-td5575307.html.
I've confirmed that the workaround mentioned by Jonathan Nieder
in that thread fixes my problem too (removing "dir.flags |=
DIR_SHOW_OTHER_DIRECTORIES;" from builtin/clean.c). However I'm
guessing from Jonathan's comments that it would be better to fix
this bug elsewhere (somewhere in dir.c perhaps).
Is anyone interested in following up on this old bug?
Alternatively, if someone could give me some guidance as to the
best way to go about fixing this bug, I would be glad to submit a
patch.
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH v2 3/4] completion: cleanup __gitcomp*
From: SZEDER Gábor @ 2012-01-31 0:15 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Felipe Contreras, git, Felipe Contreras, Ted Pavlic,
Shawn O. Pearce, Junio C Hamano
In-Reply-To: <20120130175004.GG10618@burratino>
Hi,
On Mon, Jan 30, 2012 at 11:50:04AM -0600, Jonathan Nieder wrote:
> Felipe Contreras wrote:
>
> > I don't know why there's so much code; these functions don't seem to be
> > doing much:
>
> Unless you mean "This patch has had inadequate review and I don't
> understand the code I'm patching, so do not trust it", please drop
> this commentary or place it after the three dashes.
>
> > * no need to check $#, ${3:-$cur} is much easier
> > * __gitcomp_nl doesn't seem to using the initial IFS
> >
> > This makes the code much simpler.
> >
> > Eventually it would be nice to wrap everything that touches compgen and
> > COMPREPLY in one function for the zsh wrapper.
> >
> > Comments by Jonathan Nieder.
>
> I don't want this acknowledgement. Who should care that I commented
> on something?
>
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 20 +++-----------------
> > 1 files changed, 3 insertions(+), 17 deletions(-)
>
> This diffstat tells me more of what I wanted to know about the patch
> than the description did.
>
> I imagine it would have been enough to say something along the lines of
> "The __gitcomp and __gitcomp_nl functions are unnecessarily verbose.
> __gitcomp_nl sets IFS to " \t\n" unnecessarily
Yeah, that's unnecessary. I'm not sure why I did that, perhaps just
blindly followed suit of gitcomp_1(), without realizing that I don't
do any word-splitting in __gitcomp_nl() except when invoking compgen.
> before setting it to "\n"
> by mistake.
But that is deliberate, that's why it's called __gitcomp_nl(), see
a31e6262 (completion: optimize refs completion, 2011-10-15), third
paragraph.
> Both functions use 'if' statements to read parameters
> with defaults, where the ${parameter:-default} idiom would be just as
> clear. By fixing these, we can make each function almost a one-liner."
>
> By the way, the subject ("clean up __gitcomp*") tells me almost as
> little as something like "fix __gitcomp*". A person reading the
> shortlog would like to know _how_ you are fixing it, or what the
> impact of the change will be --- e.g., something like "simplify
> __gitcomp and __gitcomp_nl" would be clearer.
>
> [...]
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> [...]
> > @@ -524,18 +520,8 @@ __gitcomp ()
> > # appended.
> > __gitcomp_nl ()
> > {
> > - local s=$'\n' IFS=' '$'\t'$'\n'
> > - local cur_="$cur" suffix=" "
> > -
> > - if [ $# -gt 2 ]; then
> > - cur_="$3"
> > - if [ $# -gt 3 ]; then
> > - suffix="$4"
> > - fi
> > - fi
> > -
> > - IFS=$s
> > - COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
> > + local IFS=$'\n'
> > + COMPREPLY=($(compgen -P "${2-}" -S "${4:- }" -W "$1" -- "${3:-$cur}"))
>
> This loses the nice name $suffix for the -S argument. Not a problem,
> just noticing.
I think loosing the name of $suffix would be OK, because the comment
above the function explains what the fourth parameter is about.
However, that comment also says that "If [the 4. argument is]
specified but empty, nothing is appended.", but this patch changes
this behavior, because "${4:- }" is substituted by a SP when $4 is an
empty string. You have to drop the colon and use "${4- }" there:
$ foo=""
$ echo ,${foo:- },
, ,
$ echo ,${foo- },
,,
Best,
Gábor
^ permalink raw reply
* [PATCH 0/2] gitweb: Project search improvements
From: Jakub Narebski @ 2012-01-31 0:20 UTC (permalink / raw)
To: git; +Cc: Bernhard R. Link, Jakub Narebski
This was once a part of larger series improving project search in
gitweb, but those two patches can stand alone. I am sending this
series (especially the second patch) in response to Bernhard patches
adding support for project_filter.
These patches should be applied on top of his work, i.e. on top of
'bl/gitweb-project-filter' branch.
Jakub Narebski (2):
gitweb: Improve projects search form
gitweb: Make project search respect project_filter
gitweb/gitweb.perl | 35 ++++++++++++++++++++++++++++++-----
gitweb/static/gitweb.css | 7 ++++++-
2 files changed, 36 insertions(+), 6 deletions(-)
--
1.7.6
^ permalink raw reply
* [PATCH 2/2] gitweb: Make project search respect project_filter
From: Jakub Narebski @ 2012-01-31 0:20 UTC (permalink / raw)
To: git; +Cc: Bernhard R. Link, Jakub Narebski
In-Reply-To: <1327969255-26622-1-git-send-email-jnareb@gmail.com>
Make gitweb search within filtered projects (i.e. projects shown), and
change "List all projects" to "List all projects in '$project_filter/'"
if project_filter is used.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new.
gitweb/gitweb.perl | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c4e0d8e..80975dd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5158,11 +5158,18 @@ sub git_patchset_body {
sub git_project_search_form {
my ($searchtext, $search_use_regexp);
+ my $limit = '';
+ if ($project_filter) {
+ $limit = " in '$project_filter/'";
+ }
+
print "<div class=\"projsearch\">\n";
print $cgi->startform(-method => 'get', -action => $my_uri) .
- $cgi->hidden(-name => 'a', -value => 'project_list') . "\n" .
- $cgi->textfield(-name => 's', -value => $searchtext,
- -title => 'Search project by name and description',
+ $cgi->hidden(-name => 'a', -value => 'project_list') . "\n";
+ print $cgi->hidden(-name => 'pf', -value => $project_filter). "\n"
+ if (defined $project_filter);
+ print $cgi->textfield(-name => 's', -value => $searchtext,
+ -title => "Search project by name and description$limit",
-size => 60) . "\n" .
"<span title=\"Extended regular expression\">" .
$cgi->checkbox(-name => 'sr', -value => 1, -label => 're',
@@ -5170,8 +5177,9 @@ sub git_project_search_form {
"</span>\n" .
$cgi->submit(-name => 'btnS', -value => 'Search') .
$cgi->end_form() . "\n" .
- $cgi->a({-href => href(project => undef, searchtext => undef)},
- 'List all projects') . "<br />\n";
+ $cgi->a({-href => href(project => undef, searchtext => undef,
+ project_filter => $project_filter)},
+ esc_html("List all projects$limit")) . "<br />\n";
print "</div>\n";
}
--
1.7.6
^ permalink raw reply related
* [PATCH 1/2] gitweb: Improve projects search form
From: Jakub Narebski @ 2012-01-31 0:20 UTC (permalink / raw)
To: git; +Cc: Bernhard R. Link, Jakub Narebski
In-Reply-To: <1327969255-26622-1-git-send-email-jnareb@gmail.com>
Refactor generating project search form into git_project_search_form().
Make text field wider and add on mouse over explanation (via "title"
attribute), add an option to use regular expressions, and replace
'Search:' label with [Search] button.
Also add "List all projects" link to make it easier to go back from
search result to list of all projects (note that empty search term
is disallowed).
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The major advantage is that you can use regular expression in
searching projects... well, at least there is UI for it, because you
could handcraft URL earlier anyway.
This patch was presented on git mailing list earlier.
gitweb/gitweb.perl | 27 ++++++++++++++++++++++-----
gitweb/static/gitweb.css | 7 ++++++-
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fa8a300..c4e0d8e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5155,6 +5155,26 @@ sub git_patchset_body {
# . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
+sub git_project_search_form {
+ my ($searchtext, $search_use_regexp);
+
+ print "<div class=\"projsearch\">\n";
+ print $cgi->startform(-method => 'get', -action => $my_uri) .
+ $cgi->hidden(-name => 'a', -value => 'project_list') . "\n" .
+ $cgi->textfield(-name => 's', -value => $searchtext,
+ -title => 'Search project by name and description',
+ -size => 60) . "\n" .
+ "<span title=\"Extended regular expression\">" .
+ $cgi->checkbox(-name => 'sr', -value => 1, -label => 're',
+ -checked => $search_use_regexp) .
+ "</span>\n" .
+ $cgi->submit(-name => 'btnS', -value => 'Search') .
+ $cgi->end_form() . "\n" .
+ $cgi->a({-href => href(project => undef, searchtext => undef)},
+ 'List all projects') . "<br />\n";
+ print "</div>\n";
+}
+
# fills project list info (age, description, owner, category, forks)
# for each project in the list, removing invalid projects from
# returned list
@@ -6022,11 +6042,8 @@ sub git_project_list {
insert_file($home_text);
print "</div>\n";
}
- print $cgi->startform(-method => "get") .
- "<p class=\"projsearch\">Search:\n" .
- $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
- "</p>" .
- $cgi->end_form() . "\n";
+
+ git_project_search_form($searchtext, $search_use_regexp);
git_project_list_body(\@list, $order);
git_footer_html();
}
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index c7827e8..c530355 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -520,8 +520,13 @@ div.search {
right: 12px
}
-p.projsearch {
+div.projsearch {
text-align: center;
+ margin: 20px 0px;
+}
+
+div.projsearch form {
+ margin-bottom: 2px;
}
td.linenr {
--
1.7.6
^ permalink raw reply related
* Re: [PATCH v2 3/4] completion: cleanup __gitcomp*
From: Jonathan Nieder @ 2012-01-31 0:25 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Felipe Contreras, git, Felipe Contreras, Ted Pavlic,
Shawn O. Pearce, Junio C Hamano
In-Reply-To: <20120131001535.GB2632@goldbirke>
SZEDER Gábor wrote:
> On Mon, Jan 30, 2012 at 11:50:04AM -0600, Jonathan Nieder wrote:
>> I imagine it would have been enough to say something along the lines of
>> "The __gitcomp and __gitcomp_nl functions are unnecessarily verbose.
>> __gitcomp_nl sets IFS to " \t\n" unnecessarily
>
> Yeah, that's unnecessary. I'm not sure why I did that, perhaps just
> blindly followed suit of gitcomp_1(), without realizing that I don't
> do any word-splitting in __gitcomp_nl() except when invoking compgen.
>
>> before setting it to "\n"
>> by mistake.
>
> But that is deliberate, that's why it's called __gitcomp_nl(), see
> a31e6262 (completion: optimize refs completion, 2011-10-15), third
> paragraph.
Yep, sorry for the ambiguity. I meant that setting IFS to " \t\n"
(before setting it to "\n") was not done for any serious reason.
The explanation is definitely clearer with "by mistake" dropped.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH] find_pack_entry(): do not keep packed_git pointer locally
From: Nguyen Thai Ngoc Duy @ 2012-01-31 2:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nicolas Pitre
In-Reply-To: <7v62fsai1i.fsf@alter.siamese.dyndns.org>
(Pulling Nico in for Q2 below. No snipping so he has a context)
2012/1/31 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Commit f7c22cc (always start looking up objects in the last used pack
>> first - 2007-05-30) introduces a static packed_git* pointer as an
>> optimization. The kept pointer however may become invalid if
>> free_pack_by_name() happens to free that particular pack.
>>
>> Current code base does not access packs after calling
>> free_pack_by_name() so it should not be a problem. Anyway, move the
>> pointer out so that free_pack_by_name() can reset it to avoid running
>> into troubles in future.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>
> Thanks. Two curiosities:
>
> - Why is there no hunk to actually clear the pointer in
> free_pack_by_name() in this patch?
I think it's there (the patch did work for me when I tried to
integrate repack to pack-objects).
-- 8<--
> @@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
> close_pack_index(p);
> free(p->bad_object_sha1);
> *pp = p->next;
> + if (find_pack_entry_last_found == p)
> + find_pack_entry_last_found = (void*)1;
> free(p);
> return;
> }
-- 8< --
> - Could we make the magic (void *)1 value a #define'd constant? Perhaps
> we could even use NULL for that purpose?
Q1. Sure.
Q2. No NULL is probably not suitable. I think Nico wanted to express
"we tried to find but found none (i.e. NULL)" too and 1 means "no we
have not tried".
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 88f2151..4ecc953 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -54,6 +54,8 @@ static struct cached_object empty_tree = {
>> 0
>> };
>>
>> +static struct packed_git *find_pack_entry_last_found = (void *)1;
>> +
>> static struct cached_object *find_cached_object(const unsigned char *sha1)
>> {
>> int i;
>> @@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
>> close_pack_index(p);
>> free(p->bad_object_sha1);
>> *pp = p->next;
>> + if (find_pack_entry_last_found == p)
>> + find_pack_entry_last_found = (void*)1;
>> free(p);
>> return;
>> }
>> @@ -2012,14 +2016,13 @@ int is_pack_valid(struct packed_git *p)
>>
>> static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
>> {
>> - static struct packed_git *last_found = (void *)1;
>> struct packed_git *p;
>> off_t offset;
>>
>> prepare_packed_git();
>> if (!packed_git)
>> return 0;
>> - p = (last_found == (void *)1) ? packed_git : last_found;
>> + p = (find_pack_entry_last_found == (void *)1) ? packed_git : find_pack_entry_last_found;
>>
>> do {
>> if (p->num_bad_objects) {
>> @@ -2046,16 +2049,16 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
>> e->offset = offset;
>> e->p = p;
>> hashcpy(e->sha1, sha1);
>> - last_found = p;
>> + find_pack_entry_last_found = p;
>> return 1;
>> }
>>
>> next:
>> - if (p == last_found)
>> + if (p == find_pack_entry_last_found)
>> p = packed_git;
>> else
>> p = p->next;
>> - if (p == last_found)
>> + if (p == find_pack_entry_last_found)
>> p = p->next;
>> } while (p);
>> return 0;
--
Duy
^ permalink raw reply
* Re: [PATCH] find_pack_entry(): do not keep packed_git pointer locally
From: Nicolas Pitre @ 2012-01-31 4:19 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8AS0nZOoXZZfz0OEwoWe88wp2aGR5NGqpG7xQmUvwi7TA@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1534 bytes --]
On Tue, 31 Jan 2012, Nguyen Thai Ngoc Duy wrote:
> (Pulling Nico in for Q2 below. No snipping so he has a context)
>
> 2012/1/31 Junio C Hamano <gitster@pobox.com>:
> > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> >
> >> Commit f7c22cc (always start looking up objects in the last used pack
> >> first - 2007-05-30) introduces a static packed_git* pointer as an
> >> optimization. The kept pointer however may become invalid if
> >> free_pack_by_name() happens to free that particular pack.
Hmmm, good point.
> >> Current code base does not access packs after calling
> >> free_pack_by_name() so it should not be a problem. Anyway, move the
> >> pointer out so that free_pack_by_name() can reset it to avoid running
> >> into troubles in future.
> >>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> >> ---
[...]
> > - Could we make the magic (void *)1 value a #define'd constant? Perhaps
> > we could even use NULL for that purpose?
>
> Q1. Sure.
Indeed. The idea might have been to use a non null value that cannot
match any pointer...
> Q2. No NULL is probably not suitable. I think Nico wanted to express
> "we tried to find but found none (i.e. NULL)" too and 1 means "no we
> have not tried".
Well, I could imagine I might have thought about something like that.
However, looking at the latest code in the master branch I can't see
any way for last_found to ever be assigned a NULL value. So if the
(void*)1 value might have been useful, it is certainly not anymore.
Nicolas
^ permalink raw reply
* Re: [PATCH] merge: add instructions to the commit message when editing
From: Thomas Rast @ 2012-01-31 7:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git
In-Reply-To: <7vhazcamdl.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@inf.ethz.ch> writes:
>
>>> Please enter the commit message for your merge commit. Explain
>>> why the merge is necessary, especially if it merges an updated
>>> upstream into a topic branch.
>>>
>>> ... because people who need to be told to "justify it" would probably be
>>> helped by a more explicit "explain _why_ it is needed".
>>
>> Why not. The "explain..." might be construed as a bit too coercive, but
>> I cannot come up with a way to defuse it (well, except again tacking on
>> "you should") ...
>
> Would "Please enter the commit message for your merge commit, to explain
> why ..." work?
Yes, though it winds up as a rather long sentence. I could then shorten
it to
Please enter a commit message that explains why the merge commit is
necessary, especially if it merges an updated upstream into a topic
branch.
though that's only _implying_ that you're completing a merge.
Your pick ;-)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: gitk Wish hangs on Mac OS X 10.6.8
From: Matti Linnanvuori @ 2012-01-31 7:49 UTC (permalink / raw)
To: git
In-Reply-To: <E423F584-55C0-4A27-B3DD-48B78EDE1C79@portalify.com>
Hi!
gitk Wish occasionally hangs on Mac OS X 10.6.8. I have to force kill it to make it disappear. git version 1.7.8.4, too, has this bug.
regards, Matti Linnanvuori
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Michael Haggerty @ 2012-01-31 8:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v39axc9gp.fsf@alter.siamese.dyndns.org>
On 01/30/2012 07:48 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> [3] If commit 0000000 were treated specially, then there would be no
>> unborn branches but only branches pointing at the empty commit. In that
>> case, my expectation would change--the old branch should be left
>> pointing at 0000000. But currently git has no concept of an unborn
>> branch that is not HEAD.
>
> And it probably is not a good thing to add such. Under that constraints,
> HEAD that says refs/heads/foo where foo does not exist yet needs to be
> special cased at places where it matters.
>
> For that matter, even if we artificially created refs/heads/foo before any
> commit is made and made it point at 0{40}, you would need to add special
> cases to other parts of the system
No, the idea is to avoid special casing by making 0{40} into a real (but
empty) revision.
> (e.g. "commit" needs to notice that the
> result should be a root, not a child of 0{40};
No, commits that were previously generated as orphans *would* now be
generated as children of the special 0{40} commit.
> "checkout other_branch"
> needs to notice that it should refrain from running the equivalent of
> "read-tree -m HEAD other_branch" because HEAD does not point at a real
> tree;
No, it would merge the 0{40} commit with other_branch like usual,
resulting in the same contents as other_branch. Indeed, if other_branch
is also ultimately a descendant of 0{40}, this would be like a
fast-forward merge.
> etc.
This "etc" might include problems.
> so it does not change the fact that the unborn branch is case
> is special.
On the contrary, I believe that much special casing could be eliminated
and the UI made more uniform by treating everything as a descendant of a
special "NULL" commit.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Johannes Sixt @ 2012-01-31 10:01 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <4F27ACED.2050709@alum.mit.edu>
Am 1/31/2012 9:57, schrieb Michael Haggerty:
> No, the idea is to avoid special casing by making 0{40} into a real (but
> empty) revision.
But then why not just have git init perform the equivalent of
c=$(echo "Start" | git commit-tree $empty_tree_sha1) &&
git update-ref refs/heads/master $c
People who dislike an empty initial commit can always use "git commit
--amend" for the first "real" commit.
-- Hannes
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Jakub Narebski @ 2012-01-31 10:09 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <4F27ACED.2050709@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 01/30/2012 07:48 PM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> [3] If commit 0000000 were treated specially, then there would be no
>>> unborn branches but only branches pointing at the empty commit. In that
>>> case, my expectation would change--the old branch should be left
>>> pointing at 0000000. But currently git has no concept of an unborn
>>> branch that is not HEAD.
>>
>> And it probably is not a good thing to add such. Under that constraints,
>> HEAD that says refs/heads/foo where foo does not exist yet needs to be
>> special cased at places where it matters.
>>
>> For that matter, even if we artificially created refs/heads/foo before any
>> commit is made and made it point at 0{40}, you would need to add special
>> cases to other parts of the system
>
> No, the idea is to avoid special casing by making 0{40} into a real (but
> empty) revision.
>
>> (e.g. "commit" needs to notice that the
>> result should be a root, not a child of 0{40};
>
> No, commits that were previously generated as orphans *would* now be
> generated as children of the special 0{40} commit.
You would still have to have quite a bit of special cases about 0{40}
NUL commit. Perhaps less special cases, but new special cases.
[...]
>> so it does not change the fact that the unborn branch is case
>> is special.
>
> On the contrary, I believe that much special casing could be eliminated
> and the UI made more uniform by treating everything as a descendant of a
> special "NULL" commit.
I don't see how this can be done in backward-compatibile way.
Please note that in Git it is quite natural to have more than one root
(parentless) commit, even without presence of disconnected / orphan
branches. They are result of joining originally separate projects.
git.git has quite a few of them (more than 6, IIRC).
--
Jakub Narebski
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: demerphq @ 2012-01-31 10:11 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Michael Haggerty, Junio C Hamano, git
In-Reply-To: <4F27BBED.9080902@viscovery.net>
On 31 January 2012 11:01, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 1/31/2012 9:57, schrieb Michael Haggerty:
>> No, the idea is to avoid special casing by making 0{40} into a real (but
>> empty) revision.
>
> But then why not just have git init perform the equivalent of
>
> c=$(echo "Start" | git commit-tree $empty_tree_sha1) &&
> git update-ref refs/heads/master $c
>
> People who dislike an empty initial commit can always use "git commit
> --amend" for the first "real" commit.
Because it would then violate a system invariant that all commits are
descendants of the root commit.
You can model a git commit graph as a pathway through multidimensional
space of all possible commit trees. From that perspective it makes
sense that every pathway starts at the origin point of the
multidimensional space, which is conceptually the same as the proposed
root commit.
Anyway, I am not saying it should change, just that from some point of
views it makes a lot of sense.
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox