-
Notifications
You must be signed in to change notification settings - Fork 22
Allow to disable upstream repos (was: Disable upstream repositories by default) #39
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
base: master
Are you sure you want to change the base?
Conversation
Dockerfile-9.x
Outdated
# enable repositories commonly required to build | ||
RUN dnf config-manager --enable crb | ||
ENV UPSTREAM_REPOS="appstream baseos crb epel extras alma10-devel" | ||
|
||
# TODO: move this before installing any packages, so we can make sure they are actually in the XCP-ng repository | ||
# disable upstream repositories | ||
RUN echo $UPSTREAM_REPOS | xargs -n1 dnf config-manager --disable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For v9 we do not copy the upstream RPMs into ours (at least for now), so we don't want such a disabling, it would prevent most packages from building
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we could go for safer defaults and build with -U
in the meantime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now the design of v9 is "just an overlay", so it may make sense if later we decide to change it (and we may), but right now I tend to think it feels just awkward :)
Actually, it would even make sense for the "master/next" branch to just pick all upstream changes that way, and to (possibly) switch to the "copy upstream RPMs" approach we use in 8.3 when we produce a release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then should we have an option to disable the upstream repositories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it does not make any sense for v9, disabling upstream repos just make legit packages unreachable. How things evolve there still remains to be seen, so I'd rather delay any change here.
files/Alma10-devel.repo
Outdated
enabled=1 | ||
enabled=0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
Dockerfile-8.x
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a change I've always hesitated to make. There are pros and cons, due to the hybrid nature of this tool, which was initially designed by XS as a developer tool. The build env wasn't about replicating 100% of the official builds. It was made for tinkering, building stuff, and never made for replicating koji builds.
In my use cases, having the external repositories enabled was a perk. But I never forgot how the tool worked and how it is not a replication of koji, so any new package I'd build I'd check where it would pull the deps from. I would use this a tool so see if the package could build, be it at the cost of importing a few more deps from CentOS / EPEL. That even allowed me to directly get the list of required dependencies, as well as their own sub-dependencies, etc.
I don't have a strong opinion about what's the best default (but see below about whether I think it's good to use the build env at all for test builds), so I'm not against the change but this also means I don't see high added value to it either. Maybe it's best to change the default as the packaging work gets spread among the dev teams. But maybe it's the expectation of what the tool is made for that is wrong. Another way to see it would be leaving it as is, but displaying the list of pulled dependencies coming from external repositories, at the end, after the build.
Anyway, IMO, if we want a tool that replicates koji builds, we need to use mock. That's the only one tool we should use if we want to check how this would build in koji. And we can choose what tag to build for, rather than having to think about which repositories should or should not be enabled (testing, ci, incoming, etc.). If I understood correctly, @psafont intends to add support for mock in xcp-ng-dev
anyway, and I think this is the right way (as long as we use koji).
Regarding whether test repositories should be enabled or not, my choice, after letting the devs give their opinion (most didn't really have an opinion in fact, at the time), was to enable everything down to xcp-ng-ci
. If you're building, you're usually building in a development context. I could have enabled xcp-ng-incoming
too, but as its contents can be temporarily inconsistent, I chose not to. Which caused situations where developers would ask why their build fails, when it was just the xcp-ng-incoming
repository that had to be enabled for their build to pull the WIP dependencies. If you disable also ci
, testing
and candidates
by default, then I don't know what use the build env will have at all for local builds for XCP-ng developers. But again, problems that are solved by getting the mock config from koji for local builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
displaying the list of pulled dependencies coming from external repositories, at the end, after the build.
That one would be limited to the non-interactive (old --local-build
) use
fcf887b
to
f938893
Compare
I've kept the upstream repositories by default, and just added an option to easily disable them. |
f938893
to
ac2262e
Compare
Well, having them disabled by default (for v8) seems sensible to me, and the use-case mentioned by @stormi seems adequately provided by a I think we should not let any pending work on v9 change this, v9 will look different from all previous versions on a number of things. My vote would go to "disable by default on v8 and do nothing on v9 for now". |
This allows to generate an error when a dependency is not aleardy packaged in XCP-ng. The developer can then decide if the dependency should be either imported or removed. Signed-off-by: Gaëtan Lehmann <[email protected]>
ac2262e
to
62aeb6e
Compare
and add the --enable-upstream-repos option (-U for short) to enable them.
This allows to generate an error when a dependency is not aleardy packaged in XCP-ng. The developer can then decide if the dependency should be either imported or removed.
We may also want to disable all the repositories for the not-yet-published XCP-ng packages (
xcp-ng-candidates
,xcp-ng-ci
andxcp-ng-testing
).