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

context: avoid prefixing vars directories #1506

Conversation

lucab
Copy link
Contributor

@lucab lucab commented May 4, 2022

This drops the internal prefixing logic for vars directories, thus
avoiding the prepended install_root path prefix on all entries.
It also results in aligning the behaviors of dnf_context_set_vars_dir()
and dnf_context_set_repos_dir().

Existing consumers in the wild are not really expecting this kind
of prefixing to happen internally. Additionally, developers seem to
naturally assume that the lookalike APIs for repos-dir and vars-dir
behave in the same way.

Ref: coreos/rpm-ostree#3241
Ref: https://github.com/PackageKit/PackageKit/pull/369/files#r863937752
Closes: #1503

This drops the internal prefixing logic for vars directories, thus
avoiding the prepended `install_root` path prefix on all entries.
It also results in aligning the behaviors of `dnf_context_set_vars_dir()`
and `dnf_context_set_repos_dir()`.

Existing consumers in the wild are not really expecting this kind
of prefixing to happen internally. Additionally, developers seem to
naturally assume that the lookalike APIs for repos-dir and vars-dir
behave in the same way.

Ref: coreos/rpm-ostree#3241
Ref: https://github.com/PackageKit/PackageKit/pull/369/files#r863937752
Closes: rpm-software-management#1503
@cgwalters
Copy link
Collaborator

Changing the semantics of an existing API seems like a bit of a trap to me; won't this potentially break rpm-ostree/PackageKit if they update to this version without also changing how they call the API?

WDYT about adding a new API dnf_context_set_vars_dirs_absolute() or something?

@lucab
Copy link
Contributor Author

lucab commented May 4, 2022

To the best of my understanding, existing software is already written with the expectation that libdnf behaves in the way that this PR does. Prefixing get performed by the consumer, where needed.
I also believe that using a custom install_root (like rpm-ostree does) would right now result in buggy double-prefixing in those softwares.

But yes, I'd love some double-checks and feedback from libdnf/microdnf/PackageKit folks before venturing too much into this.
All the signs I've seen so far point to an implemented behavior that nobody really planned/expected.
But I may have misunderstood some of those details. I dumped all my findings so far into #1503.

HuijingHei added a commit to HuijingHei/coreos-assembler that referenced this pull request Aug 2, 2022
@jan-kolarik
Copy link
Member

Hi, sorry but we're decided to rather not merge such a change that could potentially change the existing behavior for API users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

context: vars dirs prefixing is problematic
3 participants