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

Pack client deb hwlib hwctl #253

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

nadzyah
Copy link
Collaborator

@nadzyah nadzyah commented Dec 3, 2024

This PR closes C3-913.
With these changes, the source client package now provides two binaries: librust-hwlib-dev and hwctl. To make such packaging possible, the' Debian/folder and workspaceCargo.*files were moved to theclient/` folder.

Key changes:

  • The debian/rules file was updated with an overridden dh_auto_install section. It was done dh-cargo treats the whole workspace as a single package, either library or binary package. For more information, check https://salsa.debian.org/rust-team/dh-cargo/-/blob/5cc7f7b8/cargo.pm#L168-195
  • The debian/generate-checksum.py script was updated to generate the cargo-checksum.json file only for the files in the hwlib dir, since it's needed only for the library packages
  • hwctl.1 man page was added since it's required for CLI tools

* Creates one source package that produces two binaries
  (librust-hwlib-dev and hwctl)

* Moves debian/ dir under the client/ folder

* Updates the debian/generate-checksum.py script to generate the
  cargo-checksum.json file only for the hwlib as required

* Rewrite the dh_auto_install section in the debian/rules file since
  dh-cargo tries to install the whole workspace as a package. The
  changes install the hwlib as a library and hwctl as a binary package,
  repeating the logic implemented in dh-cargo source code.
Since now we pack both hwlib and hwctl, this information in located in
the workspace Cargo.toml file
@nadzyah nadzyah requested a review from zyga December 4, 2024 14:23
@nadzyah nadzyah marked this pull request as ready for review December 4, 2024 14:23
Copy link
Collaborator

@zyga zyga left a comment

Choose a reason for hiding this comment

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

Some feedback on the man page.

Super nice for you to write it manually. I really applaud that!

does not accept any command-line options.
.SH ENVIRONMENT
.TP
.B HW_API_URL
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be .Ev

Ubuntu Hardware Certification Program, visit
https://ubuntu.com/certified.
.SH OPTIONS
.B hwctl
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be .Nm

.SH NAME
hwctl \- CLI tool to check hardware certification status
.SH SYNOPSIS
.B hwctl
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be .Nm

.B hwctl
.SH DESCRIPTION
.B hwctl
is a command-line tool that checks the Ubuntu Hardware Certification
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would put it like that:

Suggested change
is a command-line tool that checks the Ubuntu Hardware Certification
is a command-line tool that checks the
.Em Ubuntu Hardware Certification

@@ -0,0 +1,28 @@
.TH HWCTL 1 "03 Dec 2024" "Canonical Ltd."
.SH NAME
hwctl \- CLI tool to check hardware certification status
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be

Suggested change
hwctl \- CLI tool to check hardware certification status
.Nm hwctl
.Nd check status of Ubuntu Hardware Certification

.SH SYNOPSIS
.B hwctl
.SH DESCRIPTION
.B hwctl
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be .Nm

Suggested change
.B hwctl
.Nm hwctl

the certification covers for it. Also, note that root privileges are
typically required to run this command. For more information about the
Ubuntu Hardware Certification Program, visit
https://ubuntu.com/certified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
https://ubuntu.com/certified.
.Lk https://ubuntu.com/certified "The cerfification website" .

Specifies the hardware-api server URL that
.B hwctl
uses for certification queries. The default value is
https://hw.ubuntu.com
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use .Lk like above.

uses for certification queries. The default value is
https://hw.ubuntu.com
.SH BUGS
https://github.com/canonical/hardware-api/issues/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a paragraph, not just the URL and use .Lk

.SH BUGS
https://github.com/canonical/hardware-api/issues/
.SH COPYRIGHT
Copyright (C) 2024 Canonical Ltd.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can include this in a SPDX expression but the man page should not mention it. Put up

.Sh AUTHORS

Instead please.

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