Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork944
Commit5b2869f
committed
Fix old Commit_ish annotations in git.remote
The combination of changes here is a bit unintuitive.- In PushInfo.__init__, the old_commit parameter is an optional string, whose value is bound to the _old_commit_sha instance attribute. The old_commit property attempts to create a Commit object using this string. When _old_commit_sha is None, this property returns None. Otherwise it calls Repo.commit on the Repo object for the PushInfo's associated remote. Repo.commit should return, and is annotated to return, a Commit object. Its return value is produced by calling rev_parse, which is actually the implementation in git.fun, which always returns an Object (and more specifically an AnyGitObject) and whose annotation was recently fixed infe7f9f2. The way rev_parse is used appears to only be able to get a Commit, but even if it were to get something else, it would still be an Object and not a SymbolicReference or string. Therefore, the return annotation, which contained the old Commit_ish, was overly broad, in part because the old Commit_ish was defined over-broadly, but also in other ways. This changes it to declare that it returns a Commit object or None, not allowing any kind of refs, strings, or instances representing git objects other than commits. The new return annotation is also what type checkers infer for the operand of the return statement, ever since git.fun.rev_parse's own return annotation was fixed.- In contrast, the situation with FetchInfo is almost the opposite. FetchInfo.__init__ had declared its old_commit parameter as being of the old Commit_ish type or None. Given the name old_commit, this may seem at first glance to be a case where only actually commit-ish values should be accepted, such that the annotation may have been overly broad due the overbroad old definition of Commit_ish. But on closer inspection it seems that may not be so. Specifically, when "git fetch" reports the refs it updated, and a ref points to something that is not commit-ish, the "old_commit" can be something that is not a commit. In particular, if a remote lightweight tag points to a blob or tree (rather than the usual case of pointing to a commit or tag), and that tag is then changed, then a fetch -- if done with flags that allow it to be reset -- should result in an "old_commit" of the blob or tree. More specifically, in FetchInfo._from_line (in the "tag update" case), in a line with ".." or "...", old_commit is set by parsing a field with repo.rev_parse, which is git.fun.rev_parse, which will return an Object that may be any of the four types. This is later passed as the old_commmit parameter when constructing a FetchInfo instance. Since a lightweight tag is a ref that can refer to any of the four types, any could end up being parsed into the old_commit attribute. This therefore keeps those as broad as before, channging their old Commit_ish annotations to AnyGitObject rather than to the newer Commit_ish type or anything narrower.Note also that it is pure coincidence that entities namedold_commit were temporarily annotated as Old_commit_ish. (Thelatter, as noted in04a2753, is just what the old Commit_ish typewas temporarily renamed to.)1 parentb4b6e1e commit5b2869f
1 file changed
+4
-4
lines changedLines changed: 4 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
44 | | - | |
| 44 | + | |
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| |||
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
197 | | - | |
| 197 | + | |
198 | 198 | | |
199 | 199 | | |
200 | 200 | | |
| |||
360 | 360 | | |
361 | 361 | | |
362 | 362 | | |
363 | | - | |
| 363 | + | |
364 | 364 | | |
365 | 365 | | |
366 | 366 | | |
| |||
436 | 436 | | |
437 | 437 | | |
438 | 438 | | |
439 | | - | |
| 439 | + | |
440 | 440 | | |
441 | 441 | | |
442 | 442 | | |
| |||
0 commit comments
Comments
(0)