Git tag: pre-receive hook issue

11 messages, 4 authors, 2016-06-15 · open the first message on its own page

Git tag: pre-receive hook issue

From: Garbageyard <hidden>
Date: 2016-06-15 23:05:47

We have a pre-receive hook that checks for JIRA ID whenever someone pushes
code to Git server. I'm trying to avoid this check when someone is applying
a tag. Here's the link for the script: http://pastebin.com/VnMQp5ar

This is the link for output: http://pastebin.com/tBGmYaZF

Problem is that if i run the following command, the output that i get on
command line is 0

git describe --exact-match ac28ca721e67adc04078786164939989a5112d98 2>&1 |
grep -o fatal | wc -w

So i'm wondering why it's not entering the IF block (as confirmed in the
output link)

I agree this is a bad implementation (and i will change it soon) for
checking tags and can be easily done by just checking whether $refname
starts with refs/tags/ but i'll really appreciate if someone could tell me
the mistake i'm committing. I've spent few hours banging my head on this.



--
View this message in context: http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764.html
Sent from the git mailing list archive at Nabble.com.

Re: Git tag: pre-receive hook issue

From: Jacob Keller <hidden>
Date: 2016-06-15 23:05:47

On Fri, Jul 17, 2015 at 11:58 AM, Garbageyard [off-list ref] wrote:
We have a pre-receive hook that checks for JIRA ID whenever someone pushes
code to Git server. I'm trying to avoid this check when someone is applying
a tag. Here's the link for the script: http://pastebin.com/VnMQp5ar

This is the link for output: http://pastebin.com/tBGmYaZF

Problem is that if i run the following command, the output that i get on
command line is 0

git describe --exact-match ac28ca721e67adc04078786164939989a5112d98 2>&1 |
grep -o fatal | wc -w

So i'm wondering why it's not entering the IF block (as confirmed in the
output link)
Probably due to environment variables set by the git hook. But.. this
is definitely not at *ALL* what you want to do. Junio has a good
explanation below. This doesn't make even any sense at all to me...
I agree this is a bad implementation (and i will change it soon) for
checking tags and can be easily done by just checking whether $refname
starts with refs/tags/ but i'll really appreciate if someone could tell me
the mistake i'm committing. I've spent few hours banging my head on this.

Ok, so the issue I believe is this:

you're running git describe on the local side. But the pre-receive
hook hasn't actually accepted the ref yet so git-describe on the
server will fail.

