Setting multi-valued keys in git clone task

[joeshannon]

Discussed in https://github.com/orgs/eclipse-oomph/discussions/67

^{Originally posted by joeshannon February 13, 2024}
Hi,
We have configuration in our git clone tasks to add additional ref spec to a particular origin (for Gerrit purposes), it looks like:

A couple of weeks ago developers started noticing that the "Fetch from Gerrit..." option was not available anymore for newly provisioned workspaces. In the new git clones, only the first fetch refspec is added to the git repository config file instead of both of them. To be explicit, the relevant section in the config file looks like:

[remote "origin"]
	url = ssh://gerrit:29418/repo
	fetch = +refs/heads/*:refs/remotes/origin/*
	pushurl = ssh://gerrit:29418/repo
	push = HEAD:refs/for/main

Instead of:

[remote "origin"]
	url = ssh://gerrit:29418/repo
	fetch = +refs/heads/*:refs/remotes/origin/*
	fetch = refs/notes/*:refs/notes/*
	pushurl = ssh://gerrit:29418/repo
	push = HEAD:refs/for/main

I noted that there have been some changes to the git Oomph plugins recently so was wondering how we might achieve the previous behaviour?

[joeshannon]

Just had a little look at the change in more detail. The enhancement added two new parameters "force" and "recursive" to the configuration entries.

Using the "force" parameter does appear to restore the previous behaviour.

I'm not sure where that leaves this issue. I can't think of a better term than "force" but at the same time it's not entirely clear what the implications are especially for configs such as the fetch refspecs.

Perhaps this is a non-issue then, unless it is likely to affect many users that the default behaviour has changed?

[merks]

Thanks! I’ll have a closer look too. The idea for force is to add/change the property only if not already present. Perhaps multi properties are handled poorly in this regard.

[merks]

I was trying to reproduce the problem like this:

But the multi-valued entry is populated:

It looks like probably you are trying to update the confirmation at the point in time when there is already a fetch property and indeed in that case an additional fetch config property will not be added except when there is a force.

The design intent is that one can specify properties in such a way that they do not change the user's configuration if the user has configured that already or differently. Also the implementation was changed (improved) so that changes to the configuration will be applied by the git clone task when next performed (and makes it need to perform) whereas previously such changes were applied only when the clone is first created. In addition, a new task was created that allows the user to apply configuration changes to arbitrary git clone tasks defined in setups, e.g., the following allows me to create an additional remote for my fork of some other arbitrary GitHub repository:

Here I don't want to force the url property on the off chance that my fork's name is not the same as the original repository's name, e.g., if I clone such .github repository which each Eclipse GitHub organization has.

This is a long winded way of saying that I think it is working correctly as designed and it is an unfortunate side-effect that you now need to set the force property to get the old behavior. I suppose I could change the default value to true, but I think it's more typically best to respect a user's existing configuration.

What do you think?

[joeshannon]

Thank you for the thorough assessment, and yes, I observe the same behaviour as above for an arbitrarily named property.

To some extent it could be argued that certain properties should be considered as not already set if the configuration is applied to a git clone task (rather than applied by the new git configuration task) as they haven't been specified explicitly in any Oomph setup task, but implicitly by EGit upon clone. However I don't know if this would provide a more intuitive or consistent approach overall (and it might not even be possible to implement).

The design description you have outlined above does make sense and I agree with the intent.

Possibly if there are some docs/hints/tips for the clone task it might be helpful to include something about these implicit/default Git/EGit config keys that will now require use of force?

[merks]

Somewhere I should document this handy capability to augment each clone task such that it creates an additional remote for your fork of the repository being cloned. Use Navigate -> Open Setup -> User, copy the text below and paste it to the User node:

<?xml version="1.0" encoding="UTF-8"?>
<git:GitConfigurationTask
    xmi:version="2.0"
    xmlns:xmi="http://www.omg.org/XMI"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:git="http://www.eclipse.org/oomph/setup/git/1.0"
    xsi:schemaLocation="http://www.eclipse.org/oomph/setup/git/1.0 https://raw.githubusercontent.com/eclipse-oomph/oomph/master/setups/models/Git.ecore"
    remoteURIPattern="(?:https://github.com/|git@github.com:)[^/]*(?&lt;!merks)(/.*)">
  <configSections
      name="remote">
    <subsections
        name="merks">
      <properties
          key="fetch"
          value="+refs/heads/*:refs/remotes/merks/*"
          force="true"/>
      <properties
          key="url"
          value="git@github.com:merks$1"/>
    </subsections>
  </configSections>
</git:GitConfigurationTask>

Of course you should change "merks" to your GitHub account id.

[fedejeanne]

To also match the HTTPS URL (in case you're not cloning with SSH), use this remoteURIPattern instead (shamelessly taken from @merks E-Mail 😁 ) which will match both HTTPS and SSH.

remoteURIPattern="(?:https://(?:merks@)?github.com/|git@github.com:)[^/]*(?<!/merks)(/.*)"

And if you want to use HTTPS instead of SSH in your remote, replace the url property:

<properties
          key="url"
          value="https://github.com/merks$1"/>

As usual: replace merks with your own GitHub account ID.

Thank you once again for the help, Ed! 🚀 💪

[Mark-J-Lawrence]

Thanks for reporting this one, my team encountered it and updating and using force has solved it

[merks]

I dislike when new features break old things, but sometimes that's challenging. ☹

[Mark-J-Lawrence]

I've had a comment about the behaviour of this from one of the users in my team, I'm wondering if there's a way of solving this.

The fetch specs in Oomph look like this:

we were broke previously when the mentioned changes above went in, so I changed to force=true. This then solved the problem...till now.

The trouble is, users are adding their own fetch specs on-top of the oomph provided ones, and they are being removed each time the oomph tasks run, which isn't desirable behaviour.
Switching back to force=false then stops the oomph fetch specs ever being applied, even for a fresh install because a fetch spec for main already exists by default.

I'm half tempted to say it might be considered a bug, because in Oomph, each fetch spec has its own force & recursive setting, so you'd expect it to work on individual fetch spec entries. So even if all are set to force=true any additional ones are left untouched when re-syncing?