* RE: [PATCH] http(s): automatically try NTLM authentication first
From: David Turner @ 2017-02-23 1:03 UTC (permalink / raw)
To: 'brian m. carlson'
Cc: 'Junio C Hamano', git@vger.kernel.org,
Johannes Schindelin, Eric Sunshine, Jeff King
In-Reply-To: <20170222233419.q3fxqmrscosumbjm@genre.crustytoothpaste.net>
> -----Original Message-----
> From: brian m. carlson [mailto:sandals@crustytoothpaste.net]
>
> This is SPNEGO. It will work with NTLM as well as Kerberos.
>
> Browsers usually disable this feature by default, as it basically will attempt to
> authenticate to any site that sends a 401. For Kerberos against a malicious
> site, the user will either not have a valid ticket for that domain, or the user's
> Kerberos server will refuse to provide a ticket to pass to the server, so
> there's no security risk involved.
>
> I'm unclear how SPNEGO works with NTLM, so I can't speak for the security
> of it. From what I understand of NTLM and from RFC 4559, it consists of a
> shared secret. I'm unsure what security measures are in place to not send
> that to an untrusted server.
>
> As far as Kerberos, this is a desirable feature to have enabled, with little
> downside. I just don't know about the security of the NTLM part, and I don't
> think we should take this patch unless we're sure we know the
> consequences of it.
NTLM on its own is bad:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa378749(v=vs.85).aspx
says:
"
1. (Interactive authentication only) A user accesses a client computer and
provides a domain name, user name, and password. The client computes a
cryptographic hash of the password and discards the actual password.
2. The client sends the user name to the server (in plaintext).
3. The server generates a 16-byte random number, called a challenge or
nonce, and sends it to the client.
4. The client encrypts this challenge with the hash of the user's password
and returns the result to the server. This is called the response.
..."
Wait, what? If I'm a malicious server, I can get access to an offline oracle
for whether I've correctly guessed the user's password? That doesn't
sound secure at all! Skimming the SPNEGO RFCs, there appears to be no
mitigation for this.
So, I guess, this patch might be considered a security risk. But on the
other hand, even *without* this patch, and without http.allowempty at
all, I think a config which simply uses a https:// url without the magic :@
would try SPNEGO. As I understand it, the http.allowempty config just
makes the traditional :@ urls work.
Actually, though, I am not sure this is as bad as it seems, because gssapi
might protect us. When I locally tried a fake server, git (libcurl) refused to
send my Kerberos credentials because "Server not found in Kerberos
database". I don't have a machine set up with NTLM authentication
(because, apparently, that would be insane), so I don't know how to
confirm that gssapi would operate off of a whitelist for NTLM as well.
^ permalink raw reply
* RE: [PATCH 2/2] http: add an "auto" mode for http.emptyauth
From: David Turner @ 2017-02-23 1:16 UTC (permalink / raw)
To: 'Jeff King', Junio C Hamano
Cc: git@vger.kernel.org, sandals@crustytoothpaste.net,
Johannes Schindelin, Eric Sunshine
In-Reply-To: <20170222234059.iajn2zuwzkzjxit2@sigill.intra.peff.net>
I don't know enough about how libcurl handles authentication to know whether
these patches are a good idea, but I have a minor comment anyway.
> -----Original Message-----
> From: Jeff King [mailto:peff@peff.net]
> +static int curl_empty_auth_enabled(void) {
> + if (curl_empty_auth < 0) {
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> + /*
> + * In the automatic case, kick in the empty-auth
> + * hack as long as we would potentially try some
> + * method more exotic than "Basic".
> + *
> + * But only do so when this is _not_ our initial
> + * request, as we would not then yet know what
> + * methods are available.
> + */
Eliminate double-negative:
"But only do this when this is our second or subsequent request,
as by then we know what methods are available."
^ permalink raw reply
* Re: feature request: user email config per domain
From: Igor Djordjevic BugA @ 2017-02-23 1:30 UTC (permalink / raw)
To: Tushar Kapila, git
In-Reply-To: <CAN0Skmmjd5Y0uWz_WC69mAStucZ6nR0mjdp4-ODJz2UnTaB-eQ@mail.gmail.com>
Hi Tushar,
On 22/02/2017 14:12, Tushar Kapila <tgkprog@gmail.com> wrote:
> So when remote URL has github.com push as tgkprog@search.com but for
> testing.abc.doman:8080 use tgkprog@test.xyz.com ?
I`m not sure if this is sensible, as authorship information is baked
into the commit at the time of committing, which (usually ;) happens
before you get to 'git push' to the other repo.
If possible, changing this info after the fact, on 'git push', would
influence the existing commit you`re trying to send over, so your
'git-push' would have a surprising consequence of not actually
pushing your desired commit at all, but creating a totally new commit
inside the other repo -- this new commit would be exactly the same
patch-wise (in regards to differences introduced), but because of the
changed user info it would be considered a different commit
nonetheless (different hash).
> ... I know I can over ride it per repository, but sometimes
> forget to do that. And even if I unset it, it inadvertantly gets set
> elsewhere when I make a repo and the site 'helps' me by showing me the
> commands to init and clone my new repo.
Otherwise, as you already stated that you find the current local (per
repo) user settings override logic inconvenient (error-prone), you
might be interested in approach described in this[1] Stack Overflow
post.
In short, it uses a template-injected 'post-checkout' hook (triggered
on 'git clone' as well) alongside '.gitconfig' (global) settings to
achieve what seems to be pretty similar to what you asked for (but
might be a bit more sensible), where you may fine-tune it further to
better suit your needs.
On 20/02/2017 21:12, Grant Humphries[2] wrote[1]:
> This answer is partially inspired by the post by @Saucier, but I was
> looking for an automated way to set user.name and user.email on a per
> repo basis, based on the remote, that was a little more light weight
> than the git-passport package that he developed. Also h/t to @John
> for the useConfigOnly setting. Here is my solution:
>
> .gitconfig changes:
>
> [github]
> name = <github username>
> email = <github email>
> [gitlab]
> name = <gitlab username>
> email = <gitlab email>
> [init]
> templatedir = ~/.git-templates
> [user]
> useConfigOnly = true
>
> post-checkout hook which should be saved to the following path:
> ~/.git-templates/hooks/post-checkout:
>
> #!/usr/bin/env bash
>
> # make regex matching below case insensitive
> shopt -s nocasematch
>
> # values in the services array should have a corresponding section in
> # .gitconfig where the 'name' and 'email' for that service are specified
> remote_url="$( git config --get --local remote.origin.url )"
> services=(
> 'github'
> 'gitlab'
> )
>
> set_local_user_config() {
> local service="${1}"
> local config="${2}"
> local service_config="$( git config --get ${service}.${config} )"
> local local_config="$( git config --get --local user.${config} )"
>
> if [[ "${local_config}" != "${service_config}" ]]; then
> git config --local "user.${config}" "${service_config}"
> echo "repo 'user.${config}' has been set to '${service_config}'"
> fi
> }
>
> # if remote_url doesn't contain the any of the values in the services
> # array the user name and email will remain unset and the
> # user.useConfigOnly = true setting in .gitconfig will prompt for those
> # credentials and prevent commits until they are defined
> for s in "${services[@]}"; do
> if [[ "${remote_url}" =~ "${s}" ]]; then
> set_local_user_config "${s}" 'name'
> set_local_user_config "${s}" 'email'
> break
> fi
> done
>
> I use different credentials for github and gitlab, but those
> references in the code above could be replaced or augmented with any
> service that you use. In order to have the post-checkout hook
> automatically set the user name and email locally for a repo after a
> checkout make sure the service name appears in the remote url, add it
> to the services array in the post-checkout script and create a
> section for it in your .gitconfig that contains your user name and
> email for that service.
>
> If none of the service names appear in the remote url or the repo
> doesn't have a remote the user name and email will not be set
> locally. In these cases the user.useConfigOnly setting will be in
> play which will not allow you to make commits until the user name and
> email are set at the repo level, and will prompt the user to
> configure that information.
Regards,
Buga
*P.S.* For the purpose of completeness and archiving I copied the
Stack Overflow post[1] here as well, but all the credits go to its
author[2] (you may upvote the linked post[1] if you find it helpful).
Please feel free let me know if this practice is otherwise to be
avoided.
[1] http://stackoverflow.com/a/42354282
[2] http://stackoverflow.com/users/2167004
^ permalink raw reply
* Re: [PATCH 2/2] http: add an "auto" mode for http.emptyauth
From: Jeff King @ 2017-02-23 1:37 UTC (permalink / raw)
To: David Turner
Cc: Junio C Hamano, git@vger.kernel.org, sandals@crustytoothpaste.net,
Johannes Schindelin, Eric Sunshine
In-Reply-To: <b5778a7988ad4dfa9adfc8d312432189@exmbdft7.ad.twosigma.com>
On Thu, Feb 23, 2017 at 01:16:33AM +0000, David Turner wrote:
> I don't know enough about how libcurl handles authentication to know whether
> these patches are a good idea, but I have a minor comment anyway.
As somebody who is using non-Basic auth, can you apply these patches and
show us the output of:
GIT_TRACE_CURL=1 \
git ls-remote https://your-server 2>&1 >/dev/null |
egrep '(Send|Recv) header: (GET|HTTP|Auth)'
(without http.emptyauth turned on, obviously).
> > + * But only do so when this is _not_ our initial
> > + * request, as we would not then yet know what
> > + * methods are available.
> > + */
>
> Eliminate double-negative:
>
> "But only do this when this is our second or subsequent request,
> as by then we know what methods are available."
Yeah, that is clearer.
-Peff
^ permalink raw reply
* Re: [PATCH] http(s): automatically try NTLM authentication first
From: Junio C Hamano @ 2017-02-23 2:15 UTC (permalink / raw)
To: Jeff King
Cc: brian m. carlson, David Turner, git@vger.kernel.org,
Johannes Schindelin, Eric Sunshine
In-Reply-To: <20170222234246.wjp3567vesdusiaf@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Feb 22, 2017 at 11:34:19PM +0000, brian m. carlson wrote:
>
>> Browsers usually disable this feature by default, as it basically will
>> attempt to authenticate to any site that sends a 401. For Kerberos
>> against a malicious site, the user will either not have a valid ticket
>> for that domain, or the user's Kerberos server will refuse to provide a
>> ticket to pass to the server, so there's no security risk involved.
>>
>> I'm unclear how SPNEGO works with NTLM, so I can't speak for the
>> security of it. From what I understand of NTLM and from RFC 4559, it
>> consists of a shared secret. I'm unsure what security measures are in
>> place to not send that to an untrusted server.
>>
>> As far as Kerberos, this is a desirable feature to have enabled, with
>> little downside. I just don't know about the security of the NTLM part,
>> and I don't think we should take this patch unless we're sure we know
>> the consequences of it.
>
> Hmm. That would be a problem with my proposed patch 2 then, too, if only
> because it turns the feature on by default in more places.
>
> If it _is_ dangerous to turn on all the time, I'd think we should
> consider warning people in the http.emptyauth documentation.
Yeah, http.<url>.emptyAuth that knows where it is going may be a lot
safer but a blanket http.emptyAuth does sound bad.
^ permalink raw reply
* Re: [PATCH] http(s): automatically try NTLM authentication first
From: brian m. carlson @ 2017-02-23 4:19 UTC (permalink / raw)
To: David Turner
Cc: 'Junio C Hamano', git@vger.kernel.org,
Johannes Schindelin, Eric Sunshine, Jeff King
In-Reply-To: <b152fad7e79046c5aa6cac9e21066c1c@exmbdft7.ad.twosigma.com>
[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]
On Thu, Feb 23, 2017 at 01:03:39AM +0000, David Turner wrote:
> So, I guess, this patch might be considered a security risk. But on the
> other hand, even *without* this patch, and without http.allowempty at
> all, I think a config which simply uses a https:// url without the magic :@
> would try SPNEGO. As I understand it, the http.allowempty config just
> makes the traditional :@ urls work.
No, it's a bit different. libcurl won't try to authenticate to a server
unless it has a username (and possibly password). With the curl command
line client, you use a dummy value or -u: to force it to do auth anyway
(because you want, say, GSSAPI). http.emptyAuth just sets that option
to “:” so libcurl will auth:
if (curl_empty_auth)
curl_easy_setopt(result, CURLOPT_USERPWD, ":");
I just use a dummy username for my URLs, but you can write :@ or any
other permutation to get it to work without emptyAuth. As a
consequence, you have to opt-in to that on a per-URL (or per-domain)
basis, which is a bit more secure.
> Actually, though, I am not sure this is as bad as it seems, because gssapi
> might protect us. When I locally tried a fake server, git (libcurl) refused to
> send my Kerberos credentials because "Server not found in Kerberos
> database". I don't have a machine set up with NTLM authentication
> (because, apparently, that would be insane), so I don't know how to
> confirm that gssapi would operate off of a whitelist for NTLM as well.
Yup. That's pretty much what I thought would happen, since the Kerberos
server has no HTTP/malicious.evil.tld@YOURREALM.TLD service ticket.
Again, I don't know how NTLM does things, or if it's wrapped in a
suitable ticket format somehow.
Last I base64-decoded an NTLM SPNEGO response, it did not contain the
OID required by GSSAPI as a prefix; it instead contained an “NTLMSSP”
header, which isn't a valid OID. I didn't delve much further, since I
was pretty sure I didn't want to know more.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* git email From: parsing (was Re: [GIT PULL] Staging/IIO driver patches for 4.11-rc1)
From: Greg KH @ 2017-02-23 6:04 UTC (permalink / raw)
To: Linus Torvalds, Simon Sandström
Cc: Andrew Morton, git, Linux Kernel Mailing List,
Linux Driver Project
In-Reply-To: <CA+55aFy1JpXmo_PpC7f0zZa0YAP6rz+bztJ+fpDUoWgCz0_FMw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]
On Wed, Feb 22, 2017 at 11:59:01AM -0800, Linus Torvalds wrote:
> On Wed, Feb 22, 2017 at 6:56 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > =?UTF-8?q?Simon=20Sandstr=C3=B6m?= (1):
> > staging: vt6656: Add missing identifier names
>
> Wow, your scripts really screwed up that name.
>
> I'm assuming this is quilt not doing proper character set handling..
>
> Because if it's git, we need to get that fixed (but I'm pretty sure
> git gets this right - there are various tests for email header
> quoting).
>
> Alternatively, somebody hand-edited some email and moved the From:
> header to the body without fixing up the RFC 1342 mail header quoting
> (which is very different from how quoting works in the *body* of an
> email).
>
> Poor Simon Sandström.
>
> Funnily enough, this only exists for one commit. You've got several
> other commits from Simon that get his name right.
>
> What happened?
I don't know what happened, I used git for this, I don't use quilt for
"normal" patches accepted into my trees anymore, only for stable kernel
work.
So either the mail is malformed, or git couldn't figure it out, I've
attached the original message below, and cc:ed the git mailing list.
Also, Simon emailed me after this was committed saying something went
wrong, but I couldn't go back and rebase my tree. Simon, did you ever
figure out if something was odd on your end?
Git developers, any ideas?
thanks,
greg k-h
[-- Attachment #2: messy_email.mbox --]
[-- Type: application/mbox, Size: 6355 bytes --]
^ permalink raw reply
* Re: git email From: parsing (was Re: [GIT PULL] Staging/IIO driver patches for 4.11-rc1)
From: Jeff King @ 2017-02-23 6:17 UTC (permalink / raw)
To: Greg KH
Cc: Linus Torvalds, Simon Sandström, Andrew Morton, git,
Linux Kernel Mailing List, Linux Driver Project
In-Reply-To: <20170223060444.GA26196@kroah.com>
On Thu, Feb 23, 2017 at 07:04:44AM +0100, Greg KH wrote:
> > Poor Simon Sandström.
> >
> > Funnily enough, this only exists for one commit. You've got several
> > other commits from Simon that get his name right.
> >
> > What happened?
>
> I don't know what happened, I used git for this, I don't use quilt for
> "normal" patches accepted into my trees anymore, only for stable kernel
> work.
>
> So either the mail is malformed, or git couldn't figure it out, I've
> attached the original message below, and cc:ed the git mailing list.
>
> Also, Simon emailed me after this was committed saying something went
> wrong, but I couldn't go back and rebase my tree. Simon, did you ever
> figure out if something was odd on your end?
>
> Git developers, any ideas?
The problem isn't on the applying end, but rather on the generating end.
The From header in the attached mbox is:
From: =?us-ascii?B?PT9VVEYtOD9xP1NpbW9uPTIwU2FuZHN0cj1DMz1CNm0/PQ==?= <simon@nikanor.nu>
If you de-base64 that, you get:
=?UTF-8?q?Simon=20Sandstr=C3=B6m?=
So something double-encoded it before it got to your mbox.
-Peff
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Jeff King @ 2017-02-23 5:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <xmqqd1ebfd9l.fsf_-_@gitster.mtv.corp.google.com>
On Tue, Feb 21, 2017 at 01:24:38PM -0800, Junio C Hamano wrote:
> The parsing of one-shot assignments of configuration variables that
> come from the command line historically was quite loose and allowed
> anything to pass.
>
> The configuration variable names that come from files are validated
> in git_config_parse_source(), which uses get_base_var() that grabs
> the <section> (and subsection) while making sure that <section>
> consists of iskeychar() letters, the function itself that makes sure
> that the first letter in <variable> is isalpha(), and get_value()
> that grabs the remainder of the <variable> name while making sure
> that it consists of iskeychar() letters.
>
> Perform an equivalent check in canonicalize_config_variable_name()
> to catch invalid configuration variable names that come from the
> command line.
FWIW, the code looks OK here. It is a shame to duplicate the policy
found in git_config_parse_key(), though.
I wonder if we could make a master version of that which canonicalizes
in-place, and then just wrap it for the git_config_parse_key()
interface. Actually, I guess the function you just wrote _is_ that inner
function, as long as it learned about the "quiet" flag.
-Peff
^ permalink raw reply
* Re: [RFC][Git GUI] Make Commit message field in git GUI re sizable.
From: Jessie Hernandez @ 2017-02-23 6:46 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Bert Wesarg, Git Mailing List, Pat Thoyts
In-Reply-To: <59ab152c-62d1-a2ec-eee5-0bd909674988@xiplink.com>
> On 2017-02-22 06:59 AM, Jessie Hernandez wrote:
>>> HI,
>>>
>>> the reason why it is fixed, is because commit messages should be
>>> wrapped at 76 characters to be used in mails. So it helps you with the
>>> wrapping.
>>>
>>> Bert
>>
>> Right ok. I understand.
>> Knowing this I think I might start writing my commit messages
>> differently
>> then.
>
> You can configure gui.commitMsgWidth if you don't like the default
> (which is 75, not 76).
>
> M.
Hi Marc,
Yeah I did that in my local version and got my desired effect.
But knowing it was designed like this and and not a bug,
I am happy to use the designed behavior and adjust my way of working.
-----------------
Jessie Hernandez
>
>
>> Thank you for this.
>>
>> Regards
>>
>> -----------------
>> Jessie Hernandez
>>
>>>
>>>
>>> On Wed, Feb 22, 2017 at 10:27 AM, Jessie Hernandez
>>> <jessie@jessiehernandez.com> wrote:
>>>> Hi all,
>>>>
>>>> I have been using git for a few years now and really like the
>>>> software.
>>>> I have a small annoyance and was wondering if I could get the
>>>> communities
>>>> view on this.
>>>>
>>>> When using git GUI I find it handy to be able to re-size the "Unstaged
>>>> Changes" and the "Staged Changed" fields.
>>>>
>>>> I would like the same thing for the "Commit Message" field, or to have
>>>> it
>>>> re-size with the git GUI window.
>>>>
>>>> I can re-size the "Commit Message" vertically when making the
>>>> "Modified"
>>>> panel smaller.
>>>>
>>>> Does this make sense?
>>>> I would be happy to get into more detail if that is necessary or if
>>>> something is not clear.
>>>>
>>>> Thank you.
>>>>
>>>> -----------------
>>>> Jessie Hernandez
>>>>
>>>>
>>>
>>
>>
>
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Junio C Hamano @ 2017-02-23 7:19 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20170223055831.u3yofkby3c56t7l4@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> FWIW, the code looks OK here. It is a shame to duplicate the policy
> found in git_config_parse_key(), though.
>
> I wonder if we could make a master version of that which canonicalizes
> in-place, and then just wrap it for the git_config_parse_key()
> interface. Actually, I guess the function you just wrote _is_ that inner
> function, as long as it learned about the "quiet" flag.
Hmm, I obviously missed an opportunity. I thought about doing a
similar thing with the policy in parse-source, but that side didn't
seem worth doing, as the config-parse-source callgraph is quite a
mess (as it has to parse the .ini like format with line breaks and
comments, not the simple "<string>[.<string>].<string>" thing, it
cannot quite avoid it), and it needs to take advantage of _some_
policy to parse the pieces.
We could loosen the policy implemented by config-parse-key interface
(e.g. change config-parse-source to let anything that begins with a
non-whitespace continue to be processed with get_value(), instead of
only allowing string that begins with isalpha(); similarly loosen
get_value() to allow any non-dot non-space string, not just
iskeychar() bytes) and first turn what is read into the simple
"<string>[.<string>].<string>" format, and then reuse the new
"master" logic to validate. That would allow us to update the
"master" logic to make it tighter or looser to some degree, but the
source parser still needs to hardcode _some_ policy (e.g. the first
level and the last level names begin with a non-space) that allows
it to guess what "<string>" pieces the contents being parsed from
the .ini looking format meant to express.
But you are right. config-parse-key does have the simpler string
that can just be given to the canonicalize thing and we should be
able to reuse it.
^ permalink raw reply
* Re: git email From: parsing (was Re: [GIT PULL] Staging/IIO driver patches for 4.11-rc1)
From: Simon Sandström @ 2017-02-23 7:59 UTC (permalink / raw)
To: Jeff King, Greg KH
Cc: Linus Torvalds, Andrew Morton, git, Linux Kernel Mailing List,
Linux Driver Project
In-Reply-To: <20170223061702.bzzgrntotppvwdw6@sigill.intra.peff.net>
On Thu, Feb 23, 2017 at 01:17:02AM -0500, Jeff King wrote:
> On Thu, Feb 23, 2017 at 07:04:44AM +0100, Greg KH wrote:
>
> >
> > I don't know what happened, I used git for this, I don't use quilt for
> > "normal" patches accepted into my trees anymore, only for stable kernel
> > work.
> >
> > So either the mail is malformed, or git couldn't figure it out, I've
> > attached the original message below, and cc:ed the git mailing list.
> >
> > Also, Simon emailed me after this was committed saying something went
> > wrong, but I couldn't go back and rebase my tree. Simon, did you ever
> > figure out if something was odd on your end?
> >
> > Git developers, any ideas?
>
> The problem isn't on the applying end, but rather on the generating end.
> The From header in the attached mbox is:
>
> From: =?us-ascii?B?PT9VVEYtOD9xP1NpbW9uPTIwU2FuZHN0cj1DMz1CNm0/PQ==?= <simon@nikanor.nu>
>
> If you de-base64 that, you get:
>
> =?UTF-8?q?Simon=20Sandstr=C3=B6m?=
>
> So something double-encoded it before it got to your mbox.
>
> -Peff
Hi,
Yes, Mutt on my end caused this. I used Mutt 1.5.23, and I either had
it misconfigured or it simply can't handle files that contains encoded
From:/Subject:-fields when you run it with mutt -H <file>.
I upgraded to Mutt 1.7.1 which solved the problem.
- Simon
^ permalink raw reply
* Re: possible bug: inconsistent CLI behaviour for empty user.name
From: Jeff King @ 2017-02-23 8:11 UTC (permalink / raw)
To: bs.x.ttp; +Cc: git
In-Reply-To: <20170203051309.a737846dd26a6ed8df1e4112@gmx.de>
On Fri, Feb 03, 2017 at 05:13:09AM +0100, bs.x.ttp@recursor.net wrote:
> The problem is that GIT accepts a user.name of " " for some operations
> (for example when doing a simple "git commit"), but does require a
> "non-empty" user.name for others (like git commit --amend and git
> rebase). In case of the latter commands GIT fails with the message
> "fatal: empty ident name (for <email@address>) not allowed".
I think it's a bug. We try to always reject empty usernames, but the
"empty" check is done before we cut off leading and trailing cruft (like
whitespace).
The "--amend" command notices because it actually parses the name out of
the existing commit. That version has already had its whitespace eaten
up (when it was written into the original commit), and so it ends up as
blank.
Here's a series which fixes that along with a few other oddities I
noticed.
[1/4]: ident: mark error messages for translation
[2/4]: ident: handle NULL email when complaining of empty name
[3/4]: ident: reject all-crud ident name
[4/4]: ident: do not ignore empty config name/email
ident.c | 49 ++++++++++++++++++++++++++-----------------
t/t7518-ident-corner-cases.sh | 36 +++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 19 deletions(-)
create mode 100755 t/t7518-ident-corner-cases.sh
-Peff
^ permalink raw reply
* [PATCH 2/4] ident: handle NULL email when complaining of empty name
From: Jeff King @ 2017-02-23 8:13 UTC (permalink / raw)
To: bs.x.ttp; +Cc: git
In-Reply-To: <20170223081157.hwfn3msfux5udmng@sigill.intra.peff.net>
If we see an empty name, we complain about and mention the
matching email in the error message (to give it some
context). However, the "email" pointer may be NULL here if
we were planning to fill it in later from ident_default_email().
This was broken by 59f929596 (fmt_ident: refactor strictness
checks, 2016-02-04). Prior to that commit, we would look up
the default name and email before doing any other actions.
So one solution would be to go back to that.
However, we can't just do so blindly. The logic for handling
the "!email" condition has grown since then. In particular,
looking up the default email can die if getpwuid() fails,
but there are other errors that should take precedence.
Commit 734c7789a (ident: check for useConfigOnly before
auto-detection of name/email, 2016-03-30) reordered the
checks so that we prefer the error message for
useConfigOnly.
Instead, we can observe that while the name-handling depends
on "email" being set, the reverse is not true. So we can
simply set up the email variable first.
This does mean that if both are bogus, we'll complain about
the email before the name. But between the two, there is no
reason to prefer one over the other.
Signed-off-by: Jeff King <peff@peff.net>
---
ident.c | 26 +++++++++++++-------------
t/t7518-ident-corner-cases.sh | 20 ++++++++++++++++++++
2 files changed, 33 insertions(+), 13 deletions(-)
create mode 100755 t/t7518-ident-corner-cases.sh
diff --git a/ident.c b/ident.c
index dde82983a..ea6034581 100644
--- a/ident.c
+++ b/ident.c
@@ -351,6 +351,19 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
+ if (!email) {
+ if (strict && ident_use_config_only
+ && !(ident_config_given & IDENT_MAIL_GIVEN)) {
+ fputs(_(env_hint), stderr);
+ die(_("no email was given and auto-detection is disabled"));
+ }
+ email = ident_default_email();
+ if (strict && default_email_is_bogus) {
+ fputs(_(env_hint), stderr);
+ die(_("unable to auto-detect email address (got '%s')"), email);
+ }
+ }
+
if (want_name) {
int using_default = 0;
if (!name) {
@@ -378,19 +391,6 @@ const char *fmt_ident(const char *name, const char *email,
}
}
- if (!email) {
- if (strict && ident_use_config_only
- && !(ident_config_given & IDENT_MAIL_GIVEN)) {
- fputs(_(env_hint), stderr);
- die(_("no email was given and auto-detection is disabled"));
- }
- email = ident_default_email();
- if (strict && default_email_is_bogus) {
- fputs(_(env_hint), stderr);
- die(_("unable to auto-detect email address (got '%s')"), email);
- }
- }
-
strbuf_reset(&ident);
if (want_name) {
strbuf_addstr_without_crud(&ident, name);
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
new file mode 100755
index 000000000..6c057afc1
--- /dev/null
+++ b/t/t7518-ident-corner-cases.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='corner cases in ident strings'
+. ./test-lib.sh
+
+# confirm that we do not segfault _and_ that we do not say "(null)", as
+# glibc systems will quietly handle our NULL pointer
+#
+# Note also that we can't use "env" here because we need to unset a variable,
+# and "-u" is not portable.
+test_expect_success 'empty name and missing email' '
+ (
+ sane_unset GIT_AUTHOR_EMAIL &&
+ GIT_AUTHOR_NAME= &&
+ test_must_fail git commit --allow-empty -m foo 2>err &&
+ test_i18ngrep ! null err
+ )
+'
+
+test_done
--
2.12.0.rc2.597.g959f68882
^ permalink raw reply related
* [PATCH 1/4] ident: mark error messages for translation
From: Jeff King @ 2017-02-23 8:12 UTC (permalink / raw)
To: bs.x.ttp; +Cc: git
In-Reply-To: <20170223081157.hwfn3msfux5udmng@sigill.intra.peff.net>
We already translate the big "please tell me who you are"
hint, but missed the individual error messages that go with
it.
Signed-off-by: Jeff King <peff@peff.net>
---
ident.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/ident.c b/ident.c
index ac4ae02b4..dde82983a 100644
--- a/ident.c
+++ b/ident.c
@@ -357,13 +357,13 @@ const char *fmt_ident(const char *name, const char *email,
if (strict && ident_use_config_only
&& !(ident_config_given & IDENT_NAME_GIVEN)) {
fputs(_(env_hint), stderr);
- die("no name was given and auto-detection is disabled");
+ die(_("no name was given and auto-detection is disabled"));
}
name = ident_default_name();
using_default = 1;
if (strict && default_name_is_bogus) {
fputs(_(env_hint), stderr);
- die("unable to auto-detect name (got '%s')", name);
+ die(_("unable to auto-detect name (got '%s')"), name);
}
}
if (!*name) {
@@ -371,7 +371,7 @@ const char *fmt_ident(const char *name, const char *email,
if (strict) {
if (using_default)
fputs(_(env_hint), stderr);
- die("empty ident name (for <%s>) not allowed", email);
+ die(_("empty ident name (for <%s>) not allowed"), email);
}
pw = xgetpwuid_self(NULL);
name = pw->pw_name;
@@ -382,12 +382,12 @@ const char *fmt_ident(const char *name, const char *email,
if (strict && ident_use_config_only
&& !(ident_config_given & IDENT_MAIL_GIVEN)) {
fputs(_(env_hint), stderr);
- die("no email was given and auto-detection is disabled");
+ die(_("no email was given and auto-detection is disabled"));
}
email = ident_default_email();
if (strict && default_email_is_bogus) {
fputs(_(env_hint), stderr);
- die("unable to auto-detect email address (got '%s')", email);
+ die(_("unable to auto-detect email address (got '%s')"), email);
}
}
@@ -403,7 +403,7 @@ const char *fmt_ident(const char *name, const char *email,
strbuf_addch(&ident, ' ');
if (date_str && date_str[0]) {
if (parse_date(date_str, &ident) < 0)
- die("invalid date format: %s", date_str);
+ die(_("invalid date format: %s"), date_str);
}
else
strbuf_addstr(&ident, ident_default_date());
--
2.12.0.rc2.597.g959f68882
^ permalink raw reply related
* [PATCH 3/4] ident: reject all-crud ident name
From: Jeff King @ 2017-02-23 8:15 UTC (permalink / raw)
To: bs.x.ttp; +Cc: git
In-Reply-To: <20170223081157.hwfn3msfux5udmng@sigill.intra.peff.net>
An ident name consisting of only "crud" characters (like
whitespace or punctuation) is effectively the same as an
empty one, because our strbuf_addstr_without_crud() will
remove those characters.
We reject an empty name when formatting a strict ident, but
don't notice an all-crud one because our check happens
before the crud-removal step.
We could skip past the crud before checking for an empty
name, but let's make it a separate code path, for two
reasons. One is that we can give a more specific error
message. And two is that unlike a blank name, we probably
don't want to kick in the fallback-to-username behavior.
Signed-off-by: Jeff King <peff@peff.net>
---
ident.c | 11 +++++++++++
t/t7518-ident-corner-cases.sh | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/ident.c b/ident.c
index ea6034581..ead09ff7f 100644
--- a/ident.c
+++ b/ident.c
@@ -203,6 +203,15 @@ static int crud(unsigned char c)
c == '\'';
}
+static int has_non_crud(const char *str)
+{
+ for (; *str; str++) {
+ if (!crud(*str))
+ return 1;
+ }
+ return 0;
+}
+
/*
* Copy over a string to the destination, but avoid special
* characters ('\n', '<' and '>') and remove crud at the end
@@ -389,6 +398,8 @@ const char *fmt_ident(const char *name, const char *email,
pw = xgetpwuid_self(NULL);
name = pw->pw_name;
}
+ if (strict && !has_non_crud(name))
+ die(_("name consists only of disallowed characters: %s"), name);
}
strbuf_reset(&ident);
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index 6c057afc1..3d2560c3c 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -17,4 +17,9 @@ test_expect_success 'empty name and missing email' '
)
'
+test_expect_success 'commit rejects all-crud name' '
+ test_must_fail env GIT_AUTHOR_NAME=" .;<>" \
+ git commit --allow-empty -m foo
+'
+
test_done
--
2.12.0.rc2.597.g959f68882
^ permalink raw reply related
* [PATCH 4/4] ident: do not ignore empty config name/email
From: Jeff King @ 2017-02-23 8:17 UTC (permalink / raw)
To: bs.x.ttp; +Cc: git
In-Reply-To: <20170223081157.hwfn3msfux5udmng@sigill.intra.peff.net>
When we read user.name and user.email from a config file,
they go into strbufs. When a caller asks ident_default_name()
for the value, we fallback to auto-detecting if the strbuf
is empty.
That means that explicitly setting an empty string in the
config is identical to not setting it at all. This is
potentially confusing, as we usually accept a configured
value as the final value.
Signed-off-by: Jeff King <peff@peff.net>
---
This one is perhaps questionable. Maybe somebody is relying on setting a
per-repo user.name to override a ~/.gitconfig value and enforce
auto-detection?
ident.c | 4 ++--
t/t7518-ident-corner-cases.sh | 11 +++++++++++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/ident.c b/ident.c
index ead09ff7f..c0364fe3a 100644
--- a/ident.c
+++ b/ident.c
@@ -153,7 +153,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email,
const char *ident_default_name(void)
{
- if (!git_default_name.len) {
+ if (!(ident_config_given & IDENT_NAME_GIVEN) && !git_default_name.len) {
copy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name);
strbuf_trim(&git_default_name);
}
@@ -162,7 +162,7 @@ const char *ident_default_name(void)
const char *ident_default_email(void)
{
- if (!git_default_email.len) {
+ if (!(ident_config_given & IDENT_MAIL_GIVEN) && !git_default_email.len) {
const char *email = getenv("EMAIL");
if (email && email[0]) {
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index 3d2560c3c..ef570ac62 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -22,4 +22,15 @@ test_expect_success 'commit rejects all-crud name' '
git commit --allow-empty -m foo
'
+# We must test the actual error message here, as an unwanted
+# auto-detection could fail for other reasons.
+test_expect_success 'empty configured name does not auto-detect' '
+ (
+ sane_unset GIT_AUTHOR_NAME &&
+ test_must_fail \
+ git -c user.name= commit --allow-empty -m foo 2>err &&
+ test_i18ngrep "empty ident name" err
+ )
+'
+
test_done
--
2.12.0.rc2.597.g959f68882
^ permalink raw reply related
* [PATCH 1/4] t7003: ensure --prune-empty can prune root commit
From: Devin J. Pohly @ 2017-02-23 8:27 UTC (permalink / raw)
To: git; +Cc: Devin J. Pohly
New test to expose a bug in filter-branch whereby the root commit is
never pruned, even though its tree is empty and --prune-empty is given.
The setup isn't exactly pretty, but I couldn't think of a simpler way to
create a parallel commit graph sans the first commit.
Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
---
t/t7003-filter-branch.sh | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index cb8fbd8e5..2dfe46250 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -313,6 +313,27 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
git cat-file tag X/2 > actual &&
test_cmp expect actual
'
+test_expect_success 'setup --prune-empty comparisons' '
+ git checkout --orphan master-no-a &&
+ git rm -rf . &&
+ unset test_tick &&
+ test_tick &&
+ GIT_COMMITTER_DATE="@0 +0000" GIT_AUTHOR_DATE="@0 +0000" &&
+ test_commit --notick B B.t B Bx &&
+ git checkout -b branch-no-a Bx &&
+ test_commit D D.t D Dx &&
+ mkdir dir &&
+ test_commit dir/D dir/D.t dir/D dir/Dx &&
+ test_commit E E.t E Ex &&
+ git checkout master-no-a &&
+ test_commit C C.t C Cx &&
+ git checkout branch-no-a &&
+ git merge Cx -m "Merge tag '\''C'\'' into branch" &&
+ git tag Fx &&
+ test_commit G G.t G Gx &&
+ test_commit H H.t H Hx &&
+ git checkout branch
+'
test_expect_success 'Prune empty commits' '
git rev-list HEAD > expect &&
@@ -341,6 +362,15 @@ test_expect_success 'prune empty works even without index/tree filters' '
test_cmp expect actual
'
+test_expect_success '--prune-empty is able to prune root commit' '
+ git rev-list branch-no-a >expect &&
+ git branch testing H &&
+ git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t" testing &&
+ git rev-list testing >actual &&
+ git branch -D testing &&
+ test_cmp expect actual
+'
+
test_expect_success '--remap-to-ancestor with filename filters' '
git checkout master &&
git reset --hard A &&
--
2.11.1
^ permalink raw reply related
* [PATCH 2/4] t7003: ensure --prune-empty removes entire branch when applicable
From: Devin J. Pohly @ 2017-02-23 8:27 UTC (permalink / raw)
To: git; +Cc: Devin J. Pohly
In-Reply-To: <20170223082736.31283-1-djpohly@gmail.com>
Sanity check before changing the logic in git_commit_non_empty_tree.
Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
---
t/t7003-filter-branch.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 2dfe46250..7cb60799b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -371,6 +371,13 @@ test_expect_success '--prune-empty is able to prune root commit' '
test_cmp expect actual
'
+test_expect_success '--prune-empty is able to prune entire branch' '
+ git branch prune-entire B &&
+ git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
+ test_path_is_missing .git/refs/heads/prune-entire &&
+ test_must_fail git reflog exists refs/heads/prune-entire
+'
+
test_expect_success '--remap-to-ancestor with filename filters' '
git checkout master &&
git reset --hard A &&
--
2.11.1
^ permalink raw reply related
* [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits
From: Devin J. Pohly @ 2017-02-23 8:27 UTC (permalink / raw)
To: git; +Cc: Devin J. Pohly
In-Reply-To: <20170223082736.31283-1-djpohly@gmail.com>
Previously, the git_commit_non_empty_tree function would always pass any
commit with no parents to git-commit-tree, regardless of whether the
tree was nonempty. The new commit would then be recorded in the
filter-branch revision map, and subsequent commits which leave the tree
untouched would be correctly filtered.
With this change, parentless commits with an empty tree are correctly
pruned, and an empty file is recorded in the revision map, signifying
that it was rewritten to "no commits." This works naturally with the
parent mapping for subsequent commits.
Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
---
Documentation/git-filter-branch.txt | 14 ++++++--------
git-filter-branch.sh | 2 ++
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 0a09698c0..6e4bb0220 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -167,14 +167,12 @@ to other tags will be rewritten to point to the underlying commit.
project root. Implies <<Remap_to_ancestor>>.
--prune-empty::
- Some kind of filters will generate empty commits, that left the tree
- untouched. This switch allow git-filter-branch to ignore such
- commits. Though, this switch only applies for commits that have one
- and only one parent, it will hence keep merges points. Also, this
- option is not compatible with the use of `--commit-filter`. Though you
- just need to use the function 'git_commit_non_empty_tree "$@"' instead
- of the `git commit-tree "$@"` idiom in your commit filter to make that
- happen.
+ Some filters will generate empty commits that leave the tree untouched.
+ This option instructs git-filter-branch to remove such commits if they
+ have exactly one or zero non-pruned parents; merge commits will
+ therefore remain intact. This option cannot be used together with
+ `--commit-filter`, though the same effect can be achieved by using the
+ provided `git_commit_non_empty_tree` function in a commit filter.
--original <namespace>::
Use this option to set the namespace where the original commits
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 86b2ff1e0..2b8cdba15 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -46,6 +46,8 @@ git_commit_non_empty_tree()
{
if test $# = 3 && test "$1" = $(git rev-parse "$3^{tree}"); then
map "$3"
+ elif test $# = 1 && test "$1" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904; then
+ :
else
git commit-tree "$@"
fi
--
2.11.1
^ permalink raw reply related
* [PATCH 4/4] p7000: add test for filter-branch with --prune-empty
From: Devin J. Pohly @ 2017-02-23 8:27 UTC (permalink / raw)
To: git; +Cc: Devin J. Pohly
In-Reply-To: <20170223082736.31283-1-djpohly@gmail.com>
Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
---
t/perf/p7000-filter-branch.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/t/perf/p7000-filter-branch.sh b/t/perf/p7000-filter-branch.sh
index 15ee5d1d5..b029586cc 100755
--- a/t/perf/p7000-filter-branch.sh
+++ b/t/perf/p7000-filter-branch.sh
@@ -16,4 +16,9 @@ test_perf 'noop filter' '
git filter-branch -f base..HEAD
'
+test_perf 'noop prune-empty' '
+ git checkout --detach tip &&
+ git filter-branch -f --prune-empty base..HEAD
+'
+
test_done
--
2.11.1
^ permalink raw reply related
* Re: [PATCH] http(s): automatically try NTLM authentication first
From: Mantas Mikulėnas @ 2017-02-23 9:13 UTC (permalink / raw)
To: David Turner
Cc: 'brian m. carlson', 'Junio C Hamano',
git@vger.kernel.org, Johannes Schindelin, Eric Sunshine,
Jeff King
In-Reply-To: <b152fad7e79046c5aa6cac9e21066c1c@exmbdft7.ad.twosigma.com>
On 2017-02-23 03:03, David Turner wrote:
> Actually, though, I am not sure this is as bad as it seems, because gssapi
> might protect us. When I locally tried a fake server, git (libcurl) refused to
> send my Kerberos credentials because "Server not found in Kerberos
> database". I don't have a machine set up with NTLM authentication
> (because, apparently, that would be insane), so I don't know how to
> confirm that gssapi would operate off of a whitelist for NTLM as well.
NTLM and Kerberos work very differently in that regard.
Kerberos is ticket-based so the client *first* has to obtain a ticket
from the domain's KDC, so a malicious server at minimum needs to know
what principal name to provide (i.e. which real server to try
impersonating). And even if it does that, the ticket doesn't contain
crackable hashes, just data encrypted with a key known only to the KDC
and the real server. So the whitelist is only for privacy and/or
performance reasons, I guess?
NTLM is challenge/response without any third party, and yes, it requires
the application to implement its own whitelisting to avoid the security
problems.
--
Mantas Mikulėnas <grawity@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 1/4] delete_ref: accept a reflog message argument
From: Duy Nguyen @ 2017-02-23 9:33 UTC (permalink / raw)
To: Kyle Meyer; +Cc: Junio C Hamano, Jeff King, Git Mailing List
In-Reply-To: <20170221011035.847-2-kyle@kyleam.com>
On Tue, Feb 21, 2017 at 8:10 AM, Kyle Meyer <kyle@kyleam.com> wrote:
> diff --git a/refs.h b/refs.h
> index 9fbff90e7..5880886a7 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -276,8 +276,8 @@ int reflog_exists(const char *refname);
> * exists, regardless of its old value. It is an error for old_sha1 to
> * be NULL_SHA1. flags is passed through to ref_transaction_delete().
> */
> -int delete_ref(const char *refname, const unsigned char *old_sha1,
> - unsigned int flags);
> +int delete_ref(const char *msg, const char *refname,
> + const unsigned char *old_sha1, unsigned int flags);
Is it just me who thinks it's weird that msg comes in front here? The
key identity, refname, should be the first argument imo. You'll
probably want to update the comment block above if msg can be NULL. We
have _very_ good documentation in this file, let's keep it uptodate.
--
Duy
^ permalink raw reply
* Re: [PATCH v5 01/24] refs.h: add forward declaration for structs used in this file
From: Duy Nguyen @ 2017-02-23 9:26 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Junio C Hamano, Michael Haggerty,
Johannes Schindelin, Ramsay Jones, David Turner
In-Reply-To: <CAGZ79kZTr924RrvG6RsasmhT1yswN875XShtmrrcJ8ztaAGJUw@mail.gmail.com>
On Thu, Feb 23, 2017 at 1:18 AM, Stefan Beller <sbeller@google.com> wrote:
> On Wed, Feb 22, 2017 at 6:04 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> refs.h | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/refs.h b/refs.h
>> index 9fbff90e7..c494b641a 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -1,6 +1,11 @@
>> #ifndef REFS_H
>> #define REFS_H
>>
>> +struct object_id;
>> +struct ref_transaction;
>> +struct strbuf;
>> +struct string_list;
>> +
>> /*
>> * Resolve a reference, recursively following symbolic refererences.
>> *
>> @@ -144,7 +149,6 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
>> * `ref_transaction_commit` is called. So `ref_transaction_verify`
>> * won't report a verification failure until the commit is attempted.
>> */
>> -struct ref_transaction;
>
> Leaving the detailed comment about ref_transaction dangling?
> I can understand if you don't want to move it with the declaration,
> as you want all declarations terse in a few lines.
> Maybe move the comment to be part of the first large comment
> (The one that you can see in the first hunk, starting with
> " * Resolve a reference, recursively following")
I thought the comment block covered the following declarations too,
not just ref_transaction. But on second read it's not that.
Transaction functions are way down below. I'll probably move
ref_transaction back where it was.
--
Duy
^ permalink raw reply
* [ANNOUNCE] Git Rev News edition 24
From: Christian Couder @ 2017-02-23 9:41 UTC (permalink / raw)
To: git
Cc: lwn, Thomas Ferris Nicolaisen, Jakub Narebski, Markus Jansen,
Junio C Hamano, Johannes Schindelin, Jeff King, Stefan Beller,
Pranit Bauva, Michael J Gruber, Jacob Vosmaer,
Ævar Arnfjörð Bjarmason, Josh Triplett,
Erik van Zijst, Brendan Forster, Stefan Saasen, David Turner,
Nguyen Thai Ngoc Duy, Shawn O. Pierce, Carlos Martín Nieto,
Charles Bailey, Karen Sandler, Tim Pettersen, Lars Schneider,
Ben Peart, Cornelius Schumacher
Hi everyone,
The 24th edition of Git Rev News is now published:
https://git.github.io/rev_news/2017/02/22/edition-24/
Thanks a lot to all the contributors and helpers!
Enjoy,
Christian, Thomas, Jakub and Markus.
^ 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