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

Install dir #1959

Merged
merged 3 commits into from
Jan 29, 2025
Merged

Install dir #1959

merged 3 commits into from
Jan 29, 2025

Conversation

jreidinger
Copy link
Contributor

Problem

For full offline medium support we need to check also if there is repositories in /install on medium.

Solution

Add support to detect it and use it if found.
Note: live / is not the one from iso, so it needs to check mounted initramfs

Testing

  • Tested manually

Copy link
Contributor

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

LGTM

# path to cdrom which can contain installation repositories
dir_path = "/run/initramfs/live/install"

if File.exist?(dir_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just nit-picking, I'd reverse the logic:

return false unless File.exist?(dir_path)

logger.info "/install found on installation medium"
repositories.add("dir://" + dir_path)
true

This avoids using an indented block.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, searching for a dir should be faster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kobliha I think you mean change of order of checking for dir versus label?

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 do not think it will make difference and lets merge this one for now to have something working

@jreidinger jreidinger merged commit 5489764 into master Jan 29, 2025
10 checks passed
@jreidinger jreidinger deleted the install_dir branch January 29, 2025 16:45
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