Skip to content
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

Jump to bottom

feat: allow Linux dependencies to be specified via a file path #254

Merged
merged 3 commits into from
Jul 2, 2024
Merged

feat: allow Linux dependencies to be specified via a file path #254

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c8ef225
feat: allow Linux dependencies to be specified via a file path
kevinaboos Jun 28, 2024
596c5a4
Add .changes file
kevinaboos Jun 28, 2024
76f7e71
rustfmt fix
kevinaboos Jul 2, 2024
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
9 changes: 9 additions & 0 deletions .changes/pr254.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"cargo-packager": "minor"
"@crabnebula/packager": "minor"
---

Allow Linux dependencies to be specified via a file path instead of just a direct String.
This enables the list of dependencies to by dynamically generated for both Debian `.deb` packages and pacman packages,
which can relieve the app developer from the burden of manually maintaining a fixed list of dependencies.

52 changes: 35 additions & 17 deletions bindings/packager/nodejs/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,18 +771,19 @@
"additionalProperties": false
},
"DebianConfig": {
"description": "The Linux debian configuration.",
"description": "The Linux Debian configuration.",
"type": "object",
"properties": {
"depends": {
"description": "The list of debian dependencies.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
"description": "The list of Debian dependencies.",
"anyOf": [
{
"$ref": "#/definitions/Dependencies"
},
{
"type": "null"
}
]
},
"desktopTemplate": {
"description": "Path to a custom desktop file Handlebars template.\n\nAvailable variables: `categories`, `comment` (optional), `exec`, `icon` and `name`.\n\nDefault file contents: ```text [Desktop Entry] Categories={{categories}} {{#if comment}} Comment={{comment}} {{/if}} Exec={{exec}} Icon={{icon}} Name={{name}} Terminal=false Type=Application {{#if mime_type}} MimeType={{mime_type}} {{/if}} ```",
Expand Down Expand Up @@ -818,6 +819,22 @@
},
"additionalProperties": false
},
"Dependencies": {
"description": "A list of dependencies specified as either a list of Strings or as a path to a file that lists the dependencies, one per line.",
"anyOf": [
{
"description": "The list of dependencies provided directly as a vector of Strings.",
"type": "array",
"items": {
"type": "string"
}
},
{
"description": "A path to the file containing the list of dependences, formatted as one per line: ```text libc6 libxcursor1 libdbus-1-3 libasyncns0 ... ```",
"type": "string"
}
]
},
"AppImageConfig": {
"description": "The Linux AppImage configuration.",
"type": "object",
Expand Down Expand Up @@ -890,14 +907,15 @@
}
},
"depends": {
"description": "List of softwares that must be installed for the app to build and run.\n\nSee : <https://wiki.archlinux.org/title/PKGBUILD#provides>",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
"description": "List of softwares that must be installed for the app to build and run.\n\nSee : <https://wiki.archlinux.org/title/PKGBUILD#depends>",
"anyOf": [
{
"$ref": "#/definitions/Dependencies"
},
{
"type": "null"
}
]
},
"provides": {
"description": "Additional packages that are provided by this app.\n\nSee : <https://wiki.archlinux.org/title/PKGBUILD#provides>",
Expand Down
52 changes: 35 additions & 17 deletions crates/packager/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,18 +771,19 @@
"additionalProperties": false
},
"DebianConfig": {
"description": "The Linux debian configuration.",
"description": "The Linux Debian configuration.",
"type": "object",
"properties": {
"depends": {
"description": "The list of debian dependencies.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
"description": "The list of Debian dependencies.",
"anyOf": [
{
"$ref": "#/definitions/Dependencies"
},
{
"type": "null"
}
]
},
"desktopTemplate": {
"description": "Path to a custom desktop file Handlebars template.\n\nAvailable variables: `categories`, `comment` (optional), `exec`, `icon` and `name`.\n\nDefault file contents: ```text [Desktop Entry] Categories={{categories}} {{#if comment}} Comment={{comment}} {{/if}} Exec={{exec}} Icon={{icon}} Name={{name}} Terminal=false Type=Application {{#if mime_type}} MimeType={{mime_type}} {{/if}} ```",
Expand Down Expand Up @@ -818,6 +819,22 @@
},
"additionalProperties": false
},
"Dependencies": {
"description": "A list of dependencies specified as either a list of Strings or as a path to a file that lists the dependencies, one per line.",
"anyOf": [
{
"description": "The list of dependencies provided directly as a vector of Strings.",
"type": "array",
"items": {
"type": "string"
}
},
{
"description": "A path to the file containing the list of dependences, formatted as one per line: ```text libc6 libxcursor1 libdbus-1-3 libasyncns0 ... ```",
"type": "string"
}
]
},
"AppImageConfig": {
"description": "The Linux AppImage configuration.",
"type": "object",
Expand Down Expand Up @@ -890,14 +907,15 @@
}
},
"depends": {
"description": "List of softwares that must be installed for the app to build and run.\n\nSee : <https://wiki.archlinux.org/title/PKGBUILD#provides>",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
"description": "List of softwares that must be installed for the app to build and run.\n\nSee : <https://wiki.archlinux.org/title/PKGBUILD#depends>",
"anyOf": [
{
"$ref": "#/definitions/Dependencies"
},
{
"type": "null"
}
]
},
"provides": {
"description": "Additional packages that are provided by this app.\n\nSee : <https://wiki.archlinux.org/title/PKGBUILD#provides>",
Expand Down
85 changes: 76 additions & 9 deletions crates/packager/src/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ impl DeepLinkProtocol {
}
}

