#179 [cli][frontend][python][rpmbuild] unify mock config generator
Closed 4 years ago by clime. Opened 4 years ago by praiskup.
copr/ praiskup/copr unify-mock-config-generator  into  master

No commits found

From now on, copr-cli mock-config and copr-rpmbuild use the
same mock configuration file (both use the same methods for
generating the mock configuration).

This is done with believe that mock configuration should be
"cache"-able thing -- so that it can be generated on client side
once per copr project, and then re-used to build sets of packages
without re-generating/changing the config file all over again.
There's one hack with 'enable_net' which breaks this concept,
though.

WIP of course, I'd like to discuss this carefully.

Thank you for the PR.

I understand that you would like to unify the mock config generation for copr-cli and copr-rpmbuild. Normally, such unification would be welcome but not exactly here as copr-cli mock-config was a mistake that we should not further develop upon.

It was put there simply because we lacked a better place at the time, which is copr-rpmbuild - tool responsible for actual building that needs to be concerned with the actual build configuration and it is quite natural for it to be able to print the generated configs without building anything.

Other argument is that python-copr is a library that enables users to easily communicate with copr-frontend API (in other words, it is a high-level abstraction over the API) and should not be mis-used to do anything else (e.g. generating mock configuration files). I was seeing copr-cli mock-config as a potentially useful hack that should not really be there and now it's the time to fix it when we actually have the right place where to put that functionality.

To sum up, much better approach here is to move the functionality of generating mock configs solely into copr-rpmbuild and implement something like copr-rpmbuild --rpm --build-config to print the to-be-used mock config to stdout. copr-cli mock-config will be deprecated at the same time. Thank you if you will take on this.

I understand that you would like to unify the mock config generation for copr-cli and copr-rpmbuild. Normally, such unification would be welcome but not exactly here as copr-cli mock-config was a mistake that we should not further develop upon.

I must say I'm sad to hear this, I'm not really sure what's wrong on copr mock-config functionality. That is in total sync with what koji/brew does... and what packager want's to do (work with mock.., not with copr-rpmbuild). Speaking about mistakes (mostly mine in this case, as the original author) reading the copr-rpmbuild code I'm sure the mistake was that originally copr-rpmbuild re-implemented the mock configuration generator once more, while there was one already :-( ready to take. Another mistake was that I was forced to implement build-config json as output from frontend -- I thought (and I still think) that the mock config should have been generated directly by frontend :-(.

It was put there simply because we lacked a better place at the time, which is copr-rpmbuild - tool responsible for actual building that needs to be concerned with the actual build configuration and it is quite natural for it to be able to print the generated configs without building anything.

Which this contribution totally aligns with...

Other argument is that python-copr is a library that enables users to easily communicate with copr-frontend API (in other words, it is a high-level abstraction over the API) and should not be mis-used to do anything else

What else the mock configuration generator is than API for developing packages? OTOH, I agree that better would be to have it only as cli API than python API.. I agree that the python api is basically useless for user, is this the issue?

To sum up, much better approach here is to move the functionality of generating mock configs solely into copr-rpmbuild and implement something like copr-rpmbuild --rpm --build-config to print the to-be-used mock config to stdout. copr-cli mock-config will be deprecated at the same time. Thank you if you will take on this.

