fedora-rust / rust2rpm

Clone
Source Code
GIT

#127 Optionally check if a package already exists in Fedora
Merged 2 years ago by zbyszek. Opened 2 years ago by dcavalca.
fedora-rust/ dcavalca/rust2rpm existance_check  into  master

file modified
+20 
@@ -27,6 +27,7 @@ 
 

  XDG_CACHE_HOME = os.getenv("XDG_CACHE_HOME", os.path.expanduser("~/.cache"))
 

  CACHEDIR = os.path.join(XDG_CACHE_HOME, "rust2rpm")
 

  API_URL = "https://crates.io/api/v1/"
 

+ DIST_GIT_URL = "https://src.fedoraproject.org/api/0/"
 

  JINJA_ENV = jinja2.Environment(
 

      loader=jinja2.ChoiceLoader([
 

          jinja2.FileSystemLoader(["/"]),
 
@@ -239,6 +240,15 @@ 
 

              if matcher.match(f) and not LICENSES.match(f) and not matcherex.match(f):
 

                  yield os.path.relpath(os.path.join(root, f), path)
 

  

+ def get_package_info(package):
 

+     url = requests.compat.urljoin(DIST_GIT_URL, f"rpms/{package}")
 

+     req = requests.get(url, headers={"User-Agent": "rust2rpm"})
 

+     json = req.json()
 

+     if "name" in json:
 

+         return json
 

+     else:
 

+         return None
 

+ 

  def make_diff_metadata(crate, version, patch=False, store=False):
 

      if _is_path(crate):
 

          # Only things that look like a paths are considered local arguments
 
@@ -294,6 +304,8 @@ 
 

                          help="Print mapping for specified license and exit")
 

      parser.add_argument("--no-auto-changelog-entry", action="store_true",
 

                          help="Do not generate a changelog entry")
 

+     parser.add_argument("--no-existence-check", action="store_true",
 

+                         help="Do not check whether the package already exists in dist-git")
 

      parser.add_argument("-", "--stdout", action="store_true",
 

                          help="Print spec and patches into stdout")
 

      parser.add_argument("-t", "--target", action="store",
 
@@ -374,6 +386,14 @@ 
 

      kwargs["pkg_suffix"] = suffix
 

      spec_file = pathlib.Path(f"rust-{metadata.name}{suffix}.spec")
 

  

+     if args.target in {"fedora"} and not args.no_existence_check and not os.path.exists(spec_file):
 

+         # No specfile, so this is probably a new package
 

+         package_info = get_package_info(f"rust-{metadata.name}{suffix}")
 

+         if package_info:
 

+             print(f"Crate {metadata.name}{suffix} is already packaged in Fedora ({package_info['full_url']}).")
 

+             print("Re-run with --no-existence-check if you still want to convert it.")
 

+             sys.exit(1)
 

+ 

      kwargs["auto_changelog_entry"] = not args.no_auto_changelog_entry
 

  

      if args.rpmautospec:

Basic implementation for #126. As @ngompa mentioned there, this will need some generalization to make it distro-independent, but it's a start.

I would prefer not to have this merged until we have the generalization in place.

Sure thing, I mostly put it up so it wouldn't get lost

existance → existence !

I would prefer not to have this merged until we have the generalization in place.

Yeah, we shouldn't break invocations on other distros. So please make the make the check conditional on distribution, i.e. args.target. I think it's fine to only provide an implementation for Fedora. People from other distros will add implementations for their distros as appropriate.

Also, please rebase.

Uhmmm ... what is "Re-run with --no-existance-check if you still want to convert it" supposed to mean?

Also, we're running rust2rpm for existing packages all the time, because changes in optional dependencies and feature flags need to be reflected in matching subpackages, for the dependency resolver to work correctly. Having this standard workflow blocked by an additional existence check that also makes network requests is a no-go.

So the "--no-existence-check" should really be the default behaviour, with a possible opt-in when trying to create a new package, where the additional delay by the network request also is not a problem because it will already do network access with an attempt to download the crate with the given name.

Thanks! I'll take some time later this week to better flesh this out and address the feedback here.

we're running rust2rpm for existing packages all the time, because changes in optional dependencies and feature flags need to be reflected in matching subpackages

rust2rpm should detect if it's a new package or an update. If it's a new package, it should warn. I think some heuristics would go a long way… Dunno, just a very simple check: if we are overwriting an existing spec file, don't do the check. If we're writing a new file, then assume that we're creating a new package, do the check, and warn if the package exists.

@dcavalca an update here would be great…

FWIW, in https://pagure.io/fedora-rust/rust2rpm/c/7e0afd5da2fc25c0c3bc2639bbcd402df49ad698?branch=master I needed to conditionalize on the spec file already existing. Maybe you could reuse the same simple logic here?

Sorry, other stuff got in the way. I'll try to get this wrapped up later in the week.

rebased onto 2aa1e027ce128867fe4900c752b2a23564e65f3b
2 years ago

Updated to check for the presence of a specfile.

rebased onto 8e5fc1fa8ce52a3525778c15e05ea48042ae9b43
2 years ago

Gate this to Fedora only for now

if default_target in {"fedora"}

Hmm, why default_target here, not args.target? If I'm on Fedora, but doing a package for a different distro, it doesn't make sense to check if a Fedora package exists.

Maybe change 'packaged in dist-git as …' to 'packaged in Fedora ({package_info['full_url']}).'
And 'convert it' → 'convert it.'.
The url is clickable and the user is likely to want to go there immediately to investigate how they missed the existence of the package ;)

LGTM, apart from the two comments above.

rebased onto bc04fe2
2 years ago

Updated, thanks @zbyszek!

Pull-Request has been merged by zbyszek
2 years ago
Metadata
None
No Tags
Changes Summary 1
+20 -0
file changed
rust2rpm/__main__.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.