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

Try using isoinfo(1) to extract an ISO image identifier #14

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

iskunk
Copy link
Contributor

@iskunk iskunk commented Apr 7, 2023

The first of several PRs stemming from #13. This uses a utility from the genisoimage package, if available, to extract some kind of title string for each ISO image file found.

Copy link
Member

@mika mika left a comment

Choose a reason for hiding this comment

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

Not yet tested but only went briefly through the code, only some nitpicks from me side!
Looking forward to have that available, thanks! 👍

42_grml Outdated Show resolved Hide resolved
{
local iso=${1}
local id=
if isoinfo --version >/dev/null 2>&1 ; then
Copy link
Member

Choose a reason for hiding this comment

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

Is there any specific reason why we invoke isoinfo with --version and not check for its availability/executable?
(No big deal though and fine for me, just curious!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figure it's better than test -x (requires hard-coding the binary location) or trying to use which...

42_grml Outdated Show resolved Hide resolved
42_grml Outdated Show resolved Hide resolved
42_grml Outdated Show resolved Hide resolved
42_grml Outdated Show resolved Hide resolved
42_grml Outdated Show resolved Hide resolved
@iskunk
Copy link
Contributor Author

iskunk commented Apr 8, 2023

All right, I've updated the PR. The feedback and guidance are appreciated.

Please let me know if anything remains that's not up to snuff.

@mika mika merged commit 068c824 into grml:master Sep 8, 2023
@mika
Copy link
Member

mika commented Sep 8, 2023

Thanks also for this, just verified and LGTM, rebased against your PR 15 and just submitted. Thanks again! :)

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.

3 participants