Skip to content

feat: Add "force_implicit_dirs" to dir/tree.#948

Open
volkadav wants to merge 4 commits into
goreleaser:mainfrom
volkadav:feat-add-tree-force-implicit-dirs
Open

feat: Add "force_implicit_dirs" to dir/tree.#948
volkadav wants to merge 4 commits into
goreleaser:mainfrom
volkadav:feat-add-tree-force-implicit-dirs

Conversation

@volkadav

@volkadav volkadav commented Jul 8, 2025

Copy link
Copy Markdown

This allows users to specify a list of directories to treat akin to the list of system directories, i.e. things that the constructed package should not take ownership of in e.g. the rpm database. (I'm 100% open to a better name for that, btw!)

Example use case:

  1. user belongs to FooCorp
  2. FooCorp has many packages to install under /opt/foocorp
    e.g. /opt/foocorp/service[A,B,C...]
  3. Using tree w/ forced implicit dirs keeps generated package(s)
    from claiming ownership of /opt/foocorp so that the
    service[A,B,C...] packages can be bundled and installed
    separately without path conflicts.

The particular scenario I ran into this for was with RHEL8+ being much pickier about each dir being owned by one and only one package. (We pair this with having one "foocorp-dirs" package that just owns the shared dir creation, that's used as a pre-req for the other packages.) We could have done something like explicitly list every sub-dir needed or every file for all the service packages, but using tree was much more concise and flexible with respect to package contents changing over time, and in effect this is giving the user the ability to specify dirs to treat like the hard-coded list of system directories already checked towards the same end.

I have run task ci and the normal tests pass. There seems to be some issue with opkg going away on the latest OpenWRT images that keeps the ipk acceptance test from working though:

#9 ERROR: process "/bin/sh -c opkg install /tmp/foo.ipk" did not complete successfully: exit code: 127
                                ------
                                 > [min 1/1] RUN opkg install /tmp/foo.ipk:
                                0.287 /bin/sh: opkg: not found
                                ------
                                ipk.dockerfile:12
                                --------------------
                                  10 |     # ---- minimal test ----
                                  11 |     FROM test_base AS min
                                  12 | >>> RUN opkg install /tmp/foo.ipk
                                  13 |
                                  14 |
                                --------------------

Sadly I don't know enough about OpenWRT/opkg/Docker to guess what the right solution would be for that, so I haven't included any changes for it.

Thank you for a cool tool! :)

@pull-request-size pull-request-size Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 8, 2025

@erikgeiser erikgeiser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this proposal. However, we would need a test as well. Don't worry, I'll update the PR myself.

Comment thread files/files.go
@pull-request-size pull-request-size Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 22, 2026
@erikgeiser erikgeiser force-pushed the feat-add-tree-force-implicit-dirs branch from 3550686 to e125e10 Compare March 22, 2026 14:19
@pull-request-size pull-request-size Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 22, 2026
volkadav and others added 3 commits March 22, 2026 15:23
This allows users to specify a list of directories to treat akin
to the list of system directories, i.e. things that the constructed
package should *not* take ownership of in e.g. the rpm database.

Example use case:
1) user belongs to FooCorp
2) FooCorp has many packages to install under /opt/foocorp
   e.g. /opt/foocorp/service[A,B,C...]
3) Using tree w/ forced implicit dirs keeps generated package(s)
   from claiming ownership of /opt/foocorp so that the
   service[A,B,C...] packages can be bundled and installed
   separately without path conflicts.

The particular scenario I ran into this for was with RHEL8+ being
much pickier about each dir being owned by one and only one package.
(We pair this with having one "foocorp-dirs" package that just
 owns the shared dir creation, that's used as a pre-req for the
 other packages.)
@erikgeiser erikgeiser force-pushed the feat-add-tree-force-implicit-dirs branch from b38842a to ffb107d Compare March 22, 2026 14:23
@codecov

codecov Bot commented Mar 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.76%. Comparing base (432fb67) to head (5419c42).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #948      +/-   ##
==========================================
+ Coverage   73.64%   73.76%   +0.12%     
==========================================
  Files          23       23              
  Lines        2758     2771      +13     
==========================================
+ Hits         2031     2044      +13     
  Misses        507      507              
  Partials      220      220              

☔ 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.

@erikgeiser

Copy link
Copy Markdown
Member

Before I merge this, I would like to ask @caarlos0 or @djgilcrease what they think about the name disown_subtree for this settings and whether they also think that this is a reasonable feature.

I also added a fix to an issue which is currently present in main because I encountered it when writing tests. If I add a tree to /, nfpm will try to add the explicit directory . which gets normalized to // to the package. I'm pretty sure we don't want that and it was currently not covered by tests because all tree tests have non-root directory as destination. The fix is simple: https://github.com/goreleaser/nfpm/pull/948/changes#diff-326e77b3dbf4c7c47df40989fb759cc07692519b73e1698190193f266678c8f7R484

@volkadav

volkadav commented Mar 23, 2026

Copy link
Copy Markdown
Author

Thank you, Erik! I had sort of forgotten about this (and have moved on from the company that needed it), but it's good to see it getting some traction. The disown_subtree name makes total sense. If there's anything further I can do to help, please let me know!

(edit to add: I looked at the failing checks. The semgrep failure is complaining about something in msix/msix.go which afaict is unrelated to this PR and the docs build appears to relate to populating author info in an rss feed; sadly I don't know enough about the project to know what the right fixes might be for either, but if needed I can make some educated guesses.)

@erikgeiser

Copy link
Copy Markdown
Member

Sorry for not being able to review this PR when you actually needed it.

Yes the CI issues are not related to this PR, so it could be merged as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants