On Tue, Jun 07, 2016 at 05:17:07PM -0400, Jeff King wrote: > That is much more flexible, as they could even do some more complicated > matching than a single string (though in practice, for security things, > I think simpler is better). > > I think this option is going to become a blueprint for other "extended" > checks, too. E.g., you might also want to check that the tagger ident > matches the uid on the signing key. > > My main worry is that we'll accrue a whole bunch of such logic. And even > though each one is relatively simple, it would be nice for callers to be > able to ask us to just do the standard safety checks.
I agree with this. I can't think of other checks off the top of my head, but I wouldn't be surprised if this is the case. I think that having custom flags for each check can also derive in each package manager/user picking each check based on many different rationales, which might lead to people overcomplicating things? > > If we do go with the "print it out and let the caller do their own > checks" strategy, I think I'd prefer rather than "--show-tagname" to > just respect the "--format" we use for tag-listing. That would let you > do: > > git tag -v --format='%(tag)%n%(tagger)' > > or similar. In fact you can already do that with a separate step (modulo > %n, which we do not seem to understand here), but like your example: It worries me that, in this case, the patches for upstream managers might be harder to integrate/pitch for users. Also, maybe we could take both strategies? add a --check-name for verify-tag and a --format for tag -v (I think either change is easy enough to do). > > > Or it could even do this: > > > > tag="$1" > > if ! git tag -v "$tag" > > if ! git tag -v "$tag" > > then > > echo >&2 "Bad tag." > > exit 1 > > fi > > + tag=$(git tag --show-tagname $tag) > > make dest=/usr/local/$package/$tag install > > It is racy. That probably doesn't matter for most callers, but it would > be nice to be able to get a custom format out of the "-v" invocation. Oh yeah, I didn't consider this either. I also don't think it's such an issue, but it sounds like a good idea not to have these races. > > -Peff Thanks! -Santiago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