I can not disagree more, copr-rpmbuild implements that much stuff that which is terribly hard to contribute to :-( been there last week :-(. I was thinking dozen of times about how it puts obstacles to use mock directly; and how it kills the original idea behind "copr is just thin layer above mock".

I agree that there could be copr-rpmbuild --build-config something, but why not to reuse some "internal library" code? copr-rpmbuild is not meant to be /bin/copr replacement; more it is not mean to be other than debugging helper AND glue between backend and builder (tool to easily prepare builder by the package installation).

Some idea I was thinking about during last years .. would python-copr-common package help? The package would be used to share library code among frontend, backend, distgit, and I could put the mock-config generator there... That package would be basically unusable for the end-user, so we don't have to keep the API stable..?

Well, there's some idea and belief behind this contribution :-) and I'm kindly hoping for reconsideration.. so I can update this WIP PoC and continue, rather than scratch this.

I must say I'm sad to hear this, I'm not really sure what's wrong on copr mock-config functionality. That is in total sync with what koji/brew does... and what packager want's to do (work with mock.., not with copr-rpmbuild).

koji or brew do not need to be necessarily correct about everything. copr-rpmbuild is not a known tool currently but not sure why a packager wouldn't work with it when the implementation is ready. It should provide an easier way to build directly from SRPM URL, Git repo, etc. (see comments in https://pagure.io/copr/copr/issue/102)

copr-rpmbuild re-implemented the mock configuration generator once more, while there was one already :-( ready to take.

Well, you would need dependency on copr-cli just because of that. That doesn't make much sense as copr-cli brings much of other deps. And again, copr-cli is a tool to communicate with copr-frontend API whereas copr-rpmbuild uses just internal interface dedicated for backend. It's a nice separation and I don't see a reason why to break it.

Another mistake was that I was forced to implement build-config json as output from frontend -- I thought (and I still think) that the mock config should have been generated directly by frontend :-(.

Sorry but that's another thing I would like to maintain in the code and that is that frontend only handles build configuration on higher abstraction level - like 'networking enabled', 'additional repos', 'buildroot packages', etc. and not on the level of the concrete technology. The original authors intended it, otherwise they would just allow uploading mock configuration to perform the build. I want to respect the decision of the original authors and keep this paradigm.

I can not disagree more, copr-rpmbuild implements that much stuff that which is terribly hard to contribute to :-( been there last week :-(. I was thinking dozen of times about how it puts obstacles to use mock directly; and how it kills the original idea behind "copr is just thin layer above mock".

Well, I guess it's hard to contribute to it for you because you have very different ideas about how COPR development should look like. It would be useful to sync in this but I don't think that will ever happen. I don't see much obstacles in copr-rpmbuild. And yeah, copr was originally designed as thin layer above mock but now that's not so much true anymore. And I don't think it needs to be. I don't see why we should make tighter connection to particular technology when we don't need to.

I agree that there could be copr-rpmbuild --build-config something, but why not to reuse some "internal library" code? copr-rpmbuild is not meant to be /bin/copr replacement; more it is not mean to be other than debugging helper AND glue between backend and builder (tool to easily prepare builder by the package installation).

What I am saying is that python-copr is not a very good place for that code as python-copr so far just implements just python version of the API (it just wraps API calls) and I would really like to keep it like that. Otherwise I agree with what is said in this paragraph.

Some idea I was thinking about during last years .. would python-copr-common package help? The package would be used to share library code among frontend, backend, distgit, and I could put the mock-config generator there... That package would be basically unusable for the end-user, so we don't have to keep the API stable..?

python-copr-common would be a better place for sure.

My $0.02:

I think that mock config should be generated by copr-backend. Only backend knows what version of mock and what features it is used. It would be nice if it would be small script or function, which generate it from template the way you created it.
This script can go to copr-backend-mockconfig subpackage, which can be used on client as well.

I agree with with @clime that this should likely not go to copr-cli or python-copr.

I think that mock config should be generated by copr-backend. ...

Well, we have been there and moved to generating code by a builder script, now its copr-rpmbuild which has the mock dependency and can operate with it (certain version of copr-rpmbuild might require certain version of mock that supports certain configuration arguments). This is something we cannot do if the template is moved to a library that will not (typically) have the mock dependency. It could be solved via 'Provides' (i.e. Provides: mock-conf-1.4.4) in the library although that might not be that intuitive. Possible though...

We all agree that python-copr is improper place for this code then. I hear nothing against say python-copr-internal (sub)package (previously spelled python-copr-common? Then, copr-cli and copr-rpmbuild can use this basically for free and keep all the benefits..

@clime wrote:

Well, you would need dependency on copr-cli just because of that. That doesn't make much sense as copr-cli brings much of other deps.

Agreed, that's why I'm trying to find better place than copr-cli. Underlining once more, python-copr is neither the right place.. (it is just a bit better than copr-cli).

And again, copr-cli is a tool to communicate with copr-frontend API whereas copr-rpmbuild uses just internal interface dedicated for backend. It's a nice separation and I don't see a reason why to break it.

Agreed, and I believe there's nothing against this in the WIP code.

Well, I guess it's hard to contribute to it for you because you have very different ideas about how COPR development should look like.

It's not really development, even though comparing to other upstreams I have some remarks. The thing is that I only can administrate and contribute to projects which I believe they have clear and sure future; for that I need to see that there's clear (and "finite") architecture goal. If I feel that something goes against the goal/vision, I naturally try to polish that to keep myself motivated, etc. (you probably know that it is mostly volunteering effort from my side, and I want to be honest there, this PR was the hardest and the most expensive one so far for me..). There's nothing fishy, really...

@msuchy wrote:

Only backend knows what version of mock and what features it is used.

Not anymore I think. The thing is that backend actually doesn't even implicitly know the mock version installed on builder, and frontend is in the same position (but frontend has -- without any implementation details and API calls -- all the actual build info needed to generate the so far trivial mock config). We are complicating our life IMO (the generated mock profile could document that it is useful with mock >= some_version), and could be done solely on frontend). Nevermind though if we move the generator to some central place... (then it doesn't matter who calls the internal-library call..)

@clime

This is something we cannot do if the template is moved to a library that will not (typically) have the mock dependency.

Even though I think we are solving some hypothetical issue, the library package can use Conflicts: mock <= the-version.

@clime

This is something we cannot do if the template is moved to a library that will not (typically) have the mock dependency.

Even though I think we are solving some hypothetical issue, the library package can use Conflicts: mock <= the-version.

Perhaps, just note that if you just place that functionality into copr-rpmbuild, there is no such problem even because there you can easily require the specific version of mock explicitly because it is already in Requires. Having it in library that somehow Requires or Conflicts mock package (which is a beast on completely different system level) is a bit strange. It's a hypotetical problem but it's also quite a good indicator how things should be laid out. We seemed to have agreed that copr-rpmbuild is the place to provide that functionality. I think people that are used to copr-cli mock-config can understand the functionality has shifted somewhere else where it is more approapriate. Btw. not sure why python-copr-internal. python-copr-common seems to be more fitting for something that contains common set of utilities.

But hey I am happy about you contributing. It would help to better coordinate but still, thanks for all the feedback!

Having it in library that somehow Requires or Conflicts mock package (which is a beast on completely different system level) is a bit strange.

In different wording; I don't think that this all is needed.. I'm just saying it is possible. If some library generates some mock config, it doesn't necessarily mean that the config needs to be use-able on the target system. You are just asking to get the config which is used in copr buildsystem, and you've got it (it's up to you to get appropriate version of the mock tool, e.g. by the given comment).

We seemed to have agreed that copr-rpmbuild is the place to provide that functionality.

agree that copr-rpmbuild can provide this, too (same as it could just take the config from frontend/backend).

I think people that are used to copr-cli mock-config can understand the functionality has shifted somewhere else where it is more approapriate.

On this I really dislike the need to install one more tool just to get the mock config. Same as I would really dislike movement when there's a solution without compromises.

Btw. not sure why python-copr-internal. python-copr-common seems to be more fitting for something that contains common set of utilities.

Ah didn't say the reason, I wanted to make clear statement that end-users shouldn't depend on this library; but yeah, perhaps python-copr-common is self-explaining too.. ack.

In different wording; I don't think that this all is needed.. I'm just saying it is possible. If some library generates some mock config, it doesn't necessarily mean that the config needs to be use-able on the target system. You are just asking to get the config which is used in copr buildsystem, and you've got it (it's up to you to get appropriate version of the mock tool, e.g. by the given comment).

I guess it boils down to a question: What do you need to reproduce a certain build locally.

With config generator in copr-rpmbuild, you need to use the same version of mock and copr-rpmbuild as COPR used at the time of the build.

With config generator in the library, you need the same version of mock, copr-rpmbuild, and the library to reproduce the build (you need copr-rpmbuild because it calls mock with certain command-line arguments, does the job around cloning the repo, etc.).

By placing this code in a library, you are introducing one more possible point of failure where user has the same version of copr-rpmbuild and mock (which copr-rpmbuild may make sure of) as COPR is currently using and expects the build to run the same (except for updates). But due to a different version of that library (possibly outdated), he will get a different mock config and possibly a different build result.

Yes, copr-rpmbuild now may ensure (and should) it has the right version of the library for the installed mock version but how if the library doesn't explicitly state it? It might state it with Provides but this whole thing just introduces a maintenance cost for not particularly clear benefits.

Please, reconsider this once more.

Warning warning, this "reproducible" approach is totally overestimated. I'm rather
interested in simplifying development of packages.

This is ~99% truth, and matches the real use-cases I've ever done during packaging. Packager

  • almost never reproduce succeeded builds locally, but almost always only failed builds
  • even when "reproducing" build success/failure, he doesn't expect local build environment is 100% compatible with the cloud environment

For those reasons, I don't share your worry about usefulness of mock profile generator;
anyone who will hit copr mock-config does some best-effort work, and won't blame
you that his mock does something different then in copr cloud (because disabled
tmpfs in mock, systemd-nspawn unavailability, less memory or CPU available, different
timeout, etc.).

If we really want to speak about reproducing the builds, we should concentrate on
different thing -> allocating builder for user for general purpose task:
- user can not reproduce build of RPM locally...
- user contacts copr administrator with ssh public key
- copr administrators presses magic button, get's an IP and forwards that to user

Yes, copr-rpmbuild now may ensure (and should) it has the right version of the library for the installed mock version but how if the library doesn't explicitly state it? It might state it with Provides but this whole thing just introduces a maintenance cost for not particularly clear benefits.

I agree that copr-rpmbuild, because it is mainly for copr builder purposes, needs to ensure proper version of mock and version of "the library". But that's only about mentioning proper version in 'Requries:'. I may underestimate this -> so if you think so, we can "C&P" the code with generator from rpmbuild to cli during release process ... (to not duplicate something), but that's implementation detail for solution to the problem -> duplicated generator. As we discussed for a very long time, I'm OK to solve this problem the way you request it. Really, if python-copr-common is something we are OK to try, please let me finish the proposal :-).

I agree that copr-rpmbuild, because it is mainly for copr builder purposes, needs to ensure proper version of mock and version of "the library". But that's only about mentioning proper version in 'Requries:'.

The library should explicitly state for which mock version it provides the config. If not and incompatible library version and mock are used together, things might work but give different results without a user noticing (e.g. in case of introduction of new boostrap_additional_packages configuration option). Therefore, from code and also user perspective, it's better to leave the generator in copr-rpmbuild and provide --build-config there. At the same time, deprecate copr-cli mock-config. Implementation of this will be welcome.

The status now == scratching all the discussion so far, right?

If copr-rpmbuild states which version of mock and library is needed, I don't know how this is related to copr-cli mock-config deprecation and how user will benefit from that.

If copr-rpmbuild states which version of mock and library is needed, I don't know how this is related to copr-cli mock-config deprecation and how user will benefit from that.

You need to know for what mock version the library provides the config. Comment in the code is really not sufficient because it can get easily outdated. If one programmer makes the change in the library and releases a new version of that library for the latest released version of mock, he should then also update copr-rpmbuild to require that version of library and that version of (the released) mock before next release of copr-rpmbuild.

So you are doing changes in one part of code but as a consequence you need to (you should) make changes in completely different part of code when there is no explicit relation from the library where you need to make the change to the copr-rpmbuild package where you should make the change. So a new programmer wouldn't know that and simply wouldn't do it.

This problem does not occur at all if the template is placed directly in the copr-rpmbuild package where it should be placed because copr-rpmbuild is the only system component that really needs it. copr-rpmbuild could provide --build-config to let a user debug mock manually if needed.

To me it is quite clear that you trying to push something, which is not a good code practice and I think we should keep good code practices in our code because otherwise we will start having something that is difficult to change because of some implicit links from one part of code to another. I really want to keep good code practices in the code so that other programmers that will work on COPR have relatively easy work.

If I didn't persuade and you are unwilling to move the functionality to the right place (or leave it there more precisely), then I cannot really do much about it. As a user, you have rights to demand for copr-cli mock-config to be kept and then the unification is better than nothing.

To me it is quite clear that you trying to push something, which is not a good code practice

I know it is personal (said "To me...") POV, but still it is pretty offending and self-confident.

Dunno what changed since the meeting, where we discussed good/bad practices,
came to the compromise, where "build-requires/conflicts" in rpmbuilder were considered
mostly non-problematic (and are mostly theoretical, "what if mock changed, etc."). We also
agreed the python-copr-common is mostly wanted feature, not only for this particular
use-case and it would make sense if somebody took a look at this anyway.. (so to me, even
if this was the only benefit of this PR (it's not!) it would be worth the never-ending talk).

If I didn't persuade and you are unwilling to move the functionality to the right place (or leave it there more precisely), then I cannot really do much about it. As a user, you have rights to demand for copr-cli mock-config to be kept and then the unification is better than nothing.

I can willingly move the code were you think it is more appropriate, but yes..
... I want to keep copr mock-config feature (and ideally I want to generate mock profile
on one more place (#185)). So I want to generate it on one place actually.
Removing that user command only for such trivial implementation detail would be just
unacceptable. There are different ways how to share code across packages, just pick one
please.

Then, please, prepare the code so that it can be reviewed.

Closing this for inactivity and also for not really reaching a general consensus. Feel free to reopen but I would like see an actual use-case for copr mock-config. I do not understand its purpose and I would like to hear it if there is one.

Pull-Request has been closed by clime

4 years ago

Closing this for inactivity and also for not really reaching a general consensus.

Its impossible ...

Feel free to reopen but I would like see an actual use-case for copr mock-config. I do not understand its purpose and I would like to hear it if there is one.

... since if you still don't know what the copr mock-config is for, you don't listen. The problem is also probably lack of packaging experience to understand how mock --shell feature is important (every-day thing for any moderately complicated package).

PRs can not be reopened, you know it. So let's fight with wind mils once we have
python-copr-common package.

Metadata