many: force-repo-dir instead of force-data-dir
#408
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
supakeen
requested review from
bcl,
mvo5 and
thozza
and removed request for
a team
Member
Author
|
Since we had some previous discussion about not growing too many arguments; this will (eventually, we need to decide a timeline) be the only argument left over. This also more importantly unifies the naming of arguments to be This also reintroduces the ability to load repositories directly from the root of the passed We'll need to establish a timeframe for this. |
supakeen
added a commit
to supakeen/image-builder-cli
that referenced
this pull request
Dec 15, 2025
After a chat in our community Matrix channel I realized that we have not documented the various ways of managing repositories in `image-builder`. Let's start that off with a small initial page that I will build on in the following commits. This relies on the implementation of the new `force-repo-dir` behavior as that's the behavior documented [1]. [1]: osbuild#408 Signed-off-by: Simon de Vlieger <supakeen@redhat.com>
supakeen
added a commit
to supakeen/image-builder-cli
that referenced
this pull request
Dec 15, 2025
After a chat in our community Matrix channel I realized that we have not documented the various ways of managing repositories in `image-builder`. Let's start that off with a small initial page that I will build on in the following commits. This relies on the implementation of the new `force-repo-dir` behavior as that's the behavior documented [1]. [1]: osbuild#408 Signed-off-by: Simon de Vlieger <supakeen@redhat.com>
supakeen
added a commit
to supakeen/image-builder-cli
that referenced
this pull request
Dec 15, 2025
After a chat in our community Matrix channel I realized that we have not documented the various ways of managing repositories in `image-builder`. Let's start that off with a small initial page that I will build on in the following commits. This relies on the implementation of the new `force-repo-dir` behavior as that's the behavior documented [1]. [1]: osbuild#408 Signed-off-by: Simon de Vlieger <supakeen@redhat.com>
lzap
reviewed
Dec 15, 2025
Previously I deprecated `data-dir` and renamed it to `force-data-dir` to have consistent naming for options that overwrite internal state instead of appending to it (arguments that start with `force-` are overwrites, arguments that start with `extra-` are merges). With `force-data-dir` we require a `repositories` sub directory. After some discussion we came to the idea to instead of enforcing a directory layout on the user we want to be able to point directly at a directory of repositories. This commit deprecates `force-data-dir` and introduces `force-repo-dir`. Both write to the same internal variable. We also allow repository files to live *either* in a `repositories` subdirectory to keep backwards compatibility for any of the arguments or to live directly at the top level of the passed directory. We show a deprecation warning if a `repositories` sub directory exists and inform users that the `repositories` subdirectory will not be wanted in the future and how to resolve the emitted warning. This option will be used together with the future introduction of `force-defs-dir` to handle definitions (and its `extra-defs-dir` counter part) unifying all options into a consistent naming scheme. Signed-off-by: Simon de Vlieger <supakeen@redhat.com>
lzap
approved these changes
Dec 15, 2025
thozza
reviewed
Dec 17, 2025
| _, err = registry.DistroHasRepos("testdistro-1", "x86_64") | ||
| require.EqualError(t, err, `requested repository not found: for distribution "testdistro-1"`) | ||
|
|
||
| // create a custom datadir with testdistro-1.json, the basefilename |
Member
There was a problem hiding this comment.
Nitpick: the comment should be amended, since it still mentions datadir.
Comment on lines
+74
to
+75
| repoDir := t.TempDir() | ||
| repoFile := filepath.Join(repoDir, "repositories", "testdistro-1.json") |
Member
There was a problem hiding this comment.
It may be beneficial to test this also without the repositories sub-directory.
| rootCmd.PersistentFlags().StringVar(&forceRepoDir, "force-data-dir", "", `Override the default data directory for e.g. custom repositories/*.json data`) | ||
| rootCmd.PersistentFlags().MarkDeprecated("force-data-dir", `Use --force-repo-dir instead`) | ||
| rootCmd.PersistentFlags().StringVar(&forceRepoDir, "data-dir", "", `Override the default data directory for e.g. custom repositories/*.json data`) | ||
| rootCmd.PersistentFlags().MarkDeprecated("data-dir", `Use --force-data-dir instead`) |
Member
There was a problem hiding this comment.
You may want to fix the deprecation message here to suggest --force-repo-dir instead.
| switch name { | ||
| case "data-dir": | ||
| name = "force-data-dir" | ||
| name = "force-repo-dir" |
Member
There was a problem hiding this comment.
Should the force-data-dir option be normalized as well?
| if _, err := os.Stat(withRepoSubdir); err == nil { | ||
| // we don't care about the error case here, we just want to know | ||
| // if it exists; not if we can't read it or other errors | ||
| fmt.Fprintf(os.Stderr, "WARNING: `force-repo-dir` found a `repositories` subdirectory at '%s', in the future `image-builder` will not descend into this subdirectory to look for repository files. Please move any repository files directly into the directory '%s' and remove the `repositories` subdirectory to silence this warning.\n", withRepoSubdir, repoDir) |
Member
There was a problem hiding this comment.
Mentioning the force-repo-dir option here may be misleading, since one could specify --data-dir and get the warning about an option that they didn't use.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously I deprecated
data-dirand renamed it toforce-data-dirto have consistent naming for options that overwrite internal state instead of appending to it (arguments that start withforce-are overwrites, arguments that start withextra-are merges).With
force-data-dirwe require arepositoriessub directory. After some discussion we came to the idea to instead of enforcing a directory layout on the user we want to be able to point directly at a directory of repositories.This commit deprecates
force-data-dirand introducesforce-repo-dir. Both write to the same internal variable.We also allow repository files to live either in a
repositoriessubdirectory to keep backwards compatibility for any of the arguments or to live directly at the top level of the passed directory.We show a deprecation warning if a
repositoriessub directory exists and inform users that therepositoriessubdirectory will not be wanted in the future and how to resolve the emitted warning.This option will be used together with the future introduction of
force-defs-dirto handle definitions (and itsextra-defs-dircounter part) unifying all options into a consistent naming scheme.