eclipse-oomph / oomph

Eclipse Public License 2.0
6 stars 9 forks source link

Setting multi-valued keys in git clone task #68

Open joeshannon opened 7 months ago

joeshannon commented 7 months ago

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: ![image](https://github.com/eclipse-oomph/oomph/assets/35498448/32b23012-2f24-428d-8be7-cd18e5a76581) 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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

I was trying to reproduce the problem like this:

image

But the multi-valued entry is populated:

image

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:

image

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 commented 7 months ago

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 commented 2 months ago

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 commented 2 months ago

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:)[^/]*(?&lt;!/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! 🚀 💪