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

Support HCK installer as ISO #325

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Conversation

Jedoku
Copy link
Contributor

@Jedoku Jedoku commented Mar 18, 2024

@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 1629181 to 16156f4 Compare March 18, 2024 15:33
Copy link
Contributor

@akihikodaki akihikodaki left a comment

Choose a reason for hiding this comment

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

Instead of requiring to specify iso flag, why not automatically detect it?
If File.read(path, 2) == 'MZ', it is a Windows executable. You can assume a disk image otherwise.

lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
lib/engines/hckinstall/setup_scripts_helper.rb Outdated Show resolved Hide resolved
lib/engines/hckinstall/hckinstall.rb Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 16156f4 to 613cfa3 Compare March 18, 2024 17:22
@Jedoku
Copy link
Contributor Author

Jedoku commented Mar 19, 2024

Instead of requiring to specify iso flag, why not automatically detect it? If File.read(path, 2) == 'MZ', it is a Windows executable. You can assume a disk image otherwise.

Yeah, we think about it, but for get extention you need to download file from link and check it then somehow save it to another folder it was easiest way to done this task. We use curb gem for done this and with provided links it`s impossible to get filename before downloading and you also should save it with filename.

@akihikodaki
Copy link
Contributor

Instead of requiring to specify iso flag, why not automatically detect it? If File.read(path, 2) == 'MZ', it is a Windows executable. You can assume a disk image otherwise.

Yeah, we think about it, but for get extention you need to download file from link and check it then somehow save it to another folder it was easiest way to done this task. We use curb gem for done this and with provided links it`s impossible to get filename before downloading and you also should save it with filename.

Just download it without an extension and rename it.

@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 613cfa3 to 52a7ccf Compare March 21, 2024 08:47
@Jedoku Jedoku requested a review from akihikodaki March 21, 2024 11:18
Copy link
Contributor

@akihikodaki akihikodaki left a comment

Choose a reason for hiding this comment

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

  • It's better to use httpclient or faraday gem if you are going to read HTTP headers. Note that these gems are already used by other dependencies, so requiring them does not add more dependencies.
  • Don't perform HTTP requests twice. I bet it doesn't need much code to save one HTTP request.

@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 52a7ccf to 33d6cca Compare March 25, 2024 20:15
@kostyanf14 kostyanf14 requested a review from akihikodaki March 26, 2024 08:32
lib/engines/hckinstall/kit.json Outdated Show resolved Hide resolved
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
lib/engines/hckinstall/hckinstall.rb Show resolved Hide resolved
lib/engines/hckinstall/setup_scripts_helper.rb Outdated Show resolved Hide resolved
@akihikodaki
Copy link
Contributor

Also see: #325 (comment)

@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 33d6cca to 0936be1 Compare March 26, 2024 09:46
@kostyanf14 kostyanf14 requested a review from akihikodaki March 26, 2024 10:27
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch 2 times, most recently from 77b0309 to 3cc0151 Compare March 26, 2024 13:32
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 3cc0151 to 1855ba5 Compare March 27, 2024 12:29
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch 2 times, most recently from e54b0ca to 72a1526 Compare March 28, 2024 10:12
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 72a1526 to 164f284 Compare March 28, 2024 10:34
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 164f284 to 815db85 Compare March 28, 2024 11:19
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 815db85 to 2eab4c9 Compare March 28, 2024 11:26
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch 2 times, most recently from 75df785 to cd4fd5c Compare March 28, 2024 11:37
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch 3 times, most recently from 8af1010 to ba21866 Compare March 28, 2024 11:56
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from ba21866 to e342136 Compare March 28, 2024 11:59
lib/engines/hckinstall/hckinstall.rb Outdated Show resolved Hide resolved
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from e342136 to 7366958 Compare March 28, 2024 12:11

if @kit_path.nil?
if @kit_info['download_url'].nil?
raise(AutoHCKError, 'HLK installer download URL is not provided and installer is not found')
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use move-specific EngineError

Signed-off-by: Vitalii Chulak <[email protected]>
@Jedoku Jedoku force-pushed the Support-HCK-installer-as-ISO branch from 7366958 to ac4844d Compare March 29, 2024 11:30
@kostyanf14 kostyanf14 merged commit 338d98a into master Apr 3, 2024
6 checks passed
@kostyanf14 kostyanf14 deleted the Support-HCK-installer-as-ISO branch April 3, 2024 13:37
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