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

Make sure .spec and .spec in SRPM are the same #5

Open
FrostyX opened this issue Dec 12, 2022 · 4 comments
Open

Make sure .spec and .spec in SRPM are the same #5

FrostyX opened this issue Dec 12, 2022 · 4 comments
Labels
effort/medium Can be done in 1-2 days gain/low Affect only one person, corner case, has easy workaround

Comments

@FrostyX
Copy link
Owner

FrostyX commented Dec 12, 2022

The fedora-review tool itself checks it but this won't work for Copr, and therefore even for us.

  1. The copr-review-service finds "SRPM URL" link in RHBZ and sends it to Copr
  2. Copr builds the SRPM and produces a results directory with .rpm, .src.rpm, and .spec files that all came from the initial SRPM
  3. Copr runs fedora-review on the results and those two .spec files will always be the same
  4. The "Spec URL" from RHBZ was never considered

We need to do the check manually.

@pemensik
Copy link

I am not sure spec URL should even be required, if we are able to do all things without it. I think the spec file is much easier to work with. But srpm will always contain also spec file. I do not see a need to bother user with uploading two different things, when one can be delivered from the other one.

Instead it might make sense to use the service to extract spec file from the linked SRPM and attach it as attachment to the review bug. Similarly if the spec contains only resources fetchable by spectool -g, then we should not require the user to provide srpm manually. Many reviews need any of those files. Especially if I use something like git for spec file, then the need to upload somewhere also srpm is quite annoying.

Could we just say aloud we ignore unnecessary spec and instead provide link to extracted spec from the srpm? Why should we enforce duplicate copies?

@FrostyX
Copy link
Owner Author

FrostyX commented Jan 19, 2023

Could we just say aloud we ignore unnecessary spec and instead provide link to extracted spec from the srpm? Why should we enforce duplicate copies?

I think we can't just yet because AFAIK the fedora-review tool still expects it if used with -b parameter, and also not all reviewers might be on board with this change.

But I quite like the idea. Maybe we should propose it on some official place?

@pemensik
Copy link

I think we might have then create bug or pull request to fedora-review tool. I understand it is useful for manual reviews. But when automated tool is involved, I do not see a reason for manual preparation, when the same can be obtained by automated step. Creating spec from srpm is as easy as rpm2cpio $SRPM | cpio -i '*.spec'.

Both fedora-review and fedora-create-review can extract spec file without bothering the user about it. I do not see a reason to force user to provide it extra way. Yes, I guess we should raise that idea to wider audience.

@pemensik
Copy link

Created issue on Fedora-review upstream: https://pagure.io/FedoraReview/issue/469

@FrostyX FrostyX added gain/low Affect only one person, corner case, has easy workaround effort/medium Can be done in 1-2 days labels Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium Can be done in 1-2 days gain/low Affect only one person, corner case, has easy workaround
Projects
None yet
Development

No branches or pull requests

2 participants