Add Calamares build recipe and document it - #1
Conversation
Vendor the AUR PKGBUILD (calamares 3.4.2-2) with the yay/paru support patches and version tracking, and expand the README to explain why this repo exists and how to build and publish the package.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
0001-support-paru.patch (2)
89-146: 💤 Low valueConsider extracting repeated environment setup.
Each method imports
osand sets the same environment variables. This could be extracted to a helper method or done once in__init__.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@0001-support-paru.patch` around lines 89 - 146, The four methods (install, remove, update_db, update_system) repeatedly import os and set PWD and XDG_CACHE_HOME; extract that into a single helper (e.g., _ensure_cache_env or set_cache_env) and call it from each method or set these once in __init__ to avoid duplication; update references in install, remove, update_db, update_system to call the new helper before building/running commands and ensure run_paru/reset_progress usage remains unchanged.
26-26: 💤 Low valueUnused variable
progress_match.The compiled regex is never referenced after this line.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@0001-support-paru.patch` at line 26, The new regex assignment to progress_match is unused; either remove the unused variable or apply it where intended: locate the re.compile("^\\((\\d+)/(\\d+)\\)") instantiation (progress_match) and either delete that line if progress parsing is unnecessary, or use progress_match to match progress strings (e.g., call progress_match.match(...) and extract groups) in the function that processes command output (where progress parsing occurs) so the compiled pattern is actually consumed.0001-support-yay.patch (1)
19-147: ⚖️ Poor tradeoffSignificant code duplication between PMYay and PMParu.
The PMYay and PMParu classes are nearly identical, differing only in the command name (
yayvsparu) and configuration key. Consider extracting a common base class to reduce duplication and ensure bug fixes apply to both backends.Also applies to: 151-162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@0001-support-yay.patch` around lines 19 - 147, PMYay and PMParu duplicate the same logic (backend name, config parsing, environment setup, progress/reset and run/install/remove/update methods). Refactor by extracting a shared base (e.g., PMAURBase) that contains the common implementations of __init__ (accepting a config_key and cmd_name), reset_progress, run_command (generic version of run_yay), install, remove, update_db, update_system and common attributes (progress_fraction, in_package_changes, config-parsed retry/timeout/needed flags); have PMYay and PMParu subclass this base and only set backend/cmd_name and the configuration key (replace usages of self.yay_* with generic names like self.num_retries/self_disable_timeout/self_needed_only) so the duplicated code in PMYay methods (reset_progress, run_yay, install, remove, update_db, update_system) is removed and both backends reuse the base implementation.PKGBUILD (2)
34-47: ⚡ Quick winDocument rationale for skipped modules.
The
_skip_moduleslist disables 12 Calamares modules without explanation. Adding brief comments would help future maintainers understand why each module is excluded (e.g., "test/example modules", "OpenRC-specific", "not needed for Mika").📝 Add explanatory comments
local _skip_modules=( + # dracut/initramfs: distribution-specific, may use alternative dracut dracutlukscfg + # dummy modules: test/example modules not needed in production dummycpp dummyprocess dummypython dummypythonqt initramfs initramfscfg + # interactiveterminal: not required for standard installation flow interactiveterminal + # packagechooser: not needed for this distribution packagechooser packagechooserq + # services-openrc: Arch uses systemd, not OpenRC services-openrc )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PKGBUILD` around lines 34 - 47, The _skip_modules array currently lists 12 Calamares modules without rationale; edit the PKGBUILD to add brief inline comments for each entry in the _skip_modules array (e.g., after dracut, dracutlukscfg mark "initramfs helpers not used"; after dummycpp, dummyprocess, dummypython, dummypythonqt mark "example/test modules"; after initramfs, initramfscfg mark "handled by distribution initramfs"; after interactiveterminal mark "UI not required"; after packagechooser, packagechooserq mark "custom installer UI used"; and after services-openrc mark "OpenRC-specific - not supported by Mika") so future maintainers can quickly understand why each module (referenced by name in _skip_modules) is excluded.
49-50: 💤 Low valueClarify or remove redundant comment.
The comment "Correct source directory (inside src/)" doesn't add much clarity. The variable name
_pkgsrc_dirand its value already make the purpose clear. Consider either removing the comment or making it more informative (e.g., "Source extracted to $srcdir by makepkg").🧹 Simplify or improve comment
- # Correct source directory (inside src/) local _pkgsrc_dir="$srcdir/$_pkgname-$pkgver"Or make it more informative:
- # Correct source directory (inside src/) + # Path to extracted source (makepkg extracts to $srcdir) local _pkgsrc_dir="$srcdir/$_pkgname-$pkgver"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PKGBUILD` around lines 49 - 50, The comment above the _pkgsrc_dir assignment is redundant; update or remove it: either delete the line starting with "#" before the assignment to _pkgsrc_dir, or replace it with a concise, informative comment such as "Source extracted to $srcdir by makepkg" that explains why the path uses $srcdir/$_pkgname-$pkgver; ensure the comment references _pkgsrc_dir and its purpose (the package source directory inside $srcdir).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.SRCINFO:
- Around line 20-21: The .SRCINFO's source array is missing the patch files
referenced in the PR/README; add entries for the patch filenames (e.g.
0001-support-yay.patch and 0001-support-paru.patch) to the source list alongside
the main tarball and add corresponding sha256sums entries (use the real
checksums or SKIP if kept locally) so the patches can be fetched/applied during
build; ensure the filenames exactly match the patch files referenced in the
repository and update the sha256sums field for each new source entry.
In `@0001-support-paru.patch`:
- Around line 60-63: reset_progress currently computes self.progress_fraction =
(completed_packages * 1.0 / total_packages) which can raise ZeroDivisionError if
total_packages == 0; update reset_progress to guard against zero by checking
total_packages (or self.total_packages if you refactor to instance vars) and set
self.progress_fraction = 0.0 when total_packages is 0, otherwise compute the
fraction using float conversion (e.g., completed_packages /
float(total_packages)) and keep existing self.in_package_changes reset behavior.
- Around line 77-87: The code has a hardcoded "if False:" that disables the
callback path; replace that with a real check of the callback parameter so
progress reporting runs when provided. Specifically, inside the block that
currently contains "if False: # callback:" change the condition to "if
callback:" and pass the actual callback (not always self.line_cb) to
libcalamares.utils.target_env_process_output(command, callback); otherwise call
libcalamares.utils.target_env_process_output(command) as before, leaving the
surrounding try/except and the paru_count/self.paru_num_retries logic unchanged.
- Around line 29-31: The sudoers write uses sudoers_entry and
shutil.which("pacman") but doesn't ensure the pacman path exists, doesn't use a
context manager, and doesn't set the required 0440 permissions; fix by resolving
pacman_path = shutil.which("pacman") and aborting (or skipping) if None, open
the target with a with-statement (e.g., context manager) to ensure cleanup,
write a single line using the resolved pacman_path, flush/close via the context
manager, then set strict permissions with os.chmod("/etc/sudoers.d/autoupdate",
0o440) (or write to a temp file and atomically replace) so the file is valid and
secure.
In `@0001-support-yay.patch`:
- Around line 60-63: The reset_progress method currently computes
self.progress_fraction = (completed_packages * 1.0 / total_packages) which will
raise ZeroDivisionError when total_packages is 0; update reset_progress to guard
against division by zero by checking total_packages (the global) before dividing
and set self.progress_fraction to 0.0 (or an appropriate default) when
total_packages is zero, keeping completed_packages and total_packages as globals
and preserving the existing behavior when total_packages > 0.
- Around line 77-87: Replace the hardcoded "if False:" branch so the callback is
actually used: call libcalamares.utils.target_env_process_output(command,
self.line_cb) when a callback is provided (e.g. when the accepted callback
parameter or self.line_cb is truthy), otherwise call
libcalamares.utils.target_env_process_output(command) without the callback; keep
the subprocess.CalledProcessError retry logic intact (refer to
self.yay_num_retries and yay_count) but ensure the branch uses the real callback
check instead of False.
- Around line 29-31: The snippet writing /etc/sudoers.d/autoupdate must ensure
correct file mode, handle missing pacman path, and properly close resources:
change the write to use a context manager (replace sudoers_entry open/close with
a with open(...) as f) and compute pacman_path = shutil.which("pacman") and
abort/log/skip if pacman_path is None; write "nobody ALL = NOPASSWD: " +
pacman_path, then call os.chmod on the created file to set mode 0o440; also
address the filename collision with the paru patch by either adding a clarifying
comment about mutually exclusive backends or using a distinct sudoers filename
(refer to the variables/functions in this diff: sudoers_entry and
shutil.which("pacman")).
In `@PKGBUILD`:
- Around line 1-2: The Maintainer header in the PKGBUILD is empty; update the
"Maintainer:" line to include the primary maintainer's full name and contact
(e.g., email) so it reads like "Maintainer: Full Name <email@example.com>" to
clearly identify responsibility and provide a contact for users and package
consumers.
- Around line 29-31: The PKGBUILD's source array currently only references
"$_pkgname-$pkgver.$_pkgext" and must also include the patch files
"0001-support-yay.patch" and "0001-support-paru.patch" (add them to the source
array and use "SKIP" for their sha256sums if kept locally, or add real
checksums), and add a prepare() function that applies those patches (use patch
-p1 or the project's recommended patch level) before build; update the
sha256sums array accordingly and ensure prepare() references the same patch
filenames.
---
Nitpick comments:
In `@0001-support-paru.patch`:
- Around line 89-146: The four methods (install, remove, update_db,
update_system) repeatedly import os and set PWD and XDG_CACHE_HOME; extract that
into a single helper (e.g., _ensure_cache_env or set_cache_env) and call it from
each method or set these once in __init__ to avoid duplication; update
references in install, remove, update_db, update_system to call the new helper
before building/running commands and ensure run_paru/reset_progress usage
remains unchanged.
- Line 26: The new regex assignment to progress_match is unused; either remove
the unused variable or apply it where intended: locate the
re.compile("^\\((\\d+)/(\\d+)\\)") instantiation (progress_match) and either
delete that line if progress parsing is unnecessary, or use progress_match to
match progress strings (e.g., call progress_match.match(...) and extract groups)
in the function that processes command output (where progress parsing occurs) so
the compiled pattern is actually consumed.
In `@0001-support-yay.patch`:
- Around line 19-147: PMYay and PMParu duplicate the same logic (backend name,
config parsing, environment setup, progress/reset and run/install/remove/update
methods). Refactor by extracting a shared base (e.g., PMAURBase) that contains
the common implementations of __init__ (accepting a config_key and cmd_name),
reset_progress, run_command (generic version of run_yay), install, remove,
update_db, update_system and common attributes (progress_fraction,
in_package_changes, config-parsed retry/timeout/needed flags); have PMYay and
PMParu subclass this base and only set backend/cmd_name and the configuration
key (replace usages of self.yay_* with generic names like
self.num_retries/self_disable_timeout/self_needed_only) so the duplicated code
in PMYay methods (reset_progress, run_yay, install, remove, update_db,
update_system) is removed and both backends reuse the base implementation.
In `@PKGBUILD`:
- Around line 34-47: The _skip_modules array currently lists 12 Calamares
modules without rationale; edit the PKGBUILD to add brief inline comments for
each entry in the _skip_modules array (e.g., after dracut, dracutlukscfg mark
"initramfs helpers not used"; after dummycpp, dummyprocess, dummypython,
dummypythonqt mark "example/test modules"; after initramfs, initramfscfg mark
"handled by distribution initramfs"; after interactiveterminal mark "UI not
required"; after packagechooser, packagechooserq mark "custom installer UI
used"; and after services-openrc mark "OpenRC-specific - not supported by Mika")
so future maintainers can quickly understand why each module (referenced by name
in _skip_modules) is excluded.
- Around line 49-50: The comment above the _pkgsrc_dir assignment is redundant;
update or remove it: either delete the line starting with "#" before the
assignment to _pkgsrc_dir, or replace it with a concise, informative comment
such as "Source extracted to $srcdir by makepkg" that explains why the path uses
$srcdir/$_pkgname-$pkgver; ensure the comment references _pkgsrc_dir and its
purpose (the package source directory inside $srcdir).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dcd41aa-30f1-4480-a6be-9c0de8c6665a
📒 Files selected for processing (6)
.SRCINFO.nvchecker.toml0001-support-paru.patch0001-support-yay.patchPKGBUILDREADME.md
| source = calamares-3.4.2.tar.gz::https://codeberg.org/Calamares/calamares/releases/download/v3.4.2/calamares-3.4.2.tar.gz | ||
| sha256sums = 733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc |
There was a problem hiding this comment.
Patches referenced in PR description are missing from source array.
The PR description states "Includes yay/paru support patches" and the repository documentation mentions 0001-support-yay.patch and 0001-support-paru.patch, but these patches are not listed in the source array. Without including them here, they cannot be applied during the package build, and the resulting package will lack the advertised AUR helper support.
📦 Add patches to source array
The .SRCINFO file should list all patch files after the main source tarball:
source = calamares-3.4.2.tar.gz::https://codeberg.org/Calamares/calamares/releases/download/v3.4.2/calamares-3.4.2.tar.gz
+source = 0001-support-yay.patch
+source = 0001-support-paru.patch
sha256sums = 733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc
+sha256sums = SKIP
+sha256sums = SKIPNote: Replace SKIP with actual SHA256 checksums once patches are finalized, or use SKIP if patches are maintained locally alongside the PKGBUILD.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| source = calamares-3.4.2.tar.gz::https://codeberg.org/Calamares/calamares/releases/download/v3.4.2/calamares-3.4.2.tar.gz | |
| sha256sums = 733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc | |
| source = calamares-3.4.2.tar.gz::https://codeberg.org/Calamares/calamares/releases/download/v3.4.2/calamares-3.4.2.tar.gz | |
| source = 0001-support-yay.patch | |
| source = 0001-support-paru.patch | |
| sha256sums = 733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc | |
| sha256sums = SKIP | |
| sha256sums = SKIP |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.SRCINFO around lines 20 - 21, The .SRCINFO's source array is missing the
patch files referenced in the PR/README; add entries for the patch filenames
(e.g. 0001-support-yay.patch and 0001-support-paru.patch) to the source list
alongside the main tarball and add corresponding sha256sums entries (use the
real checksums or SKIP if kept locally) so the patches can be fetched/applied
during build; ensure the filenames exactly match the patch files referenced in
the repository and update the sha256sums field for each new source entry.
| + sudoers_entry = open("/etc/sudoers.d/autoupdate", "w") | ||
| + sudoers_entry.write("nobody ALL = NOPASSWD: " + shutil.which("pacman")) | ||
| + sudoers_entry.close() |
There was a problem hiding this comment.
Sudoers file written without required permissions and missing resource cleanup.
Sudoers include files require mode 0440 (or stricter). Without setting permissions, the file may be rejected by sudo or create a security vulnerability. Additionally, shutil.which("pacman") returns None if pacman is not found, which would write an invalid sudoers entry.
🔒 Proposed fix using context manager and proper permissions
+ import os
+
+ pacman_path = shutil.which("pacman")
+ if pacman_path is None:
+ raise RuntimeError("pacman not found in PATH")
+
# Note that this writes to /etc/sudoers.d, NOT /etc/sudoers!
- sudoers_entry = open("/etc/sudoers.d/autoupdate", "w")
- sudoers_entry.write("nobody ALL = NOPASSWD: " + shutil.which("pacman"))
- sudoers_entry.close()
+ sudoers_path = "/etc/sudoers.d/autoupdate"
+ with open(sudoers_path, "w") as sudoers_entry:
+ sudoers_entry.write("nobody ALL = NOPASSWD: " + pacman_path + "\n")
+ os.chmod(sudoers_path, 0o440)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@0001-support-paru.patch` around lines 29 - 31, The sudoers write uses
sudoers_entry and shutil.which("pacman") but doesn't ensure the pacman path
exists, doesn't use a context manager, and doesn't set the required 0440
permissions; fix by resolving pacman_path = shutil.which("pacman") and aborting
(or skipping) if None, open the target with a with-statement (e.g., context
manager) to ensure cleanup, write a single line using the resolved pacman_path,
flush/close via the context manager, then set strict permissions with
os.chmod("/etc/sudoers.d/autoupdate", 0o440) (or write to a temp file and
atomically replace) so the file is valid and secure.
| + def reset_progress(self): | ||
| + self.in_package_changes = False | ||
| + # These are globals | ||
| + self.progress_fraction = (completed_packages * 1.0 / total_packages) |
There was a problem hiding this comment.
Potential division by zero in reset_progress.
If total_packages is 0 (e.g., empty package list), this will raise ZeroDivisionError.
🐛 Proposed fix
def reset_progress(self):
self.in_package_changes = False
# These are globals
- self.progress_fraction = (completed_packages * 1.0 / total_packages)
+ self.progress_fraction = (completed_packages * 1.0 / total_packages) if total_packages > 0 else 0.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + def reset_progress(self): | |
| + self.in_package_changes = False | |
| + # These are globals | |
| + self.progress_fraction = (completed_packages * 1.0 / total_packages) | |
| def reset_progress(self): | |
| self.in_package_changes = False | |
| # These are globals | |
| self.progress_fraction = (completed_packages * 1.0 / total_packages) if total_packages > 0 else 0.0 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@0001-support-paru.patch` around lines 60 - 63, reset_progress currently
computes self.progress_fraction = (completed_packages * 1.0 / total_packages)
which can raise ZeroDivisionError if total_packages == 0; update reset_progress
to guard against zero by checking total_packages (or self.total_packages if you
refactor to instance vars) and set self.progress_fraction = 0.0 when
total_packages is 0, otherwise compute the fraction using float conversion
(e.g., completed_packages / float(total_packages)) and keep existing
self.in_package_changes reset behavior.
| + if False: # callback: | ||
| + libcalamares.utils.target_env_process_output(command, self.line_cb) | ||
| + else: | ||
| + libcalamares.utils.target_env_process_output(command) | ||
| + | ||
| + return | ||
| + except subprocess.CalledProcessError: | ||
| + if paru_count <= self.paru_num_retries: | ||
| + pass | ||
| + else: | ||
| + raise |
There was a problem hiding this comment.
Callback functionality is disabled by hardcoded if False:.
The callback parameter is accepted but never used due to if False:. This disables the entire progress reporting mechanism, meaning custom_status_message updates and setprogress calls in line_cb will never execute during package operations.
This appears to be leftover debug code that should either be removed or restored to use the actual callback parameter.
🐛 Proposed fix
try:
- if False: # callback:
+ if callback:
libcalamares.utils.target_env_process_output(command, self.line_cb)
else:
libcalamares.utils.target_env_process_output(command)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + if False: # callback: | |
| + libcalamares.utils.target_env_process_output(command, self.line_cb) | |
| + else: | |
| + libcalamares.utils.target_env_process_output(command) | |
| + | |
| + return | |
| + except subprocess.CalledProcessError: | |
| + if paru_count <= self.paru_num_retries: | |
| + pass | |
| + else: | |
| + raise | |
| if callback: | |
| libcalamares.utils.target_env_process_output(command, self.line_cb) | |
| else: | |
| libcalamares.utils.target_env_process_output(command) | |
| return | |
| except subprocess.CalledProcessError: | |
| if paru_count <= self.paru_num_retries: | |
| pass | |
| else: | |
| raise |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@0001-support-paru.patch` around lines 77 - 87, The code has a hardcoded "if
False:" that disables the callback path; replace that with a real check of the
callback parameter so progress reporting runs when provided. Specifically,
inside the block that currently contains "if False: # callback:" change the
condition to "if callback:" and pass the actual callback (not always
self.line_cb) to libcalamares.utils.target_env_process_output(command,
callback); otherwise call libcalamares.utils.target_env_process_output(command)
as before, leaving the surrounding try/except and the
paru_count/self.paru_num_retries logic unchanged.
| + sudoers_entry = open("/etc/sudoers.d/autoupdate", "w") | ||
| + sudoers_entry.write("nobody ALL = NOPASSWD: " + shutil.which("pacman")) | ||
| + sudoers_entry.close() |
There was a problem hiding this comment.
Sudoers file written without required permissions and missing resource cleanup.
Same issue as paru patch. Sudoers include files require mode 0440. Additionally, shutil.which("pacman") returns None if pacman is not found.
Note: Both paru and yay patches write to the same file /etc/sudoers.d/autoupdate. If this is intentional (mutually exclusive backends), consider adding a comment. Otherwise, use distinct filenames.
🔒 Proposed fix
+ import os
+
+ pacman_path = shutil.which("pacman")
+ if pacman_path is None:
+ raise RuntimeError("pacman not found in PATH")
+
# Note that this writes to /etc/sudoers.d, NOT /etc/sudoers!
- sudoers_entry = open("/etc/sudoers.d/autoupdate", "w")
- sudoers_entry.write("nobody ALL = NOPASSWD: " + shutil.which("pacman"))
- sudoers_entry.close()
+ sudoers_path = "/etc/sudoers.d/autoupdate"
+ with open(sudoers_path, "w") as sudoers_entry:
+ sudoers_entry.write("nobody ALL = NOPASSWD: " + pacman_path + "\n")
+ os.chmod(sudoers_path, 0o440)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@0001-support-yay.patch` around lines 29 - 31, The snippet writing
/etc/sudoers.d/autoupdate must ensure correct file mode, handle missing pacman
path, and properly close resources: change the write to use a context manager
(replace sudoers_entry open/close with a with open(...) as f) and compute
pacman_path = shutil.which("pacman") and abort/log/skip if pacman_path is None;
write "nobody ALL = NOPASSWD: " + pacman_path, then call os.chmod on the created
file to set mode 0o440; also address the filename collision with the paru patch
by either adding a clarifying comment about mutually exclusive backends or using
a distinct sudoers filename (refer to the variables/functions in this diff:
sudoers_entry and shutil.which("pacman")).
| + def reset_progress(self): | ||
| + self.in_package_changes = False | ||
| + # These are globals | ||
| + self.progress_fraction = (completed_packages * 1.0 / total_packages) |
There was a problem hiding this comment.
Potential division by zero in reset_progress.
If total_packages is 0, this will raise ZeroDivisionError.
🐛 Proposed fix
def reset_progress(self):
self.in_package_changes = False
# These are globals
- self.progress_fraction = (completed_packages * 1.0 / total_packages)
+ self.progress_fraction = (completed_packages * 1.0 / total_packages) if total_packages > 0 else 0.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + def reset_progress(self): | |
| + self.in_package_changes = False | |
| + # These are globals | |
| + self.progress_fraction = (completed_packages * 1.0 / total_packages) | |
| def reset_progress(self): | |
| self.in_package_changes = False | |
| # These are globals | |
| self.progress_fraction = (completed_packages * 1.0 / total_packages) if total_packages > 0 else 0.0 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@0001-support-yay.patch` around lines 60 - 63, The reset_progress method
currently computes self.progress_fraction = (completed_packages * 1.0 /
total_packages) which will raise ZeroDivisionError when total_packages is 0;
update reset_progress to guard against division by zero by checking
total_packages (the global) before dividing and set self.progress_fraction to
0.0 (or an appropriate default) when total_packages is zero, keeping
completed_packages and total_packages as globals and preserving the existing
behavior when total_packages > 0.
| + if False: # callback: | ||
| + libcalamares.utils.target_env_process_output(command, self.line_cb) | ||
| + else: | ||
| + libcalamares.utils.target_env_process_output(command) | ||
| + | ||
| + return | ||
| + except subprocess.CalledProcessError: | ||
| + if yay_count <= self.yay_num_retries: | ||
| + pass | ||
| + else: | ||
| + raise |
There was a problem hiding this comment.
Callback functionality is disabled by hardcoded if False:.
Same issue as in the paru patch. The callback parameter is accepted but never used due to if False:, disabling the progress reporting mechanism entirely.
🐛 Proposed fix
try:
- if False: # callback:
+ if callback:
libcalamares.utils.target_env_process_output(command, self.line_cb)
else:
libcalamares.utils.target_env_process_output(command)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + if False: # callback: | |
| + libcalamares.utils.target_env_process_output(command, self.line_cb) | |
| + else: | |
| + libcalamares.utils.target_env_process_output(command) | |
| + | |
| + return | |
| + except subprocess.CalledProcessError: | |
| + if yay_count <= self.yay_num_retries: | |
| + pass | |
| + else: | |
| + raise | |
| try: | |
| if callback: | |
| libcalamares.utils.target_env_process_output(command, self.line_cb) | |
| else: | |
| libcalamares.utils.target_env_process_output(command) | |
| return | |
| except subprocess.CalledProcessError: | |
| if yay_count <= self.yay_num_retries: | |
| pass | |
| else: | |
| raise |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@0001-support-yay.patch` around lines 77 - 87, Replace the hardcoded "if
False:" branch so the callback is actually used: call
libcalamares.utils.target_env_process_output(command, self.line_cb) when a
callback is provided (e.g. when the accepted callback parameter or self.line_cb
is truthy), otherwise call libcalamares.utils.target_env_process_output(command)
without the callback; keep the subprocess.CalledProcessError retry logic intact
(refer to self.yay_num_retries and yay_count) but ensure the branch uses the
real callback check instead of False.
| # Maintainer: | ||
| # Contributor: Rustmilian Rustmilian@proton.me |
There was a problem hiding this comment.
Fill in the Maintainer field.
The Maintainer field is empty. Best practice is to specify the primary maintainer's name and contact information, especially for packages that will be distributed or used by others.
✏️ Add maintainer information
-# Maintainer:
+# Maintainer: Your Name <your.email@example.com>
# Contributor: Rustmilian Rustmilian@proton.me📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Maintainer: | |
| # Contributor: Rustmilian Rustmilian@proton.me | |
| # Maintainer: Your Name <your.email@example.com> | |
| # Contributor: Rustmilian Rustmilian@proton.me |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PKGBUILD` around lines 1 - 2, The Maintainer header in the PKGBUILD is empty;
update the "Maintainer:" line to include the primary maintainer's full name and
contact (e.g., email) so it reads like "Maintainer: Full Name
<email@example.com>" to clearly identify responsibility and provide a contact
for users and package consumers.
| _pkgext="tar.gz" | ||
| source=("$_pkgname-$pkgver.$_pkgext"::"$url/releases/download/v$pkgver/$_pkgname-$pkgver.$_pkgext") | ||
| sha256sums=('733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc') |
There was a problem hiding this comment.
Patches must be added to source array and applied in prepare() function.
The PKGBUILD only includes the main tarball but omits the 0001-support-yay.patch and 0001-support-paru.patch files mentioned in the PR description and repository documentation. Additionally, there is no prepare() function to apply these patches.
🩹 Add patches to source and create prepare() function
_pkgext="tar.gz"
-source=("$_pkgname-$pkgver.$_pkgext"::"$url/releases/download/v$pkgver/$_pkgname-$pkgver.$_pkgext")
-sha256sums=('733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc')
+source=(
+ "$_pkgname-$pkgver.$_pkgext"::"$url/releases/download/v$pkgver/$_pkgname-$pkgver.$_pkgext"
+ "0001-support-yay.patch"
+ "0001-support-paru.patch"
+)
+sha256sums=(
+ '733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc'
+ 'SKIP' # Replace with actual checksum
+ 'SKIP' # Replace with actual checksum
+)
+
+prepare() {
+ cd "$_pkgname-$pkgver"
+
+ # Apply Mika patches for AUR helper support
+ patch -Np1 -i "$srcdir/0001-support-yay.patch"
+ patch -Np1 -i "$srcdir/0001-support-paru.patch"
+}
build() {Note: Use SKIP for checksums if patches are maintained locally, or generate actual SHA256 checksums if they should be verified.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _pkgext="tar.gz" | |
| source=("$_pkgname-$pkgver.$_pkgext"::"$url/releases/download/v$pkgver/$_pkgname-$pkgver.$_pkgext") | |
| sha256sums=('733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc') | |
| _pkgext="tar.gz" | |
| source=( | |
| "$_pkgname-$pkgver.$_pkgext"::"$url/releases/download/v$pkgver/$_pkgname-$pkgver.$_pkgext" | |
| "0001-support-yay.patch" | |
| "0001-support-paru.patch" | |
| ) | |
| sha256sums=( | |
| '733bbbb00dc9f84874bd5c22960952f317ea2537565431179fa2152b2fbfdccc' | |
| 'SKIP' # Replace with actual checksum | |
| 'SKIP' # Replace with actual checksum | |
| ) | |
| prepare() { | |
| cd "$_pkgname-$pkgver" | |
| # Apply Mika patches for AUR helper support | |
| patch -Np1 -i "$srcdir/0001-support-yay.patch" | |
| patch -Np1 -i "$srcdir/0001-support-paru.patch" | |
| } | |
| build() { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PKGBUILD` around lines 29 - 31, The PKGBUILD's source array currently only
references "$_pkgname-$pkgver.$_pkgext" and must also include the patch files
"0001-support-yay.patch" and "0001-support-paru.patch" (add them to the source
array and use "SKIP" for their sha256sums if kept locally, or add real
checksums), and add a prepare() function that applies those patches (use patch
-p1 or the project's recommended patch level) before build; update the
sha256sums array accordingly and ensure prepare() references the same patch
filenames.
|
✅ Created PR with unit tests: #2 |
1 participant
Vendor the AUR PKGBUILD (calamares 3.4.2-2) with the yay/paru support patches and version tracking, and expand the README to explain why this repo exists and how to build and publish the package.
Summary by CodeRabbit
New Features
Chores
Documentation