Add config to change default pipeline config paths and extensions

[jolheiser]

Based on discussion from #6576

It feels awkward to set a variable in a package called constant 😅
But this seemed like the least intrusive way to make this change without having to push the CLI flag all around in a bigger refactor.

closes #5428

[qwerty287]

I would not change that constant, rather add a new field to server/config.go

[jolheiser]

Alright, moved to server/config.go

[6543]

beside cli this looks good :)

[jolheiser]

Still seems to have an import cycle in forge, but I am less sure how to deal with this one.

[jolheiser]

Open to suggestions here, a572dd8 was fairly simple albeit a bit hacky.

Seems that moving it to server means plumbing it through as extra args to avoid import cycles unless there is a better alternative I'm not seeing.

[qwerty287]

Not sure, but in the ui we show the default values if the field is empty, right? This would be wrong if the config is changed.

[jolheiser]

Hm, yes it would be. The translation could be changed to just that prefix "By default:" and then dynamically create the "x -> y -> z" although it may get too long depending on how it's set.

[6543]

can you also copy tje new flag to cli exec?

https://github.com/woodpecker-ci/woodpecker/blob/main/cli/exec/flags.go

[qwerty287]

Hm, yes it would be. The translation could be changed to just that prefix "By default:" and then dynamically create the "x -> y -> z" although it may get too long depending on how it's set.

I think that's OK, it's unlikely that someone will set hundreds of files there.

[6543]

just thunking ... if you wana have jsonnet and multiworkflow files ... this patcher do not work, you we also need to be able to change file extensions we do filter upon

[jolheiser]

This patch would work, because it parses out extensions bases on the paths given.
There is an edge case, however, where if you only give it directory paths it would not work.
But if it's set to e.g. .woodpecker.jsonnet,.woodpecker/ it would parse all jsonnet files in the .woodpecker dir still.

I can update the translation, however I still have the issue that all the things I've tried after moving to server/config.go lead to import cycles. Plumbing it all the way through seems like a lot of churn, is that preferred over modifying (or adding to) constants?

[qwerty287]

Which import cycles? This should not happen at all I think. Where do you want to use the configs? In packages like api etc. this shouldn't give you import cycles.

I think for the file extensions I would actually prefer a separate env var to configure that instead of inferring from the paths.

[jolheiser]

https://ci.woodpecker-ci.org/repos/3780/pipeline/34580/10

package go.woodpecker-ci.org/woodpecker/v3/cmd/server
imports go.woodpecker-ci.org/woodpecker/v3/server from grpc_server.go
imports go.woodpecker-ci.org/woodpecker/v3/server/services from config.go
imports go.woodpecker-ci.org/woodpecker/v3/server/services/config from manager.go
imports go.woodpecker-ci.org/woodpecker/v3/server from forge.go: import cycle not allowed

[woodpecker-bot]

Surge PR preview deployment was removed

[qwerty287]

Ah I see, then we probably should not store that globally. The services indeed shouldn't use the global config. Maybe you could store the config diretly in the forgeFetcher struct instead? And pass it using NewForge below.

[qwerty287]

@6543 you wanted to add this to the cli as well, right? Beside that also lgtm.

[6543]

thx yes cli should also have this flag ...

also test fails ...

[jolheiser]

afaict the exec command doesn't touch the config extension, so wiring it up doesn't do much currently. Do you still want it plumbed for a potential future enhancement?

[6543]

afaict the exec command doesn't touch the config extension, so wiring it up doesn't do much currently. Do you still want it plumbed for a potential future enhancement?

aww ... in this case we can wait till we have a proper exec ... I'm currently refactoring the bases to make it work propperly anyway

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.71%. Comparing base (8363eff) to head (62e7804).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6580      +/-   ##
==========================================
+ Coverage   41.60%   41.71%   +0.11%     
==========================================
  Files         432      433       +1     
  Lines       28805    28889      +84     
==========================================
+ Hits        11983    12050      +67     
- Misses      15747    15751       +4     
- Partials     1075     1088      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.