Git: do not refetch tags and refactor

[gracinet]

Follow-up after PR #78 (see there for more details)

• we'd like not to query remotes if refspec is a locally known tag, with an option to force the querying
• the logic to avoid remote queries needs to be refactored.

Not sure at this point if a potential integration with git-aggregator (see PR #79) would make this obsolete or not

[leorochael]

Hi, sorry I couldn't reply earlier directly to the original PR (#78)

There is one simplification/refactor that is possible with the code I contributed:

The git cat-file command accepts a parameter like --batch-check='%(objecttype) %(objectname)' that will then expect references to be fed on the stdin and outputs the type and hash in a single line.

With this we could rewrite get_local_hash_for_ref() to use this invocation of git cat-file and then rewrite self.has_commit(ref) as:

return self.get_local_hash_for_ref(ref) is not None

Regarding refactoring, tags and remote queries: using the git show-ref, is the "local" equivalent of git ls-remote, and could be parsed identically to know if a reference is a locally known tag or branch. This would allow us to skip doing a remote query for tags in the absence of a, say, force-refresh or force-tag-refresh parameter.

However while looking into this, it would be nice if anybox.recipe.odoo would skip doing a remote query if the reference is a branch and buidout:newest = false (i.e. if buildout is invoked with the -N parameter).

So, I would suggest the following logic for checking out a reference:

• Check reference against git show-ref.
• Immediately checkout without a remote query and then stop if:
  • The ref is a locally known tag and the force... parameter is not set
  • The ref is a locally known branch and buildout:newest == "false"
• Use git cat-file to know if ref is a locally known hash (i.e. what PR Skip querying remote ref for locally known hash #78 did):
  • if yes, checkout without a remote query and stop
• Proceed like it's currently done now (i.e. with git ls-remote).

I would also suggest, for the next major version, that when a hash is used directly as ref, then the branch option must be used. It could contain the value of a tag instead of a branch, but it must be provided so that a naked git fetch is not necessary and we could check that the requested hash is actually inside the log of the reference branch/tag.

[gracinet]

Hi, thanks for the tips, I certainly agree with the expected behaviour.

One reason to call for refactors is that in my mind (even after I refreshed my memory of the code base while reviewing your changes), not querying on locally known SHAs was already supposed to work, and that's the job of has_commit(): query_remote_ref shouldn't even be called.

About -N, yes it's interesting, and if that's what most buildout users would expect, let's go for it.
By the way, freeze/extract should force its use, their are some rare yet known corner cases where a surprise can happen with python libraries.

PS: I suppose you can't comment on #78 because merging closed it.

[leorochael]

PS: I suppose you can't comment on #78 because merging closed it.

Actually, it looks like I can still comment there. I just meant that I didn't get around to replying to you before you closed it, so it felt more appropriate to continue the discussion here instead of there.

[gracinet]

@leorochael you're right, it's better to discuss this in an open issue.
PS: don't be surprised, I'll be mostly inactive till mid-september after b1 release.