/// The Linux debian configuration.
/// The Linux Debian configuration.
#[derive(Clone, Debug, Default, Deserialize, Serialize)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
#[non_exhaustive]
pub struct DebianConfig {
/// The list of debian dependencies.
pub depends: Option<Vec<String>>,
/// The list of Debian dependencies.
pub depends: Option<Dependencies>,
/// Path to a custom desktop file Handlebars template.
///
/// Available variables: `categories`, `comment` (optional), `exec`, `icon` and `name`.
Expand Down Expand Up @@ -221,14 +221,25 @@ impl DebianConfig {
Self::default()
}

/// Set the list of debian dependencies.
/// Set the list of Debian dependencies directly using an iterator of strings.
pub fn depends<I, S>(mut self, depends: I) -> Self
where
I: IntoIterator<Item = S>,
S: Into<String>,
{
self.depends
.replace(depends.into_iter().map(Into::into).collect());
.replace(Dependencies::List(depends.into_iter().map(Into::into).collect()));
self
}

/// Set the list of Debian dependencies indirectly via a path to a file,
/// which must contain one dependency (a package name) per line.
pub fn depends_path<P>(mut self, path: P) -> Self
where
P: Into<PathBuf>
{
self.depends
.replace(Dependencies::Path(path.into()));
self
}

Expand Down Expand Up @@ -288,6 +299,49 @@ impl DebianConfig {
}
}


/// A list of dependencies specified as either a list of Strings
/// or as a path to a file that lists the dependencies, one per line.
#[derive(Debug, Clone, Deserialize, Serialize)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(untagged)]
#[non_exhaustive]
pub enum Dependencies {
/// The list of dependencies provided directly as a vector of Strings.
List(Vec<String>),
/// A path to the file containing the list of dependences, formatted as one per line:
/// ```text
/// libc6
/// libxcursor1
/// libdbus-1-3
/// libasyncns0
/// ...
/// ```
Path(PathBuf),
Comment on lines +311 to +319
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems a bit unconventional, how about adding support for specifying dependencies as environment variables, CARGO_PACKAGER_DEB_DEPS="dep1,dep2,dep3" and CARGO_PACKAGER_PACMAN_DEPS="dep1,dep2,dep3"?

Copy link
Contributor Author

@kevinaboos kevinaboos Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, a pair of env vars would also work, especially for simple cases like the one I gave in the rustdoc above. I can add that as well.

Would it be alright if we kept the file path option in there? There are a few reasons that come to mind, some from personal experience:

