* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Ethan Reesor @ 2013-02-11 7:18 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211071235.GL15329@elie.Belkin>
On Mon, Feb 11, 2013 at 2:12 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Isn't that a criticism of the git-shell-commands facility in general?
> If it is common to have a lot of users with distinct home directories
> but all with git-shell as their login shell, then the
> git-shell-commands should not go in their home directory to begin
> with, no?
I know nothing of the security issues, but why not have a
/etc/git-shell-commands?
--
Ethan Reesor (Gmail)
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 7:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7v8v6vpbej.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Isn't that a criticism of the git-shell-commands facility in general?
>> If it is common to have a lot of users with distinct home directories
>> but all with git-shell as their login shell, then the
>> git-shell-commands should not go in their home directory to begin
>> with, no?
>
> You can give one set of commands to some users while restricting
> others, no?
Yes, I assume one goal of the current design was to let you set up
multiple configurations by making multiple home directories.
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 7:22 UTC (permalink / raw)
To: Ethan Reesor
Cc: Jonathan Nieder, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <CAE_TNi=EG6vziVObJ-a__smeOv7RgZ5R146eonD6M828H7ziNQ@mail.gmail.com>
Ethan Reesor <firelizzard@gmail.com> writes:
>> For those who _do_ want to give customized commands to their users,
>> they can already have "help" script to give a friendly message. It
>> just felt silly to force sites to create the directory only to
>> refuse an access to the "custom commands" feature, especially when
>> the existence of that directory is a signal that the site may want
>> to give its users an acess to that feature.
>
> Again, would it not be more elegant and powerful to A) have the
> shell-disabled message/hook/etc specified by git-config on some level,
> be it /etc/gitconfig or ~/.gitconfig, and B) have Jonathan's patch
> whereby ~/git-shell-commands/help returning non-zero closes the
> connection?
Isn't that what I have essentially been saying?
For sites that do not want per-user customizable "other commands",
have a single site-wide hook instead of having to create otherwise
empty shell-commands directories for all users. For users a site
wants to allow customized commands, have the directory and custom
"help" message. I do not care too deeply if "help" exiting non-zero
caused the connection closed, but I care about not forcing a lot of
effort to customize messages to people who do *not* need
customizability.
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Ethan Reesor @ 2013-02-11 7:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7v4nhjpb69.fsf@alter.siamese.dyndns.org>
On Mon, Feb 11, 2013 at 2:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ethan Reesor <firelizzard@gmail.com> writes:
>> Again, would it not be more elegant and powerful to A) have the
>> shell-disabled message/hook/etc specified by git-config on some level,
>> be it /etc/gitconfig or ~/.gitconfig, and B) have Jonathan's patch
>> whereby ~/git-shell-commands/help returning non-zero closes the
>> connection?
>
> Isn't that what I have essentially been saying?
That is what you've been saying. I reiterated because I like the idea
of having it managed via git config.
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 7:28 UTC (permalink / raw)
To: Ethan Reesor
Cc: Jonathan Nieder, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <CAE_TNin+WcPodGfXKQuzBVujK7Yx3iCUR2rqgoc20WgwhJSR4g@mail.gmail.com>
Ethan Reesor <firelizzard@gmail.com> writes:
> On Mon, Feb 11, 2013 at 2:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ethan Reesor <firelizzard@gmail.com> writes:
>>> Again, would it not be more elegant and powerful to A) have the
>>> shell-disabled message/hook/etc specified by git-config on some level,
>>> be it /etc/gitconfig or ~/.gitconfig, and B) have Jonathan's patch
>>> whereby ~/git-shell-commands/help returning non-zero closes the
>>> connection?
>>
>> Isn't that what I have essentially been saying?
>
> That is what you've been saying. I reiterated because I like the idea
> of having it managed via git config.
Yes, and I've been ignoring the "git config". I do not think it
gives enough customizability Jonathan's example of listing user
owned repositories, for example. Having a config variable in
/etc/gitconfig that points at a random script on the filesystem does
not buy us much over an approach to have a global hook at a known
place on the filesystem, no?
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 7:44 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211072154.GN15329@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> Isn't that a criticism of the git-shell-commands facility in general?
>>> If it is common to have a lot of users with distinct home directories
>>> but all with git-shell as their login shell, then the
>>> git-shell-commands should not go in their home directory to begin
>>> with, no?
>>
>> You can give one set of commands to some users while restricting
>> others, no?
>
> Yes, I assume one goal of the current design was to let you set up
> multiple configurations by making multiple home directories.
Even if the site configures its 47 git-shell users to share the same
home directory /home/gituser, I still think it is a bad design to
force the administrator to create a directory in it, only to add a
script called "help".
The purpose of the directory is to keep custom commands that are
allowed. If the site administrator does not want any command, it
would be more natural to expect that the way to disable them would
be _not_ to have that directory which is a collection of allowed
commands. Adding that directory and add a "help" that exits with
non-zero feels quite a roundabout and counter-intuitive way, no?
^ permalink raw reply
* Re: Pushing a git repository to a new server
From: Konstantin Khomoutov @ 2013-02-11 7:50 UTC (permalink / raw)
To: Ethan Reesor; +Cc: git
In-Reply-To: <CAE_TNin0Kb_38gnx9W36VZ8CTxYBZ9T1Dkhar1DUFHyQUq7ebg@mail.gmail.com>
On Sun, Feb 10, 2013 at 04:00:56PM -0500, Ethan Reesor wrote:
> I'm looking to make a command to push a git repo to a new server. The
> way I just did it is as follows:
>
> localhost> git clone --bare /path/to/MyRepo /path/to/tmpdir/MyRepo.git
> localhost> tar xz /path/to/tmpdir/MyRepo.git | ssh myuser@remotehost
> tar cz \~/ # If I don't escape '~', my local machine expands it
> localhost> ssh myuser@remotehost
> remotehost> sudo chown -R git:git MyRepo.git
What's wrong with
$ ssh myuser@remotehost 'mkdir /path/to/MyRepo.git; cd $_; git init --bare'
$ git push --all git@remotehost:MyOtherRepo.git
?
> The reason I had to use my user is the git user's shell is git-prompt
There's no such thing as git-prompt. The restricted login shell for
SSH-only access typically used for such a "virtual" Git user is
git-shell.
> and ~git/git-shell-commands is empty. I have repos set up using
> 'git@remotehost:MyOtherRepo.git' as the remote and everything works.
>
> How do I make a git command that can talk to the server using
> git-prompt like the other commands do?
It's not really clear what do you want to achieve.
The reason the git-shell shell is *restricted* (read its manual page)
is to shrink the surface of possible attacks in the case the shell
account used for accessing Git repos over SSH is compromized (the key or
password stolen, for instance). This is achieved by only allowing
commands like git-upload-pack etc in the shell (no general file
manipulation commands etc). So what creating "git command that can
talk to the server using git-prompt ..." would really buy you?
I think the way to go is to start using gitolite [1] or implement by
hand a subset of what it does (a custom login shell which is allowed to
do certain things in a special area of the filesystem designated to keep
Git repositories) or just set up a special account on the server
("git-admin", for instance) which would have a regular login shell set
for it and would be in the same group as the user "git" (or even have
the same UID) so that they could share the files they create (subject to
active umasks of processes run as both users though).
1. https://github.com/sitaramc/gitolite
^ permalink raw reply
* Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command
From: Jonathan Nieder @ 2013-02-11 7:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Sitaram Chamarty, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7vd2w7pbh5.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> +To disable interactive logins, displaying a greeting instead:
>> ++
>> +----------------
>> +$ chsh -s /usr/bin/git-shell
>> +$ mkdir $HOME/git-shell-commands
>> +$ cat >$HOME/git-shell-commands/help <<\EOF
>> +#!/bin/sh
>> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
>
> Where in the sshd to git-shell exec chain is $USER variable set for
> the user? Just being curious if this is the simplest but one of the
> more robust ways to get the user's name.
That's a good question. environment= in an authorized_keys file is
obsolete, so USER generally represents the actual logged in user.
That means the main way to base behavior on private key (letting one
system user represent multiple people) is a gitolite-style command=
wrapper that checks SSH_ORIGINAL_COMMAND. In that setup, there is no
reason to forward simple no-args "are you there?" requests to the
git-shell, so we can forget about it here.
So by the time we get to git-shell, most likely either
A) this is a generic system user, with a username like "git", and the
above example would insult the client with "Hi git!" or "Hi
project-x-git!"
or
B) each person has a separate account on the system, perhaps to help
the admin to set filesystem permissions based on users and groups,
and the above would address the user by her normal name.
Jonathan
^ permalink raw reply
* Re: Pushing a git repository to a new server
From: Ethan Reesor @ 2013-02-11 7:57 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git
In-Reply-To: <20130211075040.GJ5210@localhost.localdomain>
On Mon, Feb 11, 2013 at 2:50 AM, Konstantin Khomoutov
<kostix+git@007spb.ru> wrote:
> What's wrong with
> $ ssh myuser@remotehost 'mkdir /path/to/MyRepo.git; cd $_; git init --bare'
> $ git push --all git@remotehost:MyOtherRepo.git
> ?
Nothing, I just wanted to make myself a command to do that for me.
>> The reason I had to use my user is the git user's shell is git-prompt
>
> There's no such thing as git-prompt. The restricted login shell for
> SSH-only access typically used for such a "virtual" Git user is
> git-shell.
Sorry, git-prompt is something I made for myself. I meant git-shell.
> It's not really clear what do you want to achieve.
> The reason the git-shell shell is *restricted* (read its manual page)
> is to shrink the surface of possible attacks in the case the shell
> account used for accessing Git repos over SSH is compromized (the key or
> password stolen, for instance). This is achieved by only allowing
> commands like git-upload-pack etc in the shell (no general file
> manipulation commands etc). So what creating "git command that can
> talk to the server using git-prompt ..." would really buy you?
I want to create a git-command that 1) creates a bare version of the
current repo, 2) and uploads it to the specified path on my server
(using tar, but that's not the point).
My problem is that I have no idea how things like git-push works via a
user with git-shell. Can you only run certain git commands, like
git-upload-pack? Because I tried running 'ssh git@server git status'
and that failed.
> I think the way to go is to start using gitolite [1] or implement by
> hand a subset of what it does (a custom login shell which is allowed to
> do certain things in a special area of the filesystem designated to keep
> Git repositories) or just set up a special account on the server
> ("git-admin", for instance) which would have a regular login shell set
> for it and would be in the same group as the user "git" (or even have
> the same UID) so that they could share the files they create (subject to
> active umasks of processes run as both users though).
I thought about the secondary user idea. I decided that trying to make
my own command would be more fun.
--
Ethan Reesor (Gmail)
^ permalink raw reply
* Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect
From: Michael J Gruber @ 2013-02-11 8:04 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Andreas Mohr
In-Reply-To: <20130210014900.GA7682@elie.Belkin>
Jonathan Nieder venit, vidit, dixit 10.02.2013 02:49:
> Hi,
>
> Michael J Gruber wrote:
>
>> "reset" can be easily misunderstood as resetting a bisect session to its
>> start without finishing it. Clarify that it actually finishes the bisect
>> session.
>
> FWIW,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Addressing Andreas's original concern about the discoverability of
> 'git bisect reset' would presumably require doing two more things:
>
> 1. adding an example of the normal bisection workflow to the EXAMPLES
> section
>
> 2. training users to look to the EXAMPLES section
>
> That is, something like the below. But I'm not happy with it, because
> it just runs over the same material as the current Description
> section. Maybe the current tutorial material could be moved to
> examples and replaced with something terser that fleshes out the
> descriptions in "git bisect -h" output. What do you think?
>
> diff --git i/Documentation/git-bisect.txt w/Documentation/git-bisect.txt
> index e4f46bc1..b89abd78 100644
> --- i/Documentation/git-bisect.txt
> +++ w/Documentation/git-bisect.txt
> @@ -356,6 +356,54 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
> This shows that you can do without a run script if you write the test
> on a single line.
>
> +* Bisect to find which patch caused a boot failure:
> ++
> +Install a recent kernel:
> ++
> +------------
> +$ cd ~/src/linux
> +$ git checkout origin/master
> +$ make deb-pkg # or binrpm-pkg, or tar-pkg
> +$ dpkg -i ../<name of package> # as root (or rpm -i, or tar -C / -xf)
> +$ reboot # as root
> +------------
> ++
> +Hopefully it fails to boot, so tell git so and begin bisection:
> ++
> +------------
> +$ cd ~/src/linux
> +$ git bisect start HEAD v3.2 # assuming 3.2 works fine
> +-------------
> ++
> +A candidate revision to test is automatically checked out.
> +Test it:
> ++
> +-------------
> +$ make deb-pkg # or binrpm-pkg, or tar-pkg
> +$ dpkg -i ../<name of package> # as root (or rpm -i, or tar -C / -xf)
> +$ reboot # as root
> +-------------
> ++
> +Record the result:
> ++
> +-------------
> +$ cd ~/src/linux
> +$ git bisect good # if it booted correctly
> +$ git bisect bad # if it failed to boot
> +$ git bisect skip # if some other bug made it hard to test
> +-------------
> ++
> +Repeat until bored or git prints the "first bad commit". When
> +done:
> ++
> +-------------
> +$ git bisect log >log # let others pick up where you left off
> +$ git bisect reset HEAD # exit the bisecting state
> +-------------
> ++
> +At any step, you can run `git bisect visualize` to watch the
> +regression range narrowing.
> +
> * Locate a good region of the object graph in a damaged repository
> +
> ------------
>
[BTW, sorry for failing to set --in-reply-to in the patch e-mail. Need
to get that automated somehow.]
I did not use "stop" for the exact reasons that Junio mentioned. Just
throw in a third alternative: "quit" may convey that it's not possible
to ressume, without sounding as "exceptional" as "abort" does. After
all, it's the normal end to a bisect session much unlike "--abort" for
rebase, for example.
As for the example, we have an example section, and we could simply
throw in "git reset" lines there. I would even amend my patch with that;
the great git-bisect.txt refactoring I'd definitely leave to someone else.
Michael
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 8:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7vvc9znvk6.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> The purpose of the directory is to keep custom commands that are
> allowed. If the site administrator does not want any command, it
> would be more natural to expect that the way to disable them would
> be _not_ to have that directory which is a collection of allowed
> commands. Adding that directory and add a "help" that exits with
> non-zero feels quite a roundabout and counter-intuitive way, no?
I think it comes down to the reason the site admin doesn't want to
allow interactive logins. That reason seems to be mostly that
presenting a
git>
prompt at which you can only ask for "help" or "exit" is a bit
confusing and pointless. I have sympathy for that, which is why I
looked for a way for the admin to ask to avoid the prompt altogether
in that case.
I do not think the reason is "because I don't want a
git-shell-commands directory". I think it's good to have basically
one kind of setup instead of significantly different ones with and
without that special directory --- and it means that starting from a
setup like this, one can easily drop in additional commands like
set-head or create-repo without changing anything basic. It's making
the admin's later life easier.
Maybe a better test than "help exits with special exit code" is "there
are no other custom commands than help". Would that be more sensible?
>From a "make it possible to emulate gitolite" point of view, that
doesn't permit disabling the interactive mode when there are other
commands available, so my hunch is that it wouldn't.
Jonathan
^ permalink raw reply
* [PATCHv2] git-bisect.txt: clarify that reset quits bisect
From: Michael J Gruber @ 2013-02-11 8:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Andreas Mohr, Jonathan Nieder
In-Reply-To: <5118A5F2.6000007@drmicha.warpmail.net>
"reset" can be easily misunderstood as resetting a bisect session to its
start without finishing it. Clarify that it actually quits the bisect
session.
Reported-by: Andreas Mohr <andi@lisas.de>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Documentation/git-bisect.txt | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index b4831bb..f986c5c 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -83,7 +83,7 @@ Bisect reset
~~~~~~~~~~~~
After a bisect session, to clean up the bisection state and return to
-the original HEAD, issue the following command:
+the original HEAD (i.e., to quit bisecting), issue the following command:
------------------------------------------------
$ git bisect reset
@@ -284,6 +284,7 @@ EXAMPLES
------------
$ git bisect start HEAD v1.2 -- # HEAD is bad, v1.2 is good
$ git bisect run make # "make" builds the app
+$ git bisect reset # quit the bisect session
------------
* Automatically bisect a test failure between origin and HEAD:
@@ -291,6 +292,7 @@ $ git bisect run make # "make" builds the app
------------
$ git bisect start HEAD origin -- # HEAD is bad, origin is good
$ git bisect run make test # "make test" builds and tests
+$ git bisect reset # quit the bisect session
------------
* Automatically bisect a broken test case:
@@ -302,6 +304,7 @@ make || exit 125 # this skips broken builds
~/check_test_case.sh # does the test case pass?
$ git bisect start HEAD HEAD~10 -- # culprit is among the last 10
$ git bisect run ~/test.sh
+$ git bisect reset # quit the bisect session
------------
+
Here we use a "test.sh" custom script. In this script, if "make"
@@ -351,6 +354,7 @@ use `git cherry-pick` instead of `git merge`.)
------------
$ git bisect start HEAD HEAD~10 -- # culprit is among the last 10
$ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
+$ git bisect reset # quit the bisect session
------------
+
This shows that you can do without a run script if you write the test
@@ -368,6 +372,7 @@ $ git bisect run sh -c '
rm -f tmp.$$
test $rc = 0'
+$ git bisect reset # quit the bisect session
------------
+
In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit that
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* Re: [PATCHv2] git-bisect.txt: clarify that reset quits bisect
From: Jonathan Nieder @ 2013-02-11 8:39 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano, Andreas Mohr
In-Reply-To: <d679f620f10ad7afc044c9f5dadcf433b388ad10.1360571552.git.git@drmicha.warpmail.net>
Michael J Gruber wrote:
> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
[...]
> @@ -284,6 +284,7 @@ EXAMPLES
> ------------
> $ git bisect start HEAD v1.2 -- # HEAD is bad, v1.2 is good
> $ git bisect run make # "make" builds the app
> +$ git bisect reset # quit the bisect session
I had forgotten that "git bisect run" ends in a bisecting state.
Good call.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v3 11/11] Unify appending signoff in format-patch, commit and sequencer
From: Jonathan Nieder @ 2013-02-11 9:00 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pclouds, gitster, Brandon Casey
In-Reply-To: <CA+sFfMduqEJPtDQyTe2n7QiLmDpTN7MzLGrUNf1hWf-h0rGzvA@mail.gmail.com>
Brandon Casey wrote:
> We want to be able to support lines that do
> not have email addresses on the right-hand side like:
>
> Bug: XXX
> Change-Id: XXX
Good call.
By the way, regarding what the right "--signoff" behavior is for
commit, cherry-pick, am, and format-patch:
I think the best behavior would be to check if the last signed-off-by
line (ignoring acked-by, bug, change-id, and so on lines that follow
it) matches the one to be added, and if it doesn't, add a new
sign-off. That way, the sign-off list still would accurately describe
the path of the patch, without silliness like
Signed-off-by: me
Reviewed-by: someone
Signed-off-by: me
that you mentioned.
I agree that that's orthogonal to this series and just mostly
preserving behavior (as you already do) is the right thing to do.
Thanks for noticing the edge cases.
Jonathan
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #04; Sat, 9)
From: Matthieu Moy @ 2013-02-11 9:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtxplt5u2.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> * mm/allow-contrib-build (2013-02-07) 2 commits
> - perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other directories
> - Makefile: extract perl-related rules to make them available from other dirs
>
> Will merge to 'next'.
These two patches do not make much sense without the 3rd one:
[PATCH 3/4] Makefile: factor common configuration in git-default-config.mak
because perl.mak uses $(pathsep) that is defined in Makefile, hence
unreachable from external callers.
You can either drop the series (I still think it's a good thing to make
the toplevel Makefile more modular, but I have no longer a personal
interest in it since the original goal is already reached by
mm/remote-mediawiki-build), or continue the discussion on [PATCH 3/4].
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] fixup! graph: output padding for merge subsequent parents
From: John Keeping @ 2013-02-11 10:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matthieu Moy, Michał Kiedrowicz
In-Reply-To: <7vwqufrdzd.fsf@alter.siamese.dyndns.org>
On Sun, Feb 10, 2013 at 02:38:46PM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> > On Sun, Feb 10, 2013 at 11:30:39AM -0800, Junio C Hamano wrote:
> > ...
> >> Is it correct to say that this essentially re-does 656197ad3805
> >> (graph.c: infinite loop in git whatchanged --graph -m, 2009-07-25)
> >> in a slightly different way, in that Michał's original fix also
> >> protected against the case where graph->state is flipped to
> >> GRAPH_PADDING by graph_next_line() that returns false, but with your
> >> fixup, the code knows it never happens (i.e. when graph_next_line()
> >> returns false, graph->state is always in the GRAPH_PADDING state),
> >> and the only thing we need to be careful about is when graph->state
> >> is already in the PADDING state upon entry to this function?
> >
> > Yes, although I wonder if we can end up in POST_MERGE or COLLAPSING
> > state here as well. The check in the loop guards against that because
> > those will eventually end up as PADDING.
> >
> > As far as I can see, this is okay because we have called
> > graph_show_remainder() at the end of outputting a commit, even when we
> > end up outputting the same (merge) commit more than once. But someone
> > more familiar with the graph code might want to comment here.
>
> More importantly, that kind of thought process needs to be
> documented in the log message; that will help people to diagnose the
> cause of the problem if they later find that this patch made an
> incorrect assumption while simplifying the code.
Perhaps it's best to leave the patch as it originally was to guarantee
that we can't get stuck in graph_show_commit(), even when it's called at
an unexpected time, but I see you've already squashed this change in.
Would you prefer me to resend the original patch or send an update with
this change and the above reasoning in the commit message?
John
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Duy Nguyen @ 2013-02-11 11:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramkumar Ramachandra, Robert Zeh, Git List, finnag
In-Reply-To: <CACsJy8DnvAjQPL4aP_LRC7aqx6OC4M5dMtj-OUot76qET2z08Q@mail.gmail.com>
On Mon, Feb 11, 2013 at 9:56 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> Yeah, it did not cut out syscall cost, I also cut a lot of user-space
> processing (plus .gitignore content access). From the timings I posted
> earlier,
>
>> unmodified dir.c
>> real 0m0.550s 0m0.287s
>> user 0m0.305s 0m0.201s
>> sys 0m0.240s 0m0.084s
>
> sys time is reduced from 0.24s to 0.08s, so readdir+opendir definitely
> has something to do with it (and perhaps reading .gitignore). But it
> also reduces user time from 0.305 to 0.201s. I don't think avoiding
> readdir+openddir will bring us this gain. It's probably the cost of
> matching .gitignore. I'll try to replace opendir+readdir with a
> no-syscall version. At this point "untracked caching" sounds more
> feasible (and less complex) than ".gitignore cachine".
And this is read_directory's timing breakdown (again, "git status" on
gentoo-x86,git, built with -O2 on x86-64 if I did not mention before)
opendir = 0.030s
readdir = 0.083s
closedir = 0.020s
{open,read,close}dir = 0.132s
treat_path = 0.094s (172534 times)
dir_add_name = 0.050s (101917 times)
read_directory = 0.292s
# On branch master
nothing to commit, working directory clean
real 0m0.511s
user 0m0.347s
sys 0m0.157s
Instrumentation is done with gettimeofday. Without gettimeofday calls
inside read_directory_recursive, read_directory takes 0.267s (iow,
gettimeofday cost is about 0.30s). {open,read,close}dir + treat_path +
dir_add_name + gettimeofday add up quite close to 0.292s (strbuf_*
takes just about 0.005s)
Eliminating xxxdir syscalls may save us 0.132s (or less, we need to
pay to get the information elsewhere).
Because my worktree is clean, dir_add_name spends all 0.05s in
cache_name_exists(). If we somehow know the input path is not a
tracked entry, we could avoid cache_name_exists() and save 0.05s.
If we do the "untracked cache", the number of treat_path calls should
be much lower. In this particular case of gentoo-x86, I'd expect no
more than a dozen of untracked files, which cuts down treat_path and
dir_add_name's time to near zero. On a normal repository like git.git,
untracked files are about 1075 files with 2552 tracked files, we
should be able to save 2/3 to 1/2 of treat_path calls.
--
Duy
^ permalink raw reply
* Re: Pushing a git repository to a new server
From: Konstantin Khomoutov @ 2013-02-11 12:45 UTC (permalink / raw)
To: Ethan Reesor; +Cc: Konstantin Khomoutov, git
In-Reply-To: <CAE_TNin6-weutRDToZ7-BBGJTCcf0dwJn0ChUbFcACRU=SbjzA@mail.gmail.com>
On Mon, 11 Feb 2013 02:57:51 -0500
Ethan Reesor <firelizzard@gmail.com> wrote:
[...]
> I want to create a git-command that 1) creates a bare version of the
> current repo, 2) and uploads it to the specified path on my server
> (using tar, but that's not the point).
Thanks, it's now a bit more clear.
> My problem is that I have no idea how things like git-push works via a
> user with git-shell. Can you only run certain git commands, like
> git-upload-pack?
[...]
Precisely so. With additional twist that you can create (or link)
other commands under ~/git-shell-commands, and these will be available
as well.
OK, here's the sketch.
On the server, in the home directory of your "git" user, you create a
wrapper around git-receive-pack, like this:
# mkdir ~git/git-shell-commands
# cat >~git/git-shell-commands/git-receive-new-repo
#!/bin/sh
set -e -u
if [ $# -ne 1 ]; then
echo 'Missing required argument: <directory>' >&2
exit 1
fi
mkdir "$1" && git init --quiet --bare "$1" && git-receive-pack "$1"
^D
# chmod +x $_
Then, on the client side, to push a new repo, you just do
$ git push --receive-pack=git-receive-new-repo --all git@server:repo.git
This will make `git push` to spawn not just `git receive-pack <dir>` as
it usually does but your wrapper, which would first create and
initialize a bare repository and then spawn `git receive-pack` on it
which would then communicate with the client side and receive
everything from it.
You could then create a client-side wrapper script or a Git alias for
such "creative pushing", like this:
$ git config --add --global alias.push-new-repo \
'push --receive-pack=git-receive-new-repo --all'
So the whole client call is now reduced to
$ git push-new-repo git@server:repo.git
^ permalink raw reply
* [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Erik Faye-Lund @ 2013-02-11 13:34 UTC (permalink / raw)
To: git; +Cc: Erik Faye-Lund
Even though parse-options doesn't support UTF-8 switches (which
makes sense; non-ascii switches would be difficult to enter on
some keyboard layouts), it can be useful to report incorrectly
entered UTF-8 switches to make the output somewhat less ugly
for those of us with keyboard layouts with UTF-8 characters on
it.
Make the reporting code grok UTF-8 in the option sequence, and
write a variable-width output sequence.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
As being both clumsy and Norwegian, I some times to enter the
Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the
correct ones when entering command-line options.
However, since git only looks at one byte at the time for
short-options, it ends up reporting a partial UTF-8 sequence
in such cases, leading to corruption of the output.
The "real fix" would probably be to add proper multi-byte
support to the short-option parser, but this serves little
purpose in Git; we don't internationalize the command-line
switches.
So perhaps this is a suitable band-aid instead?
parse-options.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/parse-options.c b/parse-options.c
index 67e98a6..20dc742 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
#include "cache.h"
#include "commit.h"
#include "color.h"
+#include "utf8.h"
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
if (ctx.argv[0][1] == '-') {
error("unknown option `%s'", ctx.argv[0] + 2);
} else {
- error("unknown switch `%c'", *ctx.opt);
+ const char *next = ctx.opt;
+ utf8_width(&next, NULL);
+ error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
}
usage_with_options(usagestr, options);
}
--
1.7.11.7
^ permalink raw reply related
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Matthieu Moy @ 2013-02-11 13:43 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git
In-Reply-To: <1360589687-9233-1-git-send-email-kusmabite@gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -3,6 +3,7 @@
> #include "cache.h"
> #include "commit.h"
> #include "color.h"
> +#include "utf8.h"
>
> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> const char * const *usagestr,
> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
> if (ctx.argv[0][1] == '-') {
> error("unknown option `%s'", ctx.argv[0] + 2);
> } else {
> - error("unknown switch `%c'", *ctx.opt);
> + const char *next = ctx.opt;
> + utf8_width(&next, NULL);
> + error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
> }
> usage_with_options(usagestr, options);
> }
You should be careful with the case where the user has a non-UTF8
environment, and entered a non-ascii sequence. I can see two cases:
1) The non-ascii sequence is valid UTF-8, then I guess your patch would
show two characters instead of one. Not really correct, but not really
serious either.
2) The non-ascii sequence is NOT valid UTF-8, then if I read correctly
(I didn't test) utf8_width would set next to NULL, and then you are
in big trouble.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Erik Faye-Lund @ 2013-02-11 13:57 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqobfr9da7.fsf@grenoble-inp.fr>
On Mon, Feb 11, 2013 at 2:43 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -3,6 +3,7 @@
>> #include "cache.h"
>> #include "commit.h"
>> #include "color.h"
>> +#include "utf8.h"
>>
>> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
>> const char * const *usagestr,
>> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
>> if (ctx.argv[0][1] == '-') {
>> error("unknown option `%s'", ctx.argv[0] + 2);
>> } else {
>> - error("unknown switch `%c'", *ctx.opt);
>> + const char *next = ctx.opt;
>> + utf8_width(&next, NULL);
>> + error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
>> }
>> usage_with_options(usagestr, options);
>> }
>
> You should be careful with the case where the user has a non-UTF8
> environment, and entered a non-ascii sequence. I can see two cases:
>
> 1) The non-ascii sequence is valid UTF-8, then I guess your patch would
> show two characters instead of one. Not really correct, but not really
> serious either.
Hm. So we would end up trading some form of corruption for some other.
Not the biggest problem in the world, but perhaps there's a way of
fixing it?
I'm not entirely sure how to correctly know what encoding stdin is
supposed to be. On Windows, that's easy; it's UTF-16, we re-encode it
to UTF-8 on startup in Git for Windows. But on other platforms, I have
no clue.
But isn't UTF-8 constructed to be very unlikely to clash with existing
encodings? If so, I could add a case for non-ascii and non-UTF-8, that
simply writes the byte as a hex-tuple?
> 2) The non-ascii sequence is NOT valid UTF-8, then if I read correctly
> (I didn't test) utf8_width would set next to NULL, and then you are
> in big trouble.
Outch. Yeah, you are right; this is not good at all :)
But I guess the solution above should fix this as well, no?
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Matthieu Moy @ 2013-02-11 14:05 UTC (permalink / raw)
To: kusmabite; +Cc: git
In-Reply-To: <CABPQNSbYCrdSP5rWbfLX==u--bJpQo6A6sNE46a1RuU-fMDiWg@mail.gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> But isn't UTF-8 constructed to be very unlikely to clash with existing
> encodings? If so, I could add a case for non-ascii and non-UTF-8, that
> simply writes the byte as a hex-tuple?
If it's non-ascii and non-UTF-8, I think you'd want to display the byte
as it is, because this is how it was entered. IOW, I'd say we should
keep the current behavior in this case.
>> 2) The non-ascii sequence is NOT valid UTF-8, then if I read correctly
>> (I didn't test) utf8_width would set next to NULL, and then you are
>> in big trouble.
>
> Outch. Yeah, you are right; this is not good at all :)
>
> But I guess the solution above should fix this as well, no?
It should, yes.
Of course, there's still the case where the user entered "git -é" as a
à followed by a © in a latin-1 environment, but as you said, it's
unlikely enough ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Robert Zeh @ 2013-02-11 14:13 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Ramkumar Ramachandra, Git List
In-Reply-To: <CACsJy8BvN0xX_=fx78hVLw=2Wyk=RUHYs_x9r5RJ0TvBAoA83g@mail.gmail.com>
On Sun, Feb 10, 2013 at 9:21 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Feb 11, 2013 at 2:03 AM, Robert Zeh <robert.allan.zeh@gmail.com> wrote:
>> On Sat, Feb 9, 2013 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>>
>>>> This is much better than Junio's suggestion to study possible
>>>> implementations on all platforms and designing a generic daemon/
>>>> communication channel. That's no weekend project.
>>>
>>> It appears that you misunderstood what I wrote. That was not "here
>>> is a design; I want it in my system. Go implemment it".
>>>
>>> It was "If somebody wants to discuss it but does not know where to
>>> begin, doing a small experiment like this and reporting how well it
>>> worked here may be one way to do so.", nothing more.
>>
>> What if instead of communicating over a socket, the daemon
>> dumped a file containing all of the lstat information after git
>> wrote a file? By definition the daemon should know about file writes.
>>
>> There would be no network communication, which I think would make
>> things more secure. It would simplify the rendezvous by insisting on
>> well known locations in $GIT_DIR.
>
> We need some sort of interactive communication to the daemon anyway,
> to validate that the information is uptodate. Assume that a user makes
> some changes to his worktree before starting the daemon, git needs to
> know that what the daemon provides does not represent a complete
> file-change picture and it better refreshes the index the old way
> once, then trust the daemon.
>
> I think we could solve that by storing a "session id", provided by the
> daemon, in .git/index. If the session id is not present (or does not
> match what the current daemon gives), refresh the old way. After
> refreshing, it may ask the daemon for new session id and store it.
> Next time if the session id is still valid, trust the daemon's data.
> This session id should be different every time the daemon restarts for
> this to work.
I think we could do this without interactive communication,
if we did the following:
1) The Daemon waits to see $GIT_DIR/lstat_request, and atomically
writes out $GIT_DIR/lstat_cache. By atomically I mean that it writes
things out to a temporary file, and then does a rename.
2) The client erases $GIT_DIR/lstat_cache, and writes
$GIT_DIR/lstat_request
I think this is better than socket based communication because there
are fewer places to check
for failures.
Robert
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Erik Faye-Lund @ 2013-02-11 14:27 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqy5ev7xpp.fsf@grenoble-inp.fr>
On Mon, Feb 11, 2013 at 3:05 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> But isn't UTF-8 constructed to be very unlikely to clash with existing
>> encodings? If so, I could add a case for non-ascii and non-UTF-8, that
>> simply writes the byte as a hex-tuple?
>
> If it's non-ascii and non-UTF-8, I think you'd want to display the byte
> as it is, because this is how it was entered. IOW, I'd say we should
> keep the current behavior in this case.
>
Yes, you are of course right. We should detect UTF-8, and only in
those cases do anything special. Because the likely alternatives are
other 8-byte encodings (which the terminal already should grok, since
the user was able to input it), or other multi-byte sequences (which
already is broken, and is tricky to handle). So at least we'd only
break in very unlikely cases.
But, I wonder, could mbrlen be used to detect the length instead? It
consults LC_CTYPE to find out what encoding to use, which seems like
it might give the correct answer in all non-corrupted cases... I'm far
from an expert on UNIX-internationalization, though. And this approach
is likely to break on Windows, but I suspect that we can perform some
well-placed hack for it, as we already know that we're doing UTF-8
there.
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 16:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Sitaram Chamarty, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7v8v6vpbej.fsf@alter.siamese.dyndns.org>
On Sun, Feb 10, 2013 at 11:17:24PM -0800, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
> > Isn't that a criticism of the git-shell-commands facility in general?
> > If it is common to have a lot of users with distinct home directories
> > but all with git-shell as their login shell, then the
> > git-shell-commands should not go in their home directory to begin
> > with, no?
>
> You can give one set of commands to some users while restricting
> others, no?
But that seems to me to argue against /etc/git/shell-disabled or
similar, which would apply to every user. Or are you proposing that the
check be:
if -d ~/git-shell-commands; then
: ok, interactive
elif -x /etc/git/shell-disabled; then
exec /etc/git/shell-disabled
else
echo >&2 'go away'
exit 1
fi
That at least means you can apply _whether_ to disable the shell
selectively for each user (by providing or not a git-shell-commands
directory), but you cannot individually select the script that runs for
that user. But it's probably still flexible enough; you can, after all, run
arbitrary code in the shell-disabled script, so it can select which
class of user it was called on and dispatch to a sub-script.
-Peff
^ 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