chore: do not use f-strings in logging statements (#32)

## Description

Thanks to the tip in
#30 (comment),
I enabled a Ruff rule to avoid using f-strings in future logging
statements we add: https://docs.astral.sh/ruff/rules/logging-f-string/,
and converted the previous ones to comply with the rule. It's not
necessarily required, but it's good to enable best practices.

> [!IMPORTANT]  
> This PR should be merged with a merge commit, not squashed.

Author: agriyakhetarpal

.git-blame-ignore-revs

@@ -0,0 +1,2 @@
+# Enable Ruff G004 to enforce logging best practices
+4c3043200ca15ade613fa3e3f8ca84bbe3522f21

pyodide_build/_py_compile.py

@@ -114,7 +114,7 @@ def _compile(
     output_name = output_path.name
 
     with set_log_level(logger, verbose):
-        logger.debug(f"Running py-compile on {input_path} to {output_path}")
+        logger.debug("Running py-compile on %s to %s", input_path, output_path)
 
         if compression_level > 0:
             compression = zipfile.ZIP_DEFLATED

pyodide_build/bash_runner.py

@@ -76,11 +76,11 @@ def run(
         if cwd is None:
             cwd = Path.cwd()
         cwd = Path(cwd).absolute()
-        logger.info(f"Running {script_name} in {str(cwd)}")
+        logger.info("Running %s in %s", script_name, str(cwd))
         opts["cwd"] = cwd
         result = self.run_unchecked(cmd, **opts)
         if result.returncode != 0:
-            logger.error(f"ERROR: {script_name} failed")
+            logger.error("ERROR: %s failed", script_name)
             logger.error(textwrap.indent(cmd, "    "))
             exit_with_stdio(result)
         return result

pyodide_build/buildall.py

@@ -434,7 +434,7 @@ def generate_dependency_graph(
 
     if disabled_packages:
         logger.warning(
-            f"The following packages are disabled: {', '.join(disabled_packages)}"
+            "The following packages are disabled: %s", ", ".join(disabled_packages)
         )
 
     return pkg_map
@@ -688,17 +688,17 @@ def build_from_graph(
 
     if already_built:
         logger.info(
-            "The following packages are already built: "
-            f"[bold]{format_name_list(sorted(already_built))}[/bold]"
+            "The following packages are already built: [bold]%s[/bold]",
+            format_name_list(sorted(already_built)),
         )
     if not needs_build:
         logger.success("All packages already built. Quitting.")
         return
 
     sorted_needs_build = sorted(needs_build)
     logger.info(
-        "Building the following packages: "
-        f"[bold]{format_name_list(sorted_needs_build)}[/bold]"
+        "Building the following packages: [bold]%s[/bold]",
+        format_name_list(sorted_needs_build),
     )
     build_state = _GraphBuilder(pkg_map, build_args, build_dir, set(needs_build))
     try:
@@ -850,14 +850,14 @@ def copy_logs(pkg_map: dict[str, BasePackage], log_dir: Path) -> None:
     """
 
     log_dir.mkdir(exist_ok=True, parents=True)
-    logger.info(f"Copying build logs to {log_dir}")
+    logger.info("Copying build logs to %s", log_dir)
 
     for pkg in pkg_map.values():
         log_file = pkg.pkgdir / "build.log"
         if log_file.exists():
             shutil.copy(log_file, log_dir / f"{pkg.name}.log")
         else:
-            logger.warning(f"Warning: {pkg.name} has no build log")
+            logger.warning("Warning: %s has no build log", pkg.name)
 
 
 def install_packages(
@@ -881,7 +881,7 @@ def install_packages(
 
     output_dir.mkdir(exist_ok=True, parents=True)
 
-    logger.info(f"Copying built packages to {output_dir}")
+    logger.info("Copying built packages to %s", output_dir)
     copy_packages_to_dist_dir(
         pkg_map.values(),
         output_dir,
@@ -890,7 +890,7 @@ def install_packages(
     )
 
     lockfile_path = output_dir / "pyodide-lock.json"
-    logger.info(f"Writing pyodide-lock.json to {lockfile_path}")
+    logger.info("Writing pyodide-lock.json to %s", lockfile_path)
 
     package_data = generate_lockfile(output_dir, pkg_map)
     package_data.to_json(lockfile_path)

pyodide_build/buildpkg.py

@@ -130,7 +130,7 @@ def build(self) -> None:
 
         t0 = datetime.now()
         timestamp = t0.strftime("%Y-%m-%d %H:%M:%S")
-        logger.info(f"[{timestamp}] Building package {self.name}...")
+        logger.info("[%s] Building package %s...", timestamp, self.name)
         success = True
         try:
             self._build()
@@ -439,7 +439,7 @@ def _package_wheel(
             # Retag platformed wheels to pyodide
             wheel = retag_wheel(wheel, wheel_platform())
 
-        logger.info(f"Unpacking wheel to {str(wheel)}")
+        logger.info("Unpacking wheel to %s", str(wheel))
 
         name, ver, _ = wheel.name.split("-", 2)
 
@@ -509,7 +509,7 @@ def _patch(self) -> None:
                 cwd=self.src_extract_dir,
             )
             if result.returncode != 0:
-                logger.error(f"ERROR: Patch {patch_abspath} failed")
+                logger.error("ERROR: Patch %s failed", patch_abspath)
                 exit_with_stdio(result)
 
         # Add any extra files
@@ -612,7 +612,7 @@ def copy_sharedlibs(
         logger.info("Copied shared libraries:")
         for lib, path in dep_map_new.items():
             original_path = dep_map[lib]
-            logger.info(f"  {original_path} -> {path}")
+            logger.info("  %s -> %s", original_path, path)
 
         return dep_map_new
 
@@ -695,7 +695,7 @@ def needs_rebuild(
     packaged_token = buildpath / ".packaged"
     if not packaged_token.is_file():
         logger.debug(
-            f"{pkg_root} needs rebuild because {packaged_token} does not exist"
+            "%s needs rebuild because %s does not exist", pkg_root, packaged_token
         )
         return True
 

pyodide_build/cli/skeleton.py

@@ -80,10 +80,10 @@ def new_recipe_pypi(
                 update_patched=update_patched,
             )
         except mkpkg.MkpkgFailedException as e:
-            logger.error(f"{name} update failed: {e}")
+            logger.error("%s update failed: %s", name, e)
             sys.exit(1)
         except mkpkg.MkpkgSkipped as e:
-            logger.warning(f"{name} update skipped: {e}")
+            logger.warning("%s update skipped: %s", name, e)
         except Exception:
             print(name)
             raise

pyodide_build/common.py

@@ -101,7 +101,7 @@ def _has_python_file(subdir: zipfile.Path) -> bool:
 
     if not top_level_imports:
         logger.warning(
-            f"WARNING: failed to parse top level import name from {whlfile}."
+            "WARNING: failed to parse top level import name from %s.", whlfile
         )
         return None
 
@@ -293,7 +293,7 @@ def unpack_wheel(wheel_path: Path, target_dir: Path | None = None) -> None:
         encoding="utf-8",
     )
     if result.returncode != 0:
-        logger.error(f"ERROR: Unpacking wheel {wheel_path.name} failed")
+        logger.error("ERROR: Unpacking wheel %s failed", wheel_path.name)
         exit_with_stdio(result)
 
 
@@ -306,7 +306,7 @@ def pack_wheel(wheel_dir: Path, target_dir: Path | None = None) -> None:
         encoding="utf-8",
     )
     if result.returncode != 0:
-        logger.error(f"ERROR: Packing wheel {wheel_dir} failed")
+        logger.error("ERROR: Packing wheel %s failed", wheel_dir)
         exit_with_stdio(result)
 
 
@@ -348,7 +348,7 @@ def retag_wheel(wheel_path: Path, platform: str) -> Path:
         capture_output=True,
     )
     if result.returncode != 0:
-        logger.error(f"ERROR: Retagging wheel {wheel_path} to {platform} failed")
+        logger.error("ERROR: Retagging wheel %s to %s failed", wheel_path, platform)
         exit_with_stdio(result)
     return wheel_path.parent / result.stdout.splitlines()[-1].strip()
 

pyodide_build/config.py

@@ -110,7 +110,8 @@ def _load_config_file(
             for key, v in configs["tool"]["pyodide"]["build"].items():
                 if key not in OVERRIDABLE_BUILD_KEYS:
                     logger.warning(
-                        f"WARNING: The provided build key {key} is either invalid or not overridable, hence ignored."
+                        "WARNING: The provided build key %s is either invalid or not overridable, hence ignored.",
+                        key,
                     )
                     continue
                 build_config[key] = _environment_substitute_str(v, env)

pyodide_build/mkpkg.py

@@ -151,7 +151,7 @@ def make_package(
     Creates a template that will work for most pure Python packages,
     but will have to be edited for more complex things.
     """
-    logger.info(f"Creating meta.yaml package for {package}")
+    logger.info("Creating meta.yaml package for %s", package)
 
     yaml = YAML()
 
@@ -226,7 +226,7 @@ def update_package(
 
     meta_path = root / package / "meta.yaml"
     if not meta_path.exists():
-        logger.error(f"{meta_path} does not exist")
+        logger.error("%s does not exist", meta_path)
         raise MkpkgFailedException(f"{package} recipe not found at {meta_path}")
 
     yaml_content = yaml.load(meta_path.read_bytes())
@@ -281,16 +281,19 @@ def update_package(
         return
 
     logger.info(
-        f"{package} is out of date:"
-        f" either {local_ver} < {pypi_ver}"
-        f" or checksums might have mismatched: received {sha256} against local {sha256_local} 🚨"
+        "%s is out of date: either %s < %s or checksums might have mismatched: received %s against local %s 🚨",
+        package,
+        local_ver,
+        pypi_ver,
+        sha256,
+        sha256_local,
     )
 
     if yaml_content["source"].get("patches"):
         if update_patched:
             logger.warning(
-                f"Pyodide applies patches to {package}. Update the "
-                "patches (if needed) to avoid build failing."
+                "Pyodide applies patches to %s. Update the patches (if needed) to avoid build failing.",
+                package,
             )
         else:
             raise MkpkgFailedException(

pyodide_build/out_of_tree/pypi.py

@@ -95,7 +95,7 @@ def _get_built_wheel_internal(url):
             source_path = build_path / files[0]
         else:
             source_path = build_path
-    logger.info(f"Building wheel for {gz_name}...")
+    logger.info("Building wheel for %s...", gz_name)
     with (
         tempfile.NamedTemporaryFile(mode="w+") as logfile,
         stream_redirected(to=logfile, stream=sys.stdout),

pyodide_build/out_of_tree/venv.py

@@ -254,11 +254,11 @@ def install_stdlib(venv_bin: Path) -> None:
 
 def create_pyodide_venv(dest: Path) -> None:
     """Create a Pyodide virtualenv and store it into dest"""
-    logger.info(f"Creating Pyodide virtualenv at {dest}")
+    logger.info("Creating Pyodide virtualenv at %s", dest)
     from virtualenv import session_via_cli
 
     if dest.exists():
-        logger.error(f"ERROR: dest directory '{dest}' already exists")
+        logger.error("ERROR: dest directory '%s' already exists", dest)
         sys.exit(1)
 
     interp_path = pyodide_dist_dir() / "python"

pyodide_build/recipe.py

@@ -87,8 +87,9 @@ def load_recipes(
 
         elif name_or_tag in ("core", "min-scipy-stack"):
             logger.warning(
-                f"Using meta package without the 'tag:' prefix is deprecated,"
-                f" use 'tag:{name_or_tag}' instead."
+                "Using meta package without the 'tag:' prefix is deprecated, "
+                "use 'tag:%s' instead.",
+                name_or_tag,
             )
             for recipe in tagged_recipes[name_or_tag]:
                 recipes[recipe.package.name] = recipe.model_copy(deep=True)

pyodide_build/xbuildenv.py

@@ -106,7 +106,7 @@ def use_version(self, version: str) -> None:
         version
             The version of xbuildenv to use.
         """
-        logger.info(f"Using Pyodide cross-build environment version: {version}")
+        logger.info("Using Pyodide cross-build environment version: %s", version)
 
         version_path = self._path_for_version(version)
         if not version_path.exists():
@@ -358,7 +358,8 @@ def _create_package_index(self, xbuildenv_pyodide_root: Path, version: str) -> N
 
         if not lockfile_path.exists():
             logger.warning(
-                f"Pyodide lockfile not found at {lockfile_path}. Skipping PyPI index creation"
+                "Pyodide lockfile not found at %s. Skipping PyPI index creation",
+                lockfile_path,
             )
             return
 

pyproject.toml

@@ -125,7 +125,11 @@ lint.select = [
   "PLC",    # pylint conventions
   "PLE",    # pylint errors
   "UP",     # pyupgrade
+  "G004",   # f-string logging should be avoided
 ]
+
+lint.logger-objects = ["pyodide_build.logger.logger"]
+
 lint.ignore = ["E402", "E501", "E731", "E741", "UP038"]
 # line-length = 219  # E501: Recommended goal is 88 to match black
 target-version = "py311"