  • Unix utils tend to interop more easily with files; env vars are harder to access & modify by comparison.
  • Files are useful as intermediary artifacts, making it easy to manually inspect & modify them if needed in between various packaging steps
    • For example, if you have a set of different binaries in a single app, appending things to a file, deduplicating lines in a file, or many other operations are easier than doing so in an env var
  • It can be tough to specify complex dependency conditions while using the right quotation & special char escaping syntax to marshal them properly into env vars
    • To clarify, if you have a complex dependency that uses special characters like [, *, ~, !, etc, then it's easy to have a shell environment accidentally misinterpret those characters (especially for non-expert users), and especially when multiple different shell environments may be in use by various team members or build systems
      • Some examples of weird dependency strings are: openjdk-7-jre-headless (>= 7~u3-2.1.1), libc6 (>= 2.2.1), default-mta | mail-transport-agent,foo [linux-any], bar [any-i386], baz [!linux-any], libssl1.0.0_1.0.1t-1+deb8u6_amd64.deb etc. These are all quite a bit easier to express in a plaintext file.
  • Setting env vars differs across host OSes and can be (mildly) difficult to deal with in an abstract manner, and some runtime environments (chroots and the like) may clear or override env vars, which can be hard to debug.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for explanation, let's go with your suggestion using a file

}
impl Dependencies {
/// Returns the dependencies as a list of Strings.
pub fn to_list(&self) -> crate::Result<Vec<String>> {
match self {
Self::List(v) => Ok(v.clone()),
Self::Path(path) => {
let trimmed_lines = std::fs::read_to_string(path)?
.lines()
.filter_map(|line| {
let trimmed = line.trim();
if !trimmed.is_empty() {
Some(trimmed.to_owned())
} else {
None
}
})
.collect();
Ok(trimmed_lines)
}
}
}
}

/// The Linux AppImage configuration.
#[derive(Clone, Debug, Default, Deserialize, Serialize)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
Expand Down Expand Up @@ -397,8 +451,8 @@ pub struct PacmanConfig {
pub files: Option<HashMap<String, String>>,
/// List of softwares that must be installed for the app to build and run.
///
/// See : <https://wiki.archlinux.org/title/PKGBUILD#provides>
pub depends: Option<Vec<String>>,
/// See : <https://wiki.archlinux.org/title/PKGBUILD#depends>
pub depends: Option<Dependencies>,
/// Additional packages that are provided by this app.
///
/// See : <https://wiki.archlinux.org/title/PKGBUILD#provides>
Expand Down Expand Up @@ -439,16 +493,29 @@ impl PacmanConfig {
);
self
}
/// Set the list of pacman dependencies.

/// Set the list of pacman dependencies directly using an iterator of strings.
pub fn depends<I, S>(mut self, depends: I) -> Self
where
I: IntoIterator<Item = S>,
S: Into<String>,
{
self.depends
.replace(depends.into_iter().map(Into::into).collect());
.replace(Dependencies::List(depends.into_iter().map(Into::into).collect()));
self
}

/// Set the list of pacman dependencies indirectly via a path to a file,
/// which must contain one dependency (a package name) per line.
pub fn depends_path<P>(mut self, path: P) -> Self
where
P: Into<PathBuf>
{
self.depends
.replace(Dependencies::Path(path.into()));
self
}

/// Set the list of additional packages that are provided by this app.
pub fn provides<I, S>(mut self, provides: I) -> Self
where
Expand Down
15 changes: 8 additions & 7 deletions crates/packager/src/package/deb/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,15 @@ fn generate_control_file(
if let Some(homepage) = &config.homepage {
writeln!(file, "Homepage: {}", homepage)?;
}
let dependencies = config
if let Some(depends) = config
.deb()
.cloned()
.and_then(|d| d.depends)
.unwrap_or_default();
if !dependencies.is_empty() {
writeln!(file, "Depends: {}", dependencies.join(", "))?;
}
.and_then(|d| d.depends.as_ref())
{
let dependencies = depends.to_list()?;
if !dependencies.is_empty() {
writeln!(file, "Depends: {}", dependencies.join(", "))?;
}
}

writeln!(
file,
Expand Down
4 changes: 2 additions & 2 deletions crates/packager/src/package/pacman/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ fn generate_pkgbuild_file(

let dependencies = config
.pacman()
.and_then(|d| d.depends.clone())
.unwrap_or_default();
.and_then(|d| d.depends.as_ref())
.map_or_else(|| Ok(Vec::new()), |d| d.to_list())?;
writeln!(file, "depends=({})", dependencies.join(" \n"))?;

let provides = config
Expand Down
Loading