Skip to content

feat(ci): armv7 cross builds #3892

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 9, 2021
Merged

feat(ci): armv7 cross builds #3892

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
820ff03
feat(ci): armv7 cross builds
Aug 3, 2021
0788e94
fix(ci): adjust armv7 architectures
Aug 7, 2021
1da2558
chore(ci): explain get_nfpm_arch in a comment
Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(ci): adjust armv7 architectures
  • Loading branch information
Akash Satheesan committed Aug 7, 2021
commit 0788e947229dd22e62f5825266891cf17bc93f5e
8 changes: 8 additions & 0 deletions ci/build/arch-override.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"rpm": {
"armv7l": "armhfp"
},
"deb": {
"armv7l": "armhf"
Comment on lines +2 to +6
Copy link
Contributor

@jsjoeio jsjoeio Aug 9, 2021

Choose a reason for hiding this comment

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

I worry about adding an extra file just because it seems like more to maintain. Also the json means we can't really put comments to explain why we have this. Any thoughts on writing a function to pass in rpm or deb and then returning values instead?

Copy link
Author

@oxy oxy Aug 9, 2021

Choose a reason for hiding this comment

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

My reasoning to put this in a separate file is that its easier to add overrides for other architectures should we need it in the future (say powerpc/s390x) and that it also separates code (building packages) from data (what overrides to apply when)

Copy link
Contributor

@jsjoeio jsjoeio Aug 9, 2021

Choose a reason for hiding this comment

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

Ah, okay that makes sense then :) Let's leave as is!

}
}
19 changes: 16 additions & 3 deletions ci/build/build-packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,27 @@ release_gcp() {
cp "./release-packages/$release_name.tar.gz" "./release-gcp/latest/$OS-$ARCH.tar.gz"
}

get_nfpm_arch() {
if jq -re ".${PKG_FORMAT}.${ARCH}" ./ci/build/arch-override.json > /dev/null; then
jq -re ".${PKG_FORMAT}.${ARCH}" ./ci/build/arch-override.json
else
echo "$ARCH"
fi
}

Comment on lines +50 to +57
Copy link
Contributor

@jsjoeio jsjoeio Aug 9, 2021

Choose a reason for hiding this comment

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

Maybe we could add a comment above explaining why we have to override the arch? Or why we have this function?

Copy link
Author

@oxy oxy Aug 9, 2021

Choose a reason for hiding this comment

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

Sure! I can do that - just give me a minute.

Copy link
Contributor

@jsjoeio jsjoeio Aug 9, 2021

Choose a reason for hiding this comment

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

Only request then is to add a comment or two explaining why we have to override the arch and then we should approve and merge!

Copy link
Author

@oxy oxy Aug 9, 2021

Choose a reason for hiding this comment

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

Done!

# Generates deb and rpm packages.
release_nfpm() {
local nfpm_config

PKG_FORMAT="deb"
NFPM_ARCH="$(get_nfpm_arch)"
nfpm_config="$(envsubst < ./ci/build/nfpm.yaml)"
nfpm pkg -f <(echo "$nfpm_config") --target "release-packages/code-server_${VERSION}_${NFPM_ARCH}.deb"

# The underscores are convention for .deb.
nfpm pkg -f <(echo "$nfpm_config") --target "release-packages/code-server_${VERSION}_$ARCH.deb"
nfpm pkg -f <(echo "$nfpm_config") --target "release-packages/code-server-$VERSION-$ARCH.rpm"
PKG_FORMAT="rpm"
NFPM_ARCH="$(get_nfpm_arch)"
nfpm_config="$(envsubst < ./ci/build/nfpm.yaml)"
nfpm pkg -f <(echo "$nfpm_config") --target "release-packages/code-server-$VERSION-$NFPM_ARCH.rpm"
}

main "$@"
2 changes: 1 addition & 1 deletion ci/build/nfpm.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: "code-server"
arch: "${ARCH}"
arch: "${NFPM_ARCH}"
platform: "linux"
version: "v${VERSION}"
section: "devel"
Expand Down