Skip to content

fix(bundler): inline linuxdeploy plugin scripts#13

Open
tomerqodo wants to merge 2 commits intosentry_combined_20260121_augment_sentry_coderabbit_1_base_fixbundler_inline_linuxdeploy_plugin_scripts_pr154from
sentry_combined_20260121_augment_sentry_coderabbit_1_head_fixbundler_inline_linuxdeploy_plugin_scripts_pr154
Open

fix(bundler): inline linuxdeploy plugin scripts#13
tomerqodo wants to merge 2 commits intosentry_combined_20260121_augment_sentry_coderabbit_1_base_fixbundler_inline_linuxdeploy_plugin_scripts_pr154from
sentry_combined_20260121_augment_sentry_coderabbit_1_head_fixbundler_inline_linuxdeploy_plugin_scripts_pr154

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#154

Comment on lines +312 to +313
fs::set_permissions(path, fs::Permissions::from_mode(0o770)).map_err(FileWriteError::Permissions)?;
fs::write(path, data).map_err(FileWriteError::Write)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The write_and_make_executable function incorrectly calls fs::set_permissions() before fs::write(), causing an error when the target file does not yet exist.
Severity: HIGH

Suggested Fix

In the write_and_make_executable function, reverse the order of operations. Call fs::write(path, data) first to create the file, and then call fs::set_permissions(path, ...) to set its permissions. This ensures the file exists before its permissions are modified.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: crates/tauri-bundler/src/bundle/linux/appimage/mod.rs#L312-L313

Potential issue: The function `write_and_make_executable` attempts to set file
permissions using `fs::set_permissions()` before writing the file with `fs::write()`.
According to Rust documentation, `fs::set_permissions()` will return a `NotFound` error
if the file does not exist. This function is called for several tools (AppRun,
linuxdeploy, GStreamer plugin, AppImage plugin) only when the corresponding file does
not exist, which will cause the permission setting to fail and terminate the bundling
process on the first run.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +243 to 246
if gtk.exists() {
let data = include_bytes!("./linuxdeploy-plugin-gtk.sh");
write_and_make_executable(&gtk, data)?;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The condition to write the GTK plugin script is inverted (if gtk.exists()), preventing it from being created when it doesn't exist and causing the build to fail.
Severity: HIGH

Suggested Fix

Invert the condition for writing the GTK plugin script. Change if gtk.exists() to if !gtk.exists(). This will align its behavior with the other plugins in the function, ensuring the script is created when it is missing.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: crates/tauri-bundler/src/bundle/linux/appimage/mod.rs#L243-L246

Potential issue: The logic to create the `linuxdeploy-plugin-gtk.sh` script is inverted.
It uses the condition `if gtk.exists()`, meaning it only attempts to write the script if
it already exists. This prevents the script from being created on a fresh run when it is
missing. Consequently, when `linuxdeploy` is executed with the `--plugin gtk` flag, it
will fail to find the plugin, causing the AppImage bundling process to fail. Other
plugins in the same function correctly use `if !<file>.exists()`.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants