Skip to content

MissingRemoteId: also check EGIT_REPO_URI #643

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.

Sign up for GitHub

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

Draft
parona-source wants to merge 1 commit into
base: master
Choose a base branch
from
+60 −1
Draft

MissingRemoteId: also check EGIT_REPO_URI #643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/pkgcheck/checks/metadata_xml.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,14 @@ def feed(self, pkgset):
urls = set(filter(self.__filter_url, all_urls))
urls = sorted(urls.union(pkg.homepage), key=len)

if "git-r3" in pkg.inherited and hasattr(pkg, "environment"):
Copy link
Contributor

@ferringb ferringb Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is hasattr needed here?

I don't know this codepath fully- is there an implication that PackageRepoSource isn't guaranteed to surface the environment text data? Because from the pkgcore side, it should always be accessible afaik.

And if it isn't, I believe that is a bug and should be fixed.

egit_repo_uri = re.compile(r"^declare -- EGIT_REPO_URI=\"(.*)\"$")
Copy link
Contributor

@ferringb ferringb Feb 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're hardcoding that bash currently uses " for these directives, and that the export statement doesn't inject crap like actual 'export' (which the env saving implementation should- declare -e iirc, if it's marked as exported). Any of those will break this; a code comment note, and/or an expansion of the regex is warranted.

For the regex, this should be moved to a class var; it both is easier to spot it, and it avoids re.compile relying on it's internal LRU to avoid the recompile. If it must recompile, it's surprisingly not cheap.

Your code here implies pkgcore should provide a better api for this, also.

Copy link
Contributor

@ferringb ferringb Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also- considering that regex has mild complexity to get it right- as I mentioned for things like export or array- perhaps this should be broke out in a later PR as a util func, and other regex's rebased to it?

for env_line in pkg.environment.data.splitlines():
result = re.search(egit_repo_uri, env_line)
Copy link
Contributor

@ferringb ferringb Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

if result := re.search(egit_repo_uri, env_line):
  urls.append ...

if result:
urls.append(result.group(1).removesuffix(".git"))
break

for remote_type, regex in self.remotes_map:
if remote_type in remotes:
continue
Expand Down
1 change: 1 addition & 0 deletions testdata/data/repos/eapis-testing/MissingRemoteIdCheck/MissingRemoteId/expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{"__class__": "MissingRemoteId", "category": "MissingRemoteIdCheck", "package": "MissingRemoteId", "remote_type": "gitlab", "value": "pkgcore/pkgcheck/extra/MissingRemoteId", "uri": "https://gitlab.com/pkgcore/pkgcheck/extra/MissingRemoteId/-/archive/1/MissingRemoteId-1.tar.bz2"}
{"__class__": "MissingRemoteId", "category": "MissingRemoteIdCheck", "package": "MissingRemoteId", "remote_type": "kde-invent", "value": "pkgcore/pkgcheck", "uri": "https://invent.kde.org/pkgcore/pkgcheck"}
{"__class__": "MissingRemoteId", "category": "MissingRemoteIdCheck", "package": "MissingRemoteId", "remote_type": "heptapod", "value": "pkgcore/pkgcore", "uri": "https://foss.heptapod.net/pkgcore/pkgcore/-/archive/4/MissingRemoteId-4.tar.bz2"}
{"__class__": "MissingRemoteId", "category": "MissingRemoteIdCheck", "package": "MissingRemoteId", "remote_type": "pypi", "value": "MissingRemoteId", "uri": "https://files.pythonhosted.org/packages/source/M/MissingRemoteId/MissingRemoteId-1.tar.gz"}
{"__class__": "MissingRemoteId", "category": "MissingRemoteIdCheck", "package": "MissingRemoteId", "remote_type": "sourceforge", "value": "pkgcheck", "uri": "https://downloads.sourceforge.net/pkgcheck/MissingRemoteId-2.tar.gz"}
3 changes: 2 additions & 1 deletion testdata/data/repos/eapis-testing/MissingRemoteIdCheck/MissingRemoteId/fix.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
--- eapis-testing/MissingRemoteIdCheck/MissingRemoteId/metadata.xml
+++ fixed/MissingRemoteIdCheck/MissingRemoteId/metadata.xml
@@ -3,6 +3,10 @@
@@ -3,6 +3,11 @@
<pkgmetadata>
<upstream>
<remote-id type="bitbucket">pkgcore/pkgcheck</remote-id>
+ <remote-id type="gitlab">pkgcore/pkgcheck/extra/MissingRemoteId</remote-id>
+ <remote-id type="heptapod">pkgcore/pkgcheck</remote-id>
+ <remote-id type="pypi">MissingRemoteId</remote-id>
+ <remote-id type="sourceforge">pkgcheck</remote-id>
+ <remote-id type="kde-invent">pkgcore/pkgcheck</remote-id>
</upstream>
<use>
<flag name="test">enable tests</flag>
9 changes: 9 additions & 0 deletions testdata/repos/eapis-testing/MissingRemoteIdCheck/MissingRemoteId/MissingRemoteId-5.ebuild
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
EAPI=7

inherit git-r3

DESCRIPTION="Check EGIT_REPO_URI"
HOMEPAGE="https://pkgcore.github.io/pkgcheck/"
EGIT_REPO_URI="https://invent.kde.org/pkgcore/pkgcheck.git"
LICENSE="BSD"
SLOT="0"
40 changes: 40 additions & 0 deletions testdata/repos/eapis-testing/eclass/git-r3.eclass
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Copyright 1999-2024 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

# @ECLASS: git-r3.eclass
# @MAINTAINER:
# Michał Górny <mgorny@gentoo.org>
# @SUPPORTED_EAPIS: 7 8
# @BLURB: Eclass for fetching and unpacking git repositories.
# @DESCRIPTION:
# Third generation eclass for easing maintenance of live ebuilds using
# git as remote repository.

case ${EAPI} in
Copy link
Contributor

@ferringb ferringb Feb 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is out of scope for your PR, just an observation.

I suspect we should inject a function into ebuild/eclass environment tests that is require_eapi $@. It'll reduce this sort of boilerplate. It's our tests, this sort of helper would be fine.

What's others thoughts? And are there are other areas we should do this? Having the capability I'd expect would lead to finding usage.

IE, if we build the injection- is it just for this, or are we improving the general case of our tests?

7|8) ;;
*) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
esac

if [[ ! ${_GIT_R3_ECLASS} ]]; then
_GIT_R3_ECLASS=1

# @ECLASS_VARIABLE: EGIT_REPO_URI
# @REQUIRED
# @DESCRIPTION:
# URIs to the repository, e.g. https://foo. If multiple URIs are
# provided, the eclass will consider the remaining URIs as fallbacks
# to try if the first URI does not work. For supported URI syntaxes,
# read the manpage for git-clone(1).
#
# URIs should be using https:// whenever possible. http:// and git://
# URIs are completely insecure and their use (even if only as
# a fallback) renders the ebuild completely vulnerable to MITM attacks.
#
# Can be a whitespace-separated list or an array.
#
# Example:
# @CODE
# EGIT_REPO_URI="https://a/b.git https://c/d.git"
# @CODE

fi
Loading