RFR: Save references to materialized branches locally
Erik Helin
ehelin at openjdk.java.net
Wed Nov 27 10:25:47 UTC 2019
On Wed, 27 Nov 2019 10:18:37 GMT, Robin Westberg <rwestberg at openjdk.org> wrote:
> On Wed, 27 Nov 2019 10:13:57 GMT, Erik Helin <ehelin at openjdk.org> wrote:
>
>> On Wed, 27 Nov 2019 09:41:46 GMT, Robin Westberg <rwestberg at openjdk.org> wrote:
>>
>>> On Wed, 27 Nov 2019 09:37:28 GMT, Erik Helin <ehelin at openjdk.org> wrote:
>>>
>>>> On Tue, 26 Nov 2019 13:00:47 GMT, Robin Westberg <rwestberg at openjdk.org> wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> Please review this change that ensure that when bots use `Repository.materialize`, they always save a named reference to what was fetched. This ensures that subsequent fetches will present these existing refs, and avoid re-fetching commits that already exist locally.
>>>>>
>>>>> It also fixes a potential bug where the mlbridge archive reader could have fetched from an incorrect branch.
>>>>>
>>>>> Best regards,
>>>>> Robin
>>>>>
>>>>> ----------------
>>>>>
>>>>> Commits:
>>>>> - 280c73b8: Use configurable ref for the mlbridge archive
>>>>> - 6b707ae4: Specify a destination ref when using materialize
>>>>>
>>>>> Changes: https://git.openjdk.java.net/skara/pull/271/files
>>>>> Webrev: https://webrevs.openjdk.java.net/skara/271/webrev.00
>>>>> Stats: 54 lines in 14 files changed: 28 ins; 0 del; 26 mod
>>>>> Patch: https://git.openjdk.java.net/skara/pull/271.diff
>>>>> Fetch: git fetch https://git.openjdk.java.net/skara pull/271/head:pull/271
>>>>
>>>> bots/hgbridge/src/main/java/org/openjdk/skara/bots/hgbridge/ExporterConfig.java line 172:
>>>>
>>>>> 171: var localRepo = Repository.materialize(scratchPath, configurationRepo.url(),
>>>>> 172: configurationRef + ":hgbridge_config_" + configurationRepo.name());
>>>>> 173:
>>>>
>>>> Why no `+` here to to imply `--force` to `git fech`?
>>>
>>> No reason, good catch!
>>
>> Should we instead change `Repository.materialize` to always prepend the `+` to the refspec? Or do you think that makes it less clear at the call sites?
>
> Not sure, it's something you only want if you have a target ref, and only if you are using git.. And since materialize is implemented as a default method in Repository, it might be a little odd to have that logic there. Perhaps it could be refactored to not be a default method though.
Ah right. No, `Repository.materialize` is `static`, not `default` I think we should leave as it is then.
The only other approach I can think if something like `Repository.materalize(Path path, URI remote, String fromRef, String toRef, boolean force, boolean checkout`), but then the parameter list is becoming really long (and `hg` can't handle `fromRef != toRef` anyhow). So I think we just leave it as it is.
PR: https://git.openjdk.java.net/skara/pull/271
More information about the skara-dev
mailing list