PR#107: [distgit] configurable modulename based on specfile name - copr/copr

copr / copr

#107 [distgit] configurable modulename based on specfile name

Closed 6 years ago by clime. Opened 6 years ago by praiskup.

Unknown source module-name-detection into master

[distgit] configurable modulename based on specfile name

Pavel Raiskup • 6 years ago

4cef1b4

file modified

		`@@ -179,6 +179,11 @@`
		`opts.git_user_email = _get_conf(`
		`cp, "dist-git", "git_user_email", "copr-devel@lists.fedorahosted.org"`
		`)`
		`+`
		`+ opts.name_tag_for_import = _get_conf(`
		`+ cp, "dist-git", "name_tag_for_import", True, mode="bool"`
		`+ )`
		`+`
		`return opts`

dist-git/dist_git/package_import.py

file modified

+4 -1

		`@@ -153,7 +153,10 @@`
		`"""`
		`log.debug("package_content: " + str(package_content))`

		`- pkg_name = helpers.get_package_name(package_content.spec_path)`
		`+ pkg_name = os.path.basename(package_content.spec_path)[:-5]`
		`+ if opts.name_tag_for_import:`
		`+ pkg_name = helpers.get_package_name(package_content.spec_path)`
		`+`
		`log.debug("pkg_name: " + str(pkg_name))`

		`pkg_evr = helpers.get_pkg_evr(package_content.spec_path)`

praiskup commented 6 years ago

When spec file is named 'foo.spec', regardless what's the 'Name'
tag or the final package name -- every packager want's import the
spec file into 'foo.git' module.

Relying on Name tag parsing is both brittle (parsing spec files is
tough, and is not task for copr) and dangerous (since we plan to
open dist-git for direct writing -- so users could do git push in
future -- parsing Name in any case might lead to total unexpected
breakage of the dist-git repos in future.

This is follow up for (my monologue) discussion in:
https://pagure.io/copr/copr/issue/106

clime commented 6 years ago

Historically, the repository name and copr package name comes from srpm Name tag, which is derived from spec file when the srpm is built. This was also true for the previous SCM "methods" (Tito and MockSCM).

Now srpms are not built on copr-dist-git and hence we opted for fetching package name directly from spec file. That should be equivalent except maybe for some obscure cases where name of package is dependent on where you build the srpm or parse the spec-file.

Name of the spec file was never important and I don't know why it should be important now. It's also not relevant when you build a srpm package with rpmbuild and I think it's good to stay consistent with that because people might be used to spec file name not to play a role.

I don't see a problem with the current implementation. If you encountered a bug, you can report it. Your comments in issue 106 were off-topic as the problem was about not removing no longer present files in a remote repo or in an uploaded srpm.

Also, there is no use-case for having this configurable.

Pull-Request has been closed by clime

6 years ago

praiskup commented 6 years ago

Historically, the repository name and copr package name comes from srpm Name tag, which is derived from spec file when the srpm is built.

Yeah, it was buggy before. As you don't generate srpm in general now, you can not simulate the previous buggy behavior; which is good. Let's do it right now.

Now srpms are not built on copr-dist-git and hence we opted for fetching package name directly from spec file.

To correct you, you derive it from Name tag (and nobody knows to what this tag will expand to on dist-git machine), not from the spec file. I prefer using spec file name because that's much more robust and logical.

Name of the spec file was never important and I don't know why it should be important now.

Spec file name was always important. It matters to experienced packagers and you should discuss such things with them; the fact you haven't noticed is that nobody complained so far.. and when I complain, you simply redirect that to /dev/null.

I think it's good to stay consistent with that because people might be used to spec file name not to play a role.

Nope, it is not good .. I can accept that you want to be compatible with the previous buggy behavior (avoid changes), but that's not important to my deployment. So because we share the same upstream code, I need to have that option to not diverge by patching.

I don't see a problem with the current implementation. If you encountered a bug, you can report it.

I see a problem in the implementation and I already commented on real bug. Importing blah.spec into foo.git is a bug.

Also, there is no use-case for having this configurable.

There's use-case in my deployment. As we talked about it previously f2f, we ended up with the fact that we'll discuss this on scrum... closing this just underlines your unfriendly maintainership; there's no technical reason to not accept this option. Ok, I'll waste another several minutes with providing new pull request (because I can not reopen). No-thanks.

praiskup commented 6 years ago

Moved to https://pagure.io/copr/copr/pull-request/111