This is why you should just check refs/tags/* as regular pre-receive
hook examples do.

Regards,
Jake
--
View this message in context: http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git tag: pre-receive hook issue

From: Gaurav Chhabra <hidden>
Date: 2016-06-15 23:05:48

Thanks for the comments Junio/Jacob! Actually, the script was written
by someone before i came and the tag check was also done by my
colleague recently. I was also trying to implement the tag check
(using refs/tags which i did saw in few links online) but since my
colleague implemented this 'git describe' thing first and it looked
like it was working for few cases that we tried, so we left it as is.
Frankly, since everything 'seemed' to be working well so far, i never
really quite looked into it. Now i guess, it's time to correct it.


@Junio: From the example you gave, i could conclude the following:

1) : gitster garbage/master; git commit --allow-empty -m third
  [master d1f1360] third
  : gitster garbage/master; git describe --exact-match HEAD ;# third
  fatal: no tag exactly matches 'd1f1360b46dfde8c6f341e48ce45d761ed37e357'
Since after # third commit, no tag was applied to HEAD, so --exact-match resulted in fatal error
2)  : gitster garbage/master; git commit --allow-empty -m second
[master d1de78e] second
: gitster garbage/master; git tag -a -m 'v0.1' v0.1
: gitster garbage/master; git describe --exact-match HEAD^ ;# second
    v0.1
Since annotated tag was applied after # second commit, so --exact-match did referenced the commit as expected.
3) : gitster garbage/master; git commit --allow-empty -m initial
  [master (root-commit) b18cac2] initial
  : gitster garbage/master; git tag v0.0 ;# lightweight
  : gitster garbage/master; git describe --exact-match HEAD^^ ;# first
  fatal: no tag exactly matches 'b18cac237055d9518f9f92bb4c0a4dac825dce17'
In this case, it's a lightweight tag and i read today that by default, git describe only shows annotated tags (without --all or --tags). I think it's because of the missing option (--all or --tags) that it resulted in fatal error in this case.
Please correct me if i misunderstood any/all of the above cases.

My queries:
A) When you mentioned: "I am feeding three _commits_, not tags.", i
didn't really get what you're trying to highlight. Is it that the code
i shared 'incorrectly' uses 'git describe' command because it's
passing the commit ($new_sha1) associated with pushing of the tag
_instead_ of passing the commit id that the tag actually points to?

B) Coming to the earlier part of the code that you questioned. Thanks
for that. As i mentioned above, some guy had written it long time back
(few years). And again, since this never caused any issue, we never
looked into it. I did read a little about rev-list today but i think
i'll have to try it out on my machine to understand it well. Will read
more and then implement the check but yes, i do get an idea what
you've tried to question. Basically, for new branch or new
development, we are not really doing complete checks. Correct?

You've also mentioned that "And you check everything on that list.  Why?"
Was this comment for the portion where the code is validating commits (git rev-list $old_sha1..$new_sha1) for existing branch? If yes, then i didn't really get your concern. Can you kindly elaborate a bit?
And thanks for sharing the modified version! :)


@Jacob: You're right. If i understood correctly what Junio explained,
then what the code is doing really shouldn't make any sense at all. :)

By the way, you mentioned, "Ok, so the issue I believe is this: you're
running git describe on the local side. But the pre-receive hook
hasn't actually accepted the ref yet so git-describe on the server
will fail."
When i push a tag, then as per the output i shared, the commit id associated with my tag push is ac28ca721e67a. So if i do a "git describe --exact-match ac28ca721e67a", then
1) First of all, it shouldn't make any sense because as "git describe"
should accept _actual_ commit id and not the commit id generated for
tag push, isn't it?

2) If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?


@Junio/Jacob: I think i've asked quite a number of questions above but
i will really appreciate if you could spare some time to clear these
doubts. I'm definitely going to change this junk code but i would like
to be sure that i've understood your explanations well.


Thanks!

On Sat, Jul 18, 2015 at 9:30 AM, Jacob Keller [off-list ref] wrote:
On Fri, Jul 17, 2015 at 11:58 AM, Garbageyard [off-list ref] wrote:
quoted
We have a pre-receive hook that checks for JIRA ID whenever someone pushes
code to Git server. I'm trying to avoid this check when someone is applying
a tag. Here's the link for the script: http://pastebin.com/VnMQp5ar

This is the link for output: http://pastebin.com/tBGmYaZF

Problem is that if i run the following command, the output that i get on
command line is 0

git describe --exact-match ac28ca721e67adc04078786164939989a5112d98 2>&1 |
grep -o fatal | wc -w

So i'm wondering why it's not entering the IF block (as confirmed in the
output link)
Probably due to environment variables set by the git hook. But.. this
is definitely not at *ALL* what you want to do. Junio has a good
explanation below. This doesn't make even any sense at all to me...
quoted
I agree this is a bad implementation (and i will change it soon) for
checking tags and can be easily done by just checking whether $refname
starts with refs/tags/ but i'll really appreciate if someone could tell me
the mistake i'm committing. I've spent few hours banging my head on this.

Ok, so the issue I believe is this:

you're running git describe on the local side. But the pre-receive
hook hasn't actually accepted the ref yet so git-describe on the
server will fail.

This is why you should just check refs/tags/* as regular pre-receive
hook examples do.

Regards,
Jake
quoted
--
View this message in context: http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git tag: pre-receive hook issue

From: Jacob Keller <hidden>
Date: 2016-06-15 23:05:48

On Sat, Jul 18, 2015 at 1:08 PM, Gaurav Chhabra
[off-list ref] wrote:
Thanks for the comments Junio/Jacob! Actually, the script was written
by someone before i came and the tag check was also done by my
colleague recently. I was also trying to implement the tag check
(using refs/tags which i did saw in few links online) but since my
colleague implemented this 'git describe' thing first and it looked
like it was working for few cases that we tried, so we left it as is.
Frankly, since everything 'seemed' to be working well so far, i never
really quite looked into it. Now i guess, it's time to correct it.


@Junio: From the example you gave, i could conclude the following:

1) : gitster garbage/master; git commit --allow-empty -m third
  [master d1f1360] third
  : gitster garbage/master; git describe --exact-match HEAD ;# third
  fatal: no tag exactly matches 'd1f1360b46dfde8c6f341e48ce45d761ed37e357'
quoted
Since after # third commit, no tag was applied to HEAD, so --exact-match resulted in fatal error
2)  : gitster garbage/master; git commit --allow-empty -m second
[master d1de78e] second
: gitster garbage/master; git tag -a -m 'v0.1' v0.1
: gitster garbage/master; git describe --exact-match HEAD^ ;# second
    v0.1
quoted
Since annotated tag was applied after # second commit, so --exact-match did referenced the commit as expected.
3) : gitster garbage/master; git commit --allow-empty -m initial
  [master (root-commit) b18cac2] initial
  : gitster garbage/master; git tag v0.0 ;# lightweight
  : gitster garbage/master; git describe --exact-match HEAD^^ ;# first
  fatal: no tag exactly matches 'b18cac237055d9518f9f92bb4c0a4dac825dce17'
quoted
In this case, it's a lightweight tag and i read today that by default, git describe only shows annotated tags (without --all or --tags). I think it's because of the missing option (--all or --tags) that it resulted in fatal error in this case.
Please correct me if i misunderstood any/all of the above cases.

My queries:
A) When you mentioned: "I am feeding three _commits_, not tags.", i
didn't really get what you're trying to highlight. Is it that the code
i shared 'incorrectly' uses 'git describe' command because it's
passing the commit ($new_sha1) associated with pushing of the tag
_instead_ of passing the commit id that the tag actually points to?

B) Coming to the earlier part of the code that you questioned. Thanks
for that. As i mentioned above, some guy had written it long time back
(few years). And again, since this never caused any issue, we never
looked into it. I did read a little about rev-list today but i think
i'll have to try it out on my machine to understand it well. Will read
more and then implement the check but yes, i do get an idea what
you've tried to question. Basically, for new branch or new
development, we are not really doing complete checks. Correct?

You've also mentioned that "And you check everything on that list.  Why?"
quoted
Was this comment for the portion where the code is validating commits (git rev-list $old_sha1..$new_sha1) for existing branch? If yes, then i didn't really get your concern. Can you kindly elaborate a bit?
And thanks for sharing the modified version! :)


@Jacob: You're right. If i understood correctly what Junio explained,
then what the code is doing really shouldn't make any sense at all. :)

By the way, you mentioned, "Ok, so the issue I believe is this: you're
running git describe on the local side. But the pre-receive hook
hasn't actually accepted the ref yet so git-describe on the server
will fail."
quoted
When i push a tag, then as per the output i shared, the commit id associated with my tag push is ac28ca721e67a. So if i do a "git describe --exact-match ac28ca721e67a", then
1) First of all, it shouldn't make any sense because as "git describe"
should accept _actual_ commit id and not the commit id generated for
tag push, isn't it?

git describe will attempt to describe the commit ID you pass it. But
the tag object for a "new" tag push will not get "described" because
the pre-receive hook runs before you push it.

It would help a lot if we understood exactly what you are trying to accomplish.

If you run git describe locally, it will find the annotated tag you
made. If you run this remotely during the pre-receive of the tag that
you now now pushing it will not. ie:

$git tag -a some-new-tag ac28ca721e67a
$git describe --exact-match ac28ca721e67a
<succeeds and finds "some-new-tag"
$git push origin some-new-tag
<pre-receive runs>
git describe --exact-match ac28ca721e67a
<FAILS because the "some-new-tag" hasn't actually bet put into refs yet>

See the difference here? Maybe this isn't what you are trying to do at
all? What exactly behavior do you want to not allow?

Regards,
Jake
2) If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?
Not with my example above. But maybe I'm incorrect entirely in what
you are actually doing?
@Junio/Jacob: I think i've asked quite a number of questions above but
i will really appreciate if you could spare some time to clear these
doubts. I'm definitely going to change this junk code but i would like
to be sure that i've understood your explanations well.

Re: Git tag: pre-receive hook issue

From: Gaurav Chhabra <hidden>
Date: 2016-06-15 23:05:48

@Junio: So from your detailed explanation (and Jake's comment), i
understand that since my ref wasn't updated on remote so querying the
same using "git describe" resulted in failure, and hence, code was not
entering the IF block. Correct?

Also, while i was reading your replies, i was just thinking that the
following question that i asked Jake doesn't really make sense because
a commit object _is_ being passed (on local machine) to "git
describe", which is what it expects so it should work for sure:

"If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?"

@Junio: You wrote: "The part that I find questionable is that by
checking with "is this commit a tagged one?" and doing different
things. What makes the initial and the third special to deserve
checking (because they are not annotated with a tag) while skipping
the validation for the second (merely because it has an annotated tag
added to it)?"
Isn't the code that i shared doing just the opposite of what you've written? It's checking for annotated tags only and skipping the lightweight ones (although it shouldn't be doing all such things in the first place). Was it a typo on your part?

@Jake: For the question you asked: "It would help a lot if we
understood exactly what you are trying to accomplish."
I'm not sure how my colleague zeroed in on this "git describe" command but i at least know what we observed (and 'seemed' to work).  We saw that if we use git-describe and pass a commit object, it throws fatal error message. On the other hand, if we pass a tag object, it doesn't throw any fatal error. That's the reason he added that tag check portion.

@Junio/Jake: After going through all the responses that i've received
so far on this forum, i'm thinking how this nonsense code worked for
few cases in the past? When this check was put in place, devs were
getting error while pushing annotated tags. Since we use Gitolite, we
added the following to gitolite.conf and the tag push worked for them:

RW+ refs/tags=developer_name

I'm wondering why.


On Sun, Jul 19, 2015 at 3:52 AM, Jacob Keller [via git]
[off-list ref] wrote:
On Sat, Jul 18, 2015 at 1:08 PM, Gaurav Chhabra
<[hidden email]> wrote:
quoted
Thanks for the comments Junio/Jacob! Actually, the script was written
by someone before i came and the tag check was also done by my
colleague recently. I was also trying to implement the tag check
(using refs/tags which i did saw in few links online) but since my
colleague implemented this 'git describe' thing first and it looked
like it was working for few cases that we tried, so we left it as is.
Frankly, since everything 'seemed' to be working well so far, i never
really quite looked into it. Now i guess, it's time to correct it.


@Junio: From the example you gave, i could conclude the following:

1) : gitster garbage/master; git commit --allow-empty -m third
  [master d1f1360] third
  : gitster garbage/master; git describe --exact-match HEAD ;# third
  fatal: no tag exactly matches 'd1f1360b46dfde8c6f341e48ce45d761ed37e357'
quoted
Since after # third commit, no tag was applied to HEAD, so --exact-match
resulted in fatal error
2)  : gitster garbage/master; git commit --allow-empty -m second
[master d1de78e] second
: gitster garbage/master; git tag -a -m 'v0.1' v0.1
: gitster garbage/master; git describe --exact-match HEAD^ ;# second
    v0.1
quoted
Since annotated tag was applied after # second commit, so --exact-match
did referenced the commit as expected.
3) : gitster garbage/master; git commit --allow-empty -m initial
  [master (root-commit) b18cac2] initial
  : gitster garbage/master; git tag v0.0 ;# lightweight
  : gitster garbage/master; git describe --exact-match HEAD^^ ;# first
  fatal: no tag exactly matches 'b18cac237055d9518f9f92bb4c0a4dac825dce17'
quoted
In this case, it's a lightweight tag and i read today that by default,
git describe only shows annotated tags (without --all or --tags). I think
it's because of the missing option (--all or --tags) that it resulted in
fatal error in this case.
Please correct me if i misunderstood any/all of the above cases.

My queries:
A) When you mentioned: "I am feeding three _commits_, not tags.", i
didn't really get what you're trying to highlight. Is it that the code
i shared 'incorrectly' uses 'git describe' command because it's
passing the commit ($new_sha1) associated with pushing of the tag
_instead_ of passing the commit id that the tag actually points to?

B) Coming to the earlier part of the code that you questioned. Thanks
for that. As i mentioned above, some guy had written it long time back
(few years). And again, since this never caused any issue, we never
looked into it. I did read a little about rev-list today but i think
i'll have to try it out on my machine to understand it well. Will read
more and then implement the check but yes, i do get an idea what
you've tried to question. Basically, for new branch or new
development, we are not really doing complete checks. Correct?

You've also mentioned that "And you check everything on that list.  Why?"
quoted
Was this comment for the portion where the code is validating commits
(git rev-list $old_sha1..$new_sha1) for existing branch? If yes, then i
didn't really get your concern. Can you kindly elaborate a bit?
And thanks for sharing the modified version! :)


@Jacob: You're right. If i understood correctly what Junio explained,
then what the code is doing really shouldn't make any sense at all. :)

By the way, you mentioned, "Ok, so the issue I believe is this: you're
running git describe on the local side. But the pre-receive hook
hasn't actually accepted the ref yet so git-describe on the server
will fail."
quoted
When i push a tag, then as per the output i shared, the commit id
associated with my tag push is ac28ca721e67a. So if i do a "git describe
--exact-match ac28ca721e67a", then
1) First of all, it shouldn't make any sense because as "git describe"
should accept _actual_ commit id and not the commit id generated for
tag push, isn't it?

git describe will attempt to describe the commit ID you pass it. But
the tag object for a "new" tag push will not get "described" because
the pre-receive hook runs before you push it.

It would help a lot if we understood exactly what you are trying to
accomplish.

If you run git describe locally, it will find the annotated tag you
made. If you run this remotely during the pre-receive of the tag that
you now now pushing it will not. ie:

$git tag -a some-new-tag ac28ca721e67a
$git describe --exact-match ac28ca721e67a
<succeeds and finds "some-new-tag"
$git push origin some-new-tag
<pre-receive runs>
git describe --exact-match ac28ca721e67a
<FAILS because the "some-new-tag" hasn't actually bet put into refs yet>

See the difference here? Maybe this isn't what you are trying to do at
all? What exactly behavior do you want to not allow?

Regards,
Jake
quoted
2) If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?
Not with my example above. But maybe I'm incorrect entirely in what
you are actually doing?
quoted
@Junio/Jacob: I think i've asked quite a number of questions above but
i will really appreciate if you could spare some time to clear these
doubts. I'm definitely going to change this junk code but i would like
to be sure that i've understood your explanations well.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


________________________________
If you reply to this email, your message will be added to the discussion
below:
http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764p7635854.html
To unsubscribe from Git tag: pre-receive hook issue, click here.
NAML

Re: Git tag: pre-receive hook issue

From: Jacob Keller <hidden>
Date: 2016-06-15 23:05:48

On Sun, Jul 19, 2015 at 12:55 AM, Gaurav Chhabra
[off-list ref] wrote:
@Junio: So from your detailed explanation (and Jake's comment), i
understand that since my ref wasn't updated on remote so querying the
same using "git describe" resulted in failure, and hence, code was not
entering the IF block. Correct?
I assume so.
Also, while i was reading your replies, i was just thinking that the
following question that i asked Jake doesn't really make sense because
a commit object _is_ being passed (on local machine) to "git
describe", which is what it expects so it should work for sure:
It works yes. Git describe finds the nearest tag. --exact-match fails
unless it can find a tag at the commit specified.
"If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?"
only if ac28ca721e67a does not have an annotated tag associated to it


@Junio: You wrote: "The part that I find questionable is that by
checking with "is this commit a tagged one?" and doing different
things. What makes the initial and the third special to deserve
checking (because they are not annotated with a tag) while skipping
the validation for the second (merely because it has an annotated tag
added to it)?"
quoted
Isn't the code that i shared doing just the opposite of what you've written? It's checking for annotated tags only and skipping the lightweight ones (although it shouldn't be doing all such things in the first place). Was it a typo on your part?
I'm not sure what the code you have is trying to do. See below.
@Jake: For the question you asked: "It would help a lot if we
understood exactly what you are trying to accomplish."
quoted
I'm not sure how my colleague zeroed in on this "git describe" command but i at least know what we observed (and 'seemed' to work).  We saw that if we use git-describe and pass a commit object, it throws fatal error message. On the other hand, if we pass a tag object, it doesn't throw any fatal error. That's the reason he added that tag check portion.
Hmmm....
@Junio/Jake: After going through all the responses that i've received
so far on this forum, i'm thinking how this nonsense code worked for
few cases in the past? When this check was put in place, devs were
getting error while pushing annotated tags. Since we use Gitolite, we
added the following to gitolite.conf and the tag push worked for them:

RW+ refs/tags=developer_name
Sounds like you needed to add RW permissions to the refs/tags namespace.
I'm wondering why.
Ok, so normally, pre-receive hook is used to implement policy. Ie:
prevent acceptance of pushes that have "bad" content as defined by the
repository owner. For example, preventing push of tags that don't
match some format, or preventing pushes which contain bad stuff.

I could provide some examples or suggestions if you would describe
what sort of policy you're trying to enforce..

git describe will tell you if the commit you're passing it is
associated with an annotated tag. I do not understand who this
information can help you implement any policy, so understanding what
the policy you want is would be the most helpful.

I can't really help more or understand exactly what you were doing
without understanding what policy you were/are trying to implement.

The thing your code is doing today is something like:

for each reference update, locate every commit

for each commit in this reference update, check to see if it already
has an associated tag connected to it.

If it doesn't have a tag, then "do some more checks" which are not
described here.

This doesn't make sense to me at all. I think what you *meant* was this:

for each reference update, if the reference being updated is a tag, skip it

otherwise, for each commit in the reference update do some checks on it.

That is *completely* different from the code you've written today.

Regards,
Jake

Re: Git tag: pre-receive hook issue

From: Gaurav Chhabra <hidden>
Date: 2016-06-15 23:05:48

The only thing we wanted to check was whether a ref is a tag. :) Rest
other things are working fine (except for the "commits=$new_sha1"
thing which Junio already pointed out and corrected). I will correct
the pre-receive hook.

The only mystery that remains is about the current nonsensical code
working fine in the past for few annotated tag pushes. It shouldn't
have worked just by providing:

RW+ refs/tags=developer_name

Ref: http://gitolite.com/gitolite/g2/aac.html (Section: "deny" rules
for refs in a repo)


On Sun, Jul 19, 2015 at 2:09 PM, Jacob Keller [via git]
[off-list ref] wrote:
On Sun, Jul 19, 2015 at 12:55 AM, Gaurav Chhabra
<[hidden email]> wrote:
quoted
@Junio: So from your detailed explanation (and Jake's comment), i
understand that since my ref wasn't updated on remote so querying the
same using "git describe" resulted in failure, and hence, code was not
entering the IF block. Correct?
I assume so.
quoted
Also, while i was reading your replies, i was just thinking that the
following question that i asked Jake doesn't really make sense because
a commit object _is_ being passed (on local machine) to "git
describe", which is what it expects so it should work for sure:
It works yes. Git describe finds the nearest tag. --exact-match fails
unless it can find a tag at the commit specified.
quoted
"If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?"
only if ac28ca721e67a does not have an annotated tag associated to it


quoted
@Junio: You wrote: "The part that I find questionable is that by
checking with "is this commit a tagged one?" and doing different
things. What makes the initial and the third special to deserve
checking (because they are not annotated with a tag) while skipping
the validation for the second (merely because it has an annotated tag
added to it)?"
quoted
Isn't the code that i shared doing just the opposite of what you've
written? It's checking for annotated tags only and skipping the lightweight
ones (although it shouldn't be doing all such things in the first place).
Was it a typo on your part?
I'm not sure what the code you have is trying to do. See below.
quoted
@Jake: For the question you asked: "It would help a lot if we
understood exactly what you are trying to accomplish."
quoted
I'm not sure how my colleague zeroed in on this "git describe" command
but i at least know what we observed (and 'seemed' to work).  We saw that if
we use git-describe and pass a commit object, it throws fatal error message.
On the other hand, if we pass a tag object, it doesn't throw any fatal
error. That's the reason he added that tag check portion.
Hmmm....
quoted
@Junio/Jake: After going through all the responses that i've received
so far on this forum, i'm thinking how this nonsense code worked for
few cases in the past? When this check was put in place, devs were
getting error while pushing annotated tags. Since we use Gitolite, we
added the following to gitolite.conf and the tag push worked for them:

RW+ refs/tags=developer_name
Sounds like you needed to add RW permissions to the refs/tags namespace.
quoted
I'm wondering why.
Ok, so normally, pre-receive hook is used to implement policy. Ie:
prevent acceptance of pushes that have "bad" content as defined by the
repository owner. For example, preventing push of tags that don't
match some format, or preventing pushes which contain bad stuff.

I could provide some examples or suggestions if you would describe
what sort of policy you're trying to enforce..

git describe will tell you if the commit you're passing it is
associated with an annotated tag. I do not understand who this
information can help you implement any policy, so understanding what
the policy you want is would be the most helpful.

I can't really help more or understand exactly what you were doing
without understanding what policy you were/are trying to implement.

The thing your code is doing today is something like:

for each reference update, locate every commit

for each commit in this reference update, check to see if it already
has an associated tag connected to it.

If it doesn't have a tag, then "do some more checks" which are not
described here.

This doesn't make sense to me at all. I think what you *meant* was this:

for each reference update, if the reference being updated is a tag, skip it

otherwise, for each commit in the reference update do some checks on it.

That is *completely* different from the code you've written today.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


________________________________
If you reply to this email, your message will be added to the discussion
below:
http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764p7635875.html
To unsubscribe from Git tag: pre-receive hook issue, click here.
NAML

Re: Git tag: pre-receive hook issue

From: Jacob Keller <hidden>
Date: 2016-06-15 23:05:49

To check whether the ref being updated is a tag, you need to check the
3rd parameter. pre-receive receives in the format

<old-value> <new-value> <ref-name>

so you need to check each line's 3rd value which is the ref-name being
updated. If it's in refs/tags then it's a tag update. If it's not, you
can check it as a branch update.

Then you should check all new commits for each branch, as Julio mentioned above.

Checking whether each commit has an associated tag is probably not
what you meant.

Regards,
Jake

On Sun, Jul 19, 2015 at 3:13 AM, Gaurav Chhabra
[off-list ref] wrote:
The only thing we wanted to check was whether a ref is a tag. :) Rest
other things are working fine (except for the "commits=$new_sha1"
thing which Junio already pointed out and corrected). I will correct
the pre-receive hook.

The only mystery that remains is about the current nonsensical code
working fine in the past for few annotated tag pushes. It shouldn't
have worked just by providing:

RW+ refs/tags=developer_name

Ref: http://gitolite.com/gitolite/g2/aac.html (Section: "deny" rules
for refs in a repo)


On Sun, Jul 19, 2015 at 2:09 PM, Jacob Keller [via git]
[off-list ref] wrote:
quoted
On Sun, Jul 19, 2015 at 12:55 AM, Gaurav Chhabra
<[hidden email]> wrote:
quoted
@Junio: So from your detailed explanation (and Jake's comment), i
understand that since my ref wasn't updated on remote so querying the
same using "git describe" resulted in failure, and hence, code was not
entering the IF block. Correct?
I assume so.
quoted
Also, while i was reading your replies, i was just thinking that the
following question that i asked Jake doesn't really make sense because
a commit object _is_ being passed (on local machine) to "git
describe", which is what it expects so it should work for sure:
It works yes. Git describe finds the nearest tag. --exact-match fails
unless it can find a tag at the commit specified.
quoted
"If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?"
only if ac28ca721e67a does not have an annotated tag associated to it


quoted
@Junio: You wrote: "The part that I find questionable is that by
checking with "is this commit a tagged one?" and doing different
things. What makes the initial and the third special to deserve
checking (because they are not annotated with a tag) while skipping
the validation for the second (merely because it has an annotated tag
added to it)?"
quoted
Isn't the code that i shared doing just the opposite of what you've
written? It's checking for annotated tags only and skipping the lightweight
ones (although it shouldn't be doing all such things in the first place).
Was it a typo on your part?
I'm not sure what the code you have is trying to do. See below.
quoted
@Jake: For the question you asked: "It would help a lot if we
understood exactly what you are trying to accomplish."
quoted
I'm not sure how my colleague zeroed in on this "git describe" command
but i at least know what we observed (and 'seemed' to work).  We saw that if
we use git-describe and pass a commit object, it throws fatal error message.
On the other hand, if we pass a tag object, it doesn't throw any fatal
error. That's the reason he added that tag check portion.
Hmmm....
quoted
@Junio/Jake: After going through all the responses that i've received
so far on this forum, i'm thinking how this nonsense code worked for
few cases in the past? When this check was put in place, devs were
getting error while pushing annotated tags. Since we use Gitolite, we
added the following to gitolite.conf and the tag push worked for them:

RW+ refs/tags=developer_name
Sounds like you needed to add RW permissions to the refs/tags namespace.
quoted
I'm wondering why.
Ok, so normally, pre-receive hook is used to implement policy. Ie:
prevent acceptance of pushes that have "bad" content as defined by the
repository owner. For example, preventing push of tags that don't
match some format, or preventing pushes which contain bad stuff.

I could provide some examples or suggestions if you would describe
what sort of policy you're trying to enforce..

git describe will tell you if the commit you're passing it is
associated with an annotated tag. I do not understand who this
information can help you implement any policy, so understanding what
the policy you want is would be the most helpful.

I can't really help more or understand exactly what you were doing
without understanding what policy you were/are trying to implement.

The thing your code is doing today is something like:

for each reference update, locate every commit

for each commit in this reference update, check to see if it already
has an associated tag connected to it.

If it doesn't have a tag, then "do some more checks" which are not
described here.

This doesn't make sense to me at all. I think what you *meant* was this:

for each reference update, if the reference being updated is a tag, skip it

otherwise, for each commit in the reference update do some checks on it.

That is *completely* different from the code you've written today.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


________________________________
If you reply to this email, your message will be added to the discussion
below:
http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764p7635875.html
To unsubscribe from Git tag: pre-receive hook issue, click here.
NAML
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git tag: pre-receive hook issue

From: Gaurav Chhabra <hidden>
Date: 2016-06-15 23:05:49

Hi Jake,

Thanks about the refs/tags check. I’m aware about this. Junio also
explained it in one of his replies. I was actually confused why my
current code was working in past for few of the annotated tags.
Anyways, now that I have clarity about the mistake in the code, I
guess, I’ll figure it out myself.

@Junio: Thanks a lot for your detailed explanation about the ‘behind
the scenes’ activity during a push process. That was definitely
helpful and will help me in future too.

@Jake: Thanks for your help and your patience in explaining things.



On Mon, Jul 20, 2015 at 5:05 AM, Jacob Keller [via git]
[off-list ref] wrote:
To check whether the ref being updated is a tag, you need to check the
3rd parameter. pre-receive receives in the format

<old-value> <new-value> <ref-name>

so you need to check each line's 3rd value which is the ref-name being
updated. If it's in refs/tags then it's a tag update. If it's not, you
can check it as a branch update.

Then you should check all new commits for each branch, as Julio mentioned
above.

Checking whether each commit has an associated tag is probably not
what you meant.

Regards,
Jake

On Sun, Jul 19, 2015 at 3:13 AM, Gaurav Chhabra
<[hidden email]> wrote:
quoted
The only thing we wanted to check was whether a ref is a tag. :) Rest
other things are working fine (except for the "commits=$new_sha1"
thing which Junio already pointed out and corrected). I will correct
the pre-receive hook.

The only mystery that remains is about the current nonsensical code
working fine in the past for few annotated tag pushes. It shouldn't
have worked just by providing:

RW+ refs/tags=developer_name

Ref: http://gitolite.com/gitolite/g2/aac.html (Section: "deny" rules
for refs in a repo)


On Sun, Jul 19, 2015 at 2:09 PM, Jacob Keller [via git]
<[hidden email]> wrote:
quoted
On Sun, Jul 19, 2015 at 12:55 AM, Gaurav Chhabra
<[hidden email]> wrote:
quoted
@Junio: So from your detailed explanation (and Jake's comment), i
understand that since my ref wasn't updated on remote so querying the
same using "git describe" resulted in failure, and hence, code was not
entering the IF block. Correct?
I assume so.
quoted
Also, while i was reading your replies, i was just thinking that the
following question that i asked Jake doesn't really make sense because
a commit object _is_ being passed (on local machine) to "git
describe", which is what it expects so it should work for sure:
It works yes. Git describe finds the nearest tag. --exact-match fails
unless it can find a tag at the commit specified.
quoted
"If i got the above right, then shouldn't Git throw an error even on
my local machine when i'm running "git describe --exact-match
ac28ca721e67a"?"
only if ac28ca721e67a does not have an annotated tag associated to it


quoted
@Junio: You wrote: "The part that I find questionable is that by
checking with "is this commit a tagged one?" and doing different
things. What makes the initial and the third special to deserve
checking (because they are not annotated with a tag) while skipping
the validation for the second (merely because it has an annotated tag
added to it)?"
quoted
Isn't the code that i shared doing just the opposite of what you've
written? It's checking for annotated tags only and skipping the
lightweight
ones (although it shouldn't be doing all such things in the first
place).
Was it a typo on your part?
I'm not sure what the code you have is trying to do. See below.
quoted
@Jake: For the question you asked: "It would help a lot if we
understood exactly what you are trying to accomplish."
quoted
I'm not sure how my colleague zeroed in on this "git describe" command
but i at least know what we observed (and 'seemed' to work).  We saw
that if
we use git-describe and pass a commit object, it throws fatal error
message.
On the other hand, if we pass a tag object, it doesn't throw any fatal
error. That's the reason he added that tag check portion.
Hmmm....
quoted
@Junio/Jake: After going through all the responses that i've received
so far on this forum, i'm thinking how this nonsense code worked for
few cases in the past? When this check was put in place, devs were
getting error while pushing annotated tags. Since we use Gitolite, we
added the following to gitolite.conf and the tag push worked for them:

RW+ refs/tags=developer_name
Sounds like you needed to add RW permissions to the refs/tags namespace.
quoted
I'm wondering why.
Ok, so normally, pre-receive hook is used to implement policy. Ie:
prevent acceptance of pushes that have "bad" content as defined by the
repository owner. For example, preventing push of tags that don't
match some format, or preventing pushes which contain bad stuff.

I could provide some examples or suggestions if you would describe
what sort of policy you're trying to enforce..

git describe will tell you if the commit you're passing it is
associated with an annotated tag. I do not understand who this
information can help you implement any policy, so understanding what
the policy you want is would be the most helpful.

I can't really help more or understand exactly what you were doing
without understanding what policy you were/are trying to implement.

The thing your code is doing today is something like:

for each reference update, locate every commit

for each commit in this reference update, check to see if it already
has an associated tag connected to it.

If it doesn't have a tag, then "do some more checks" which are not
described here.

This doesn't make sense to me at all. I think what you *meant* was this:

for each reference update, if the reference being updated is a tag, skip
it

otherwise, for each commit in the reference update do some checks on it.

That is *completely* different from the code you've written today.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


________________________________
If you reply to this email, your message will be added to the discussion
below:

http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764p7635875.html
To unsubscribe from Git tag: pre-receive hook issue, click here.
NAML
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


________________________________
If you reply to this email, your message will be added to the discussion
below:
http://git.661346.n2.nabble.com/Git-tag-pre-receive-hook-issue-tp7635764p7635968.html
To unsubscribe from Git tag: pre-receive hook issue, click here.
NAML

Re: Git tag: pre-receive hook issue

From: "Keller, Jacob E" <jacob.e.keller@intel.com>
Date: 2016-06-15 23:05:50

On Mon, 2015-07-20 at 13:13 +0530, Gaurav Chhabra wrote:
Hi Jake,

Thanks about the refs/tags check. I’m aware about this. Junio also
explained it in one of his replies. I was actually confused why my
current code was working in past for few of the annotated tags.
Anyways, now that I have clarity about the mistake in the code, I
guess, I’ll figure it out myself.

@Junio: Thanks a lot for your detailed explanation about the ‘behind
the scenes’ activity during a push process. That was definitely
helpful and will help me in future too.

@Jake: Thanks for your help and your patience in explaining things.
No problem. I'm also not certain why it would have worked in some
cases, so that is definitely confusing, but at least you can get it
sorted to do what you intend. Best of luck!

Regards,
Jake

Re: Git tag: pre-receive hook issue

From: Jakub Narębski <hidden>
Date: 2016-06-15 23:05:51

On 2015-07-19, Jacob Keller wrote:
git describe will tell you if the commit you're passing it is
associated with an annotated tag. I do not understand who this
information can help you implement any policy, so understanding what
the policy you want is would be the most helpful.
One policy I can think of that may have use of checking if commit
is tagged is requiring some extra restrictions on the commit that
is being tagged, for example that the only file that it can touch
is version.h / VERSION-FILE, and no code changes at all.

-- 
Jakub Narębski
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help