feat(config): Support presets in subdirectories

[ArmaanT]

Changes:

This PR adds the ability to extend a config preset that exists within an arbitrary path inside another repo. The new syntax is: github>user/repo//path/to/file which will load path/to/file.json within the user/repo repository.

This new syntax is supported on github, gitlab, gitea, and local (when on one of those platforms)

Context:

Closes #8674

Documentation (please check one with an [x])

• I have updated the documentation, or
• No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

• Code inspection only, or
• Newly added unit tests, or
• No new tests but ran on a real repository, or
• Both unit tests + ran on a real repository

Here's the real repository I tested with. Here's a PR that was generated using an external preset config: ArmaanT/testbed#8.

[viceice]

@rarkins I'm not sure if this is what we want. Currently we use the preset :name as filename and we don't support sub-presets on those hosted presets. So why we don't simply use the all after the : as path. Which needs to have an extension then, to support json5 or later maybe yaml

[rarkins]

Currently we use the preset :name as filename and we don't support sub-presets on those hosted presets.

github>foo/bar:fruit/banana today already has a meaning: the file fruit.json and subobject banana. So I don't think we can use it.

I think we can keep our convention of "no file extension implies .json" both for these and others.

[viceice]

so parse as follow?

• github>abc/foo//path/xyz:abc -> https://github.com/abc/foo and path/xyz/abc.json
• github>abc/foo//path/xyz -> https://github.com/abc/foo and path/xyz/default.json (path/xyz/renovate.json fallback)

[ArmaanT]

I think forcing a separation between the path and filename with : feels a bit unintuitive. Maybe we could use a trailing slash to signify we want to use default.json?
github>abc/foo//path/xyz -> path/xyz.json
github>abc/foo//path/xyz/ -> path/xyz/default.json (path/xyz/renovate.json)

[rarkins]

We need to maintain backwards compatibility at all costs here

[rarkins]

Once we are out of the root I think that having a filename specified as mandatory is an OK restriction

[viceice]

so again, we need to decide, what happens when github>abc/foo//path/xyz:abc is used. or even github>abc/foo//path/xyz:abc/def

is abc/def only a sub-preset path?

[viceice]

I think best is to split this github>abc/foo//path/xyz:abc/def as follows:

• repo: https://github.com/abc/foo
• presetName: path/xyz
• sub-preset: abc/def

[ArmaanT]

Right now the // syntax doesn't have any support for using : or subpresets. I have a slight preference to leaving it as-is (so that subdirectories are mutually exclusive with subpresets) because it offers new functionality without breaking the existing : subpreset parsing when not using a subdirectory.

I think the above idea could cause some confusion because it would overload the meaning of :abc/def where if there is no subdirectory, it loads abc.json with def subpreset, but if you're using the subdirectory syntax it would mean the abc/def subpreset within xyz.json

[rarkins]

I think best is to split this github>abc/foo//path/xyz:abc/def as follows:

• repo: https://github.com/abc/foo
• presetName: path/xyz
• sub-preset: abc/def

If it makes a difference for us, we could argue that when using paths, you don't need sub-presets.

We also have documented github>abc/foo/:xyz as follows:

• repo: https://github.com/abc/foo
• presetName: xyz
• sub-preset: none

And I've got a feeling that github>abc/foo/:xyz/def works as follows:

• repo: https://github.com/abc/foo
• presetName: xyz
• sub-preset:def

In fact I think it's this above one that has caused us to consider the // divider?

[rarkins]

Do we think that "no sub-presets if subdirectories are in use" is a reasonable limitation?

[viceice]

I'm ok with no sub-preset support. But we should throw an config error if someone tries to do that.

[rarkins]

Yes, if they combine // with : then we should fail to parse it and throw

[ArmaanT]

I think we should add a note below the table that combining path and subpreset is not supported.

That makes sense to me, but I'm not sure how that should be worded since subpresets aren't mentioned in the docs right now

[viceice]

I think we should add a note below the table that combining path and subpreset is not supported.

That makes sense to me, but I'm not sure how that should be worded since subpresets aren't mentioned in the docs right now

@HonkingGoose Do you have a suggestion ? 🙃

[HonkingGoose]

@viceice and @ArmaanT

I think a note explaining what parts of the string are "repo", "path" and "subpreset" is OK for now, until we figure out more.
I'm not sure that subpresets need to be mentioned anywhere else? It seems to me that you only need to know those terms in this part of the document anyways? Or are there other places where we should mention these terms?

I don't have a suggestion now, as I don't really understand what terms belong to which parts of the strings. 😄

[CLAassistant]

All committers have signed the CLA.

[ArmaanT]

@HonkingGoose @viceice
I just added a note to the table specifying that the path syntax and subpreset syntax aren't compatible. I still don't feel too confident about it, so I'd like to hear what you both have to say

[viceice]

@ArmaanT Looks like a bad rebase? I think you need to rebase and force push to clean this pr.

[ArmaanT]

Sorry about that @viceice, should be fixed now.

[renovate-release]

🎉 This PR is included in version 24.91.0 🎉

The release is available on:

• GitHub release
• 24.91.0

Your semantic-release bot 📦🚀