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.

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

chore: initial jammy packaging directory #1086

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

zarkdav
Copy link
Collaborator

@zarkdav zarkdav commented Apr 3, 2024

User description

This is my proposal to facilitate the creation of .deb and, most importantly, source Debian packages that can be uploaded to one's PPA.


Type

enhancement, documentation


Description

  • Adds comprehensive Debian packaging support for Trippy, targeting Ubuntu Jammy 22.04 LTS.
  • Includes detailed instructions for packaging, vendoring dependencies, and managing a PPA.
  • Configures cargo for using vendored sources, ensuring builds without network access.
  • Specifies package metadata, build dependencies, and custom build rules tailored for Rust projects.
  • Provides copyright, licensing information, and documentation for proper Debian packaging.

Changes walkthrough

Relevant files
Documentation
4 files
README.debian
Comprehensive Packaging Instructions for Debian and Ubuntu

debian/README.debian

  • Provides detailed instructions for packaging Trippy for Debian and
    Ubuntu.
  • Explains how to vendor dependencies and build the package without
    network access.
  • Offers guidance on creating and managing a Personal Package Archive
    (PPA).
  • Lists commands for targeting different Ubuntu distributions.
  • +65/-0   
    changelog
    Initial Changelog Entry for Ubuntu Jammy                                 

    debian/changelog

  • Adds initial changelog entry for the package version targeting Ubuntu
    Jammy.
  • +5/-0     
    copyright
    Copyright and Licensing Information                                           

    debian/copyright

  • Provides copyright and licensing information for the project and
    Debian packaging.
  • +32/-0   
    trippy.docs
    Documentation Inclusion for Debian Package                             

    debian/trippy.docs

  • Specifies the inclusion of the README.md file in the package
    documentation.
  • +1/-0     
    Configuration changes
    5 files
    cargo.config
    Cargo Configuration for Vendored Dependencies                       

    debian/cargo.config

    • Configures cargo to use vendored sources for dependencies.
    +5/-0     
    control
    Package Metadata and Build Dependencies Definition             

    debian/control

    • Defines package metadata, dependencies, and build requirements.
    +21/-0   
    format
    Debian Source Package Format Specification                             

    debian/source/format

    • Specifies the Debian source package format.
    +1/-0     
    include-binaries
    Inclusion of Vendored Dependencies in Source Package         

    debian/source/include-binaries

    • Includes the vendor.tar.xz file as a binary in the source package.
    +1/-0     
    trippy.install
    Installation Path Definition for Trippy Binary                     

    debian/trippy.install

    • Defines the installation path for the Trippy binary.
    +1/-0     
    Enhancement
    1 files
    rules
    Custom Build Rules for Debian Packaging                                   

    debian/rules

  • Implements custom build rules for vendoring dependencies and building
    the package.
  • Overrides default build, clean, and strip behaviors to accommodate
    Rust project structure.
  • +24/-0   

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @zarkdav zarkdav linked an issue Apr 3, 2024 that may be closed by this pull request
    Copy link

    github-actions bot commented Apr 3, 2024

    PR Description updated to latest commit (4d3fffa)

    Copy link

    github-actions bot commented Apr 3, 2024

    PR Review

    (Review updated until commit 4d3fffa)

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Issue: The vendor rule in debian/rules deletes the vendor directory without checking if it's currently in use or contains unsaved changes. This could lead to data loss if the directory is being used elsewhere.

    Possible Improvement: The packaging instructions do not mention how to handle updates to the Rust toolchain itself. Given that the build process relies on the ~/.cargo/bin directory, changes in the toolchain could affect reproducibility and compatibility.

    🔒 Security concerns

    No

    Code feedback:
    relevant filedebian/rules
    suggestion      

    Consider checking if the vendor directory is in use or contains unsaved changes before deleting it. This can prevent potential data loss. You might use a temporary directory for vendoring or prompt the user before deletion. [important]

    relevant line-[ -d vendor ] && rm -rf vendor

    relevant filedebian/README.debian
    suggestion      

    Add a section on handling updates to the Rust toolchain, including how to ensure compatibility with new toolchain versions and the impact on the build process. This will help maintainers keep the package up-to-date and ensure it builds correctly. [medium]

    relevant lineTL;DR: to generate your own debian package with your own Rust toolchain,

    relevant filedebian/control
    suggestion      

    Consider specifying minimum versions for cargo:native and rustc:native in the Build-Depends field to ensure compatibility and reproducibility of builds across different environments. [medium]

    relevant linecargo:native,

    relevant filedebian/rules
    suggestion      

    For better modularity and readability, consider moving the logic for creating the .cargo/config file and extracting the vendor.tar.xz into separate rules or a script. This can make the build process clearer and easier to maintain. [medium]

    relevant linecp debian/cargo.config .cargo/config


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    github-actions bot commented Apr 3, 2024

    PR Description updated to latest commit (4d3fffa)

    Copy link

    github-actions bot commented Apr 3, 2024

    Persistent review updated to latest commit 4d3fffa

    Copy link

    github-actions bot commented Apr 3, 2024

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Best practice
    Ensure the cargo vendor command succeeds before creating the vendor tarball.

    Consider checking the success of the cargo vendor command before proceeding to create the
    vendor.tar.xz tarball. This ensures that the build process does not continue with an
    incomplete or missing vendor directory, which could lead to build failures or incomplete
    builds.

    debian/rules [10-11]

    -cargo vendor
    -tar cJf debian/vendor.tar.xz vendor
    +cargo vendor && tar cJf debian/vendor.tar.xz vendor
     
    Use dh_auto_clean for a more idiomatic cleanup in Debian packaging.

    Use dh_auto_clean instead of manually specifying cargo clean and removing directories.
    This leverages debhelper's clean mechanism, which is more idiomatic and ensures that all
    necessary cleanup operations are performed according to Debian packaging standards.

    debian/rules [20-21]

    -cargo clean
    -rm -rf .cargo vendor
    +dh_auto_clean
     
    Use override_dh_auto_configure for setting up Cargo configuration.

    Instead of manually creating and removing the .cargo directory, consider using the
    override_dh_auto_configure target to set up the Cargo configuration. This approach is
    cleaner and separates the configuration phase from the build phase, adhering to the Debian
    packaging best practices.

    debian/rules [14-15]

    -mkdir .cargo
    -cp debian/cargo.config .cargo/config
    +override_dh_auto_configure:
    +	mkdir -p .cargo
    +	cp debian/cargo.config .cargo/config
     
    Enhancement
    Specify minimum Rust and Cargo versions in Build-Depends.

    Specify the minimum version of Rust and Cargo required to build the package in the
    Build-Depends field. This ensures that the package is built with a compatible toolchain,
    preventing potential build failures due to incompatibilities.

    debian/control [6-9]

     Build-Depends: debhelper-compat (= 13),
    - cargo:native,
    - rustc:native,
    + cargo:native (>= 1.40),
    + rustc:native (>= 1.40),
      libstd-rust-dev
     
    Maintainability
    Improve command readability in the README by using code blocks.

    It's recommended to wrap the command examples with code blocks or indent them to improve
    readability in markdown viewers. This makes it easier for users to identify and copy
    commands without accidentally missing parts of the command.

    debian/README.debian [6-8]

    +```
     ./debian/rules vendor
     then you can simply run:
         debuild --prepend-path ~/.cargo/bin -sa
    +```
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    Copy link

    github-actions bot commented Apr 3, 2024

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Ensure cargo vendor succeeds before creating the tarball.

    Consider checking the success of the cargo vendor command before proceeding to create the
    vendor.tar.xz tarball. This ensures that the build process does not continue if there are
    issues with vendoring the dependencies.

    debian/rules [10-11]

    -cargo vendor
    -tar cJf debian/vendor.tar.xz vendor
    +cargo vendor && tar cJf debian/vendor.tar.xz vendor
     
    Improve clarity of instructions with steps or bullet points.

    To improve clarity and readability, consider breaking down complex instructions into
    numbered steps or bullet points, especially for the build and packaging instructions.

    debian/README.debian [4-8]

    -TL;DR: to generate your own debian package with your own Rust toolchain,
    -the vendored dependencies need to be generated first with:
    -    ./debian/rules vendor
    -then you can simply run:
    -    debuild --prepend-path ~/.cargo/bin -sa
    +TL;DR:
    +1. Generate the vendored dependencies first with:
    +   ./debian/rules vendor
    +2. Then, you can generate your own Debian package by running:
    +   debuild --prepend-path ~/.cargo/bin -sa
     
    Maintainability
    Use variables for repeated paths or filenames.

    To improve maintainability, consider using variables for repeated paths or filenames, such
    as debian/vendor.tar.xz, to ensure consistency and ease future updates.

    debian/rules [11-16]

    -tar cJf debian/vendor.tar.xz vendor
    -tar xJf debian/vendor.tar.xz
    +VENDOR_TAR=debian/vendor.tar.xz
    +tar cJf $(VENDOR_TAR) vendor
    +tar xJf $(VENDOR_TAR)
     
    Best practice
    Clean up the .cargo directory in the clean target.

    It's a good practice to clean up the .cargo directory in the override_dh_auto_clean target
    to ensure a clean state for future builds.

    debian/rules [20-21]

     cargo clean
    -rm -rf .cargo vendor
    +rm -rf .cargo
    +rm -rf vendor
     
    Specify minimum Rust and Cargo versions in Build-Depends.

    Consider specifying the minimum version of Rust and Cargo required to build the project in
    the Build-Depends field. This ensures that the package is built with a compatible
    toolchain.

    debian/control [6-8]

     Build-Depends: debhelper-compat (= 13),
    - cargo:native,
    - rustc:native,
    + cargo:native (>= 1.40),
    + rustc:native (>= 1.40),
      libstd-rust-dev
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @fujiapple852 fujiapple852 force-pushed the 859-please-provide-debianubuntu-ppas branch from 4d3fffa to b11aa53 Compare April 4, 2024 12:25
    @fujiapple852
    Copy link
    Owner

    LGTM

    Note that I replaced your name with mine as the maintainer since we agreed to publish to https://launchpad.net/~fujiapple/+archive/ubuntu/trippy/+packages

    Thanks!

    @fujiapple852 fujiapple852 merged commit 8ef54fd into master Apr 4, 2024
    18 checks passed
    @fujiapple852 fujiapple852 deleted the 859-please-provide-debianubuntu-ppas branch April 4, 2024 12:29
    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.

    Please provide debian/ubuntu PPAs
    2 participants