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

iPXE build scripts update #89

Merged
merged 5 commits into from
Oct 3, 2023

Conversation

jacobweinstock
Copy link
Member

@jacobweinstock jacobweinstock commented Sep 13, 2023

Description

Update iPXE build scripts. Instead of having the build_ipxe.sh handle running in docker or locally with nix, we remove all that out so that it is up to the user of build_ipxe.sh to get into a nix-shell using binary/script/shell.nix. The script no longer calls docker or nix to run the iPXE build. The purpose of this is to simplify the build process, adding features, maintainability, etc. This will also fix the build issue on main.

Also updated the golangci-lint to the latest version and resolve lint issues.

Why is this needed

Fixes: #

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

Instead of having the build_ipxe.sh handle
running in docker or nix, we brake that out
so that it is up to the user of build_ipxe.sh
to get into a nix-shell. The script no longer
calls docker or nix in any way. The purpose of
this is to simplify the build process, adding
features, maintainability, etc.

Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
This will work regardless of the location you
runt the script from.

Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
@jacobweinstock jacobweinstock changed the title Ipxe build update iPXE build update Sep 13, 2023
@jacobweinstock jacobweinstock changed the title iPXE build update iPXE build scripts update Sep 13, 2023
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #89 (78ab9e0) into main (56d9647) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #89   +/-   ##
=======================================
  Coverage   97.25%   97.25%           
=======================================
  Files           5        5           
  Lines         437      437           
=======================================
  Hits          425      425           
  Misses          8        8           
  Partials        4        4           
Files Changed Coverage Δ
binary/binary.go 100.00% <ø> (ø)
itftp/itftp.go 97.08% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nshalman
Copy link
Member

nshalman commented Oct 3, 2023

... I should probably review this, huh?

It all looks good. Does it work on your machine? I get permission errors on mine.

@nshalman nshalman mentioned this pull request Oct 3, 2023
@chrisdoherty4
Copy link
Member

Lets get this in to unblock #90. If there are more issues we can rectify post-merge.

@chrisdoherty4 chrisdoherty4 merged commit de82e07 into tinkerbell:main Oct 3, 2023
5 checks passed
mergify bot added a commit that referenced this pull request Oct 4, 2023
## Description

Switch the boot floppy to use GPT partitioning

## Why is this needed

The GPT partition table allows us to label the partition more clearly
which makes it easier for other tooling to detect / ignore it.

## How Has This Been Tested?

Mostly just on my NixOS machine, though also under docker in #90 which is no longer needed thanks to #89

## How are existing users impacted? What migration steps/scripts do we need?

N/A
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