Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for configuration file inclusion (app.ini.d/*.ini) #33935

Open
StefanHamminga opened this issue Mar 19, 2025 · 10 comments
Open

Support for configuration file inclusion (app.ini.d/*.ini) #33935

StefanHamminga opened this issue Mar 19, 2025 · 10 comments
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@StefanHamminga
Copy link

Feature Description

I'm working on some external rendering scripts and one 'issue' for distributing / packaging them as open source is that the complete configuration needs to be pasted in one file, which isn't very maintenance or user friendly. This isn't really a new idea, a lot of similar software packages have moved from a singular configuration file to separate files for the very same reasons.

Possible pseudo implementation (similar to apt):

  1. Detect presence of a directory app.ini.d
  2. Find all files ending in .ini
  3. Sort those 'naturally' so number prefixes can determine inclusion order
  4. Include in main config in-order

Screenshots

No response

@StefanHamminga StefanHamminga added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Mar 19, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 19, 2025

Gitea's config system is already much more complex than it should be, so I am not sure whether it is able to introduce a new mechanism.

For your use case, if you could build your own binary, I think it only needs only a few lines to do the work: Make NewConfigProviderFromFile find the files in app.ini.d/*.ini and then call cfg.Append to add them.

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Mar 19, 2025
@StefanHamminga
Copy link
Author

Thanks for the quick reply!

I'm not proposing a new mechanism to configure thing, beyond the ability to split the current single file. It's exactly because of the overly complex configuration file it's worthwhile.

For example, here's the bit that allows rendering a KiCad schematic:

[markup.kicad_sch]
ENABLED = true
FILE_EXTENSIONS = .kicad_sch,.kicad_sch.1,.kicad_sch.2,.kicad_sch.3,.kicad_sch.4,.kicad_sch.5,.kicad_sch.6,.kicad_sch.7,.kicad_sch.8,.kicad_sch.9
RENDER_COMMAND = /usr/local/bin/gitea_render_kicad_sch.sh
IS_INPUT_FILE = true
ALLOW_DATA_URI_IMAGES = true
NEED_POSTPROCESS = false

[markup.sanitizer.kicad_sch]
ALLOW_DATA_URI_IMAGES = true
ELEMENT = img
ALLOW_ATTR = style
REGEXP = ""

Of the above information, only line 1 and 2 are 100% relevant to the end-user, line 3 is mostly determined the packager, and the rest is implementation to allow the rendered to function, which should be in a separate file so the author of said renderer can ensure it's kept up to date with Gitea changes and security best practices, not something you'd want your end users to stay aware of imho.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 19, 2025

Hmm yes, I understand this mechanism (I also use apt/nginx a lot, and I also use "include" in their config system).

But the Gitea's problem is: "Gitea's config system is already much more complex than it should be". For example, Gitea could rewrite its app.ini in some cases and change some values. Suppose there is something like:

app.ini:
LOCAL_ROOT_URL = a
app.ini.d/custom.ini:
LOCAL_ROOT_URL = b

Then the config system will go to a mess ......... At the moment I don't have a full picture about how to handle various edge cases clearly (ps: many recent config system related refactoring works were done by me: https://github.com/go-gitea/gitea/pulls?q=is%3Apr+config+system+is%3Aclosed+author%3Awxiaoguang, the Save related background is in #25395 , but there are still legacy problems and technical debts .....)

That's why I'd suggest you to only use the additional config files in your case.

@wxiaoguang
Copy link
Contributor

Maybe we could queue this feature as a sub-task of "completely refactor the legacy ini-based config system".

Indeed, the current ini package is not maintained and has a lot of bugs, Gitea eagerly needs a new config system.

@StefanHamminga
Copy link
Author

Maybe we could queue this feature as a sub-task of "completely refactor the legacy ini-based config system".

Sounds like it. I don't really know Go, but for some C++ tools I wrote/use a config file parser which traces the origin of settings (simple key = value, like INI) with an enum which can be used as bit-field / flags and also works as rank/priority:

/** @brief Where was this value set? */
enum class value_source : uint8_t {
    unset               = 0,            // This value is completely unset
    compiled            = 0b0000'0001,  // The value is based on a compiled-in default
    system_main         = 0b0000'0010,  // The value came from a main system-wide config file
    system_additional   = 0b0000'0100,  // The value came from a system-wide, additional/overriding config file (.d directory)
    user                = 0b0000'1000,  // The value came from a user-specific config file
    environment         = 0b0001'0000,  // The value came from an environment variable
    provided            = 0b0010'0000,  // The value came from a config file specified on the command-line
    argument            = 0b0100'0000,  // The value was set by a command-line argument
    overridden          = 0b1000'0000,  // The value was overridden by the program
};

Whenever duplicate values are found, the highest ranking value is taken. The rank is also used to give more detailed error messages (showing which file is the source of a misconfiguration) or for debugging (printing out / logging a complete config with settings origins).

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Mar 19, 2025

For example, here's the bit that allows rendering a KiCad schematic:

on an aside, how do you render Kicad? (heavy user and contributor)

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 19, 2025

If you really need this feature, you could refer to this branch: https://github.com/wxiaoguang/gitea/tree/demo-config-sub-path (commit: wxiaoguang@df18206 )

The code itself is not complex (only 6 lines), but I worry that it might be abused or block the future config system refactoring. It should be good enough for your own build.

@StefanHamminga
Copy link
Author

StefanHamminga commented Mar 19, 2025

For example, here's the bit that allows rendering a KiCad schematic:

on an aside, how do you render Kicad? (heavy user and contributor)

Here's the bash script. It requires KiCad and Inkscape (for the fitting). Works okayish for now, doesn't handle referred files and I'm looking to add support for panning & zooming so the images are more useful.

#!/bin/bash
#set -x
INPUT="$*"
OUTDIR=$(mktemp -d)

ln -s "$INPUT" "$OUTDIR"/schematic.kicad_sch

mapfile -t FILES \
  < <(kicad-cli sch export svg --exclude-drawing-sheet --no-background-color --output "$OUTDIR" "$OUTDIR/schematic.kicad_sch" | grep -E 'Plotted to' | sed -r "s/^[^']+'|'[^']+$//g" )

page_num=1

for FILE in "${FILES[@]}"; do
  echo -n '<img src="data:image/svg+xml;base64,'
  inkscape --actions="page-fit-to-selection" -o - "$FILE" | base64 --wrap=0
  echo '"'
  echo ' class="full_width kicad_sch pan_zoom"'
  # echo ' style="display:block;width:100%;"'
  echo '/>'
  
  page_num=$((page_num + 1))
  break # TODO: find some way to handle related (hierarchical) schematic files, as currently only the active file is made available
done

# TODO: Probably should add this to the template code
cat << 'TAIL_END'
<script src="https://unpkg.com/@panzoom/[email protected]/dist/panzoom.min.js"></script>
<script>
  try {
    const container = document.scripts[document.scripts.length - 1].parentNode;
    container.querySelectorAll('img').forEach((img) => {
      const panzoom = Panzoom(img, {
        maxScale: 5
      });
    });
  } catch (e) {
    console.error('Error loading Panzoom:', e);
  }
</script>
TAIL_END

rm -R "$OUTDIR"

@StefanHamminga
Copy link
Author

If you really need this feature, you could refer to this branch: https://github.com/wxiaoguang/gitea/tree/demo-config-sub-path (commit: wxiaoguang@df18206 )

The code itself is not complex (only 6 lines), but I worry that it might be abused or block the future config system refactoring. It should be good enough for your own build.

Thank you!

I will give this a try (though it might be a while, haven't gotten a build server / system setup for Debian atm).

@StefanHamminga
Copy link
Author

@eeyrjmr I just uploaded the KiCad renderers and related tools here:

https://git.rbts.co/rbts.co/gitea_external_renderers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

3 participants