-
Notifications
You must be signed in to change notification settings - Fork 3
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
Use Codecov Uploader instead of Bash uploader #19
Conversation
Great! It's about time to replace the bahs based uploader - can you make sure that the plugin still works as expected on a variety of hosts? The bash uploader had very few dependencies. I haven't looked at your PR in detail but I saw an APK call somewhere for example, which we can't easily make a prerequisite possibly. |
Added support for MacOS and other Linux flavors here: 97431ea |
Will test this and let you know. |
@joscha works for me on our buildkite pipeline. Tested it with one of our projects and the coverage files are uploaded correctly. Feel free to give it a spin if you have the possibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I haven't been able to test it as I don't currently have a codecov project available to me.
@@ -47,6 +176,9 @@ main() { | |||
local ci_env | |||
ci_env=$(bash <(curl -s -S --connect-timeout 10 --retry 3 --retry-delay 10 https://codecov.io/env)) | |||
|
|||
codecov_command=./codecov | |||
get_codecov_uploader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that on each run a new uploader is downloaded; always latest
currently. I wonder if we can allow a fixed version set by the plugin config and then cache away that particular version. Similar to what is done here: https://github.com/joscha/sauce-connect-buildkite-plugin/blob/master/hooks/pre-command#L30
That way the uploader stays stable longer and does not rely on the network fetch each time nor will cause trouble (read massive amounts of requests) when there are many many builds run from one subnet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 388c47d
I have added two new configs: uploader_version
and tmp_dir
with proper defaults.
The downloaded binary will be cached accordingly.
hooks/post-command
Outdated
@@ -25,6 +25,135 @@ plugin_read_list_into_result() { | |||
[[ ${#result[@]} -gt 0 ]] || return 1 | |||
} | |||
|
|||
|
|||
get_os() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the uploader is only available for plain Linux, alpine and Macos (and windows which we don't currently support) can we simplify this one? If it just returned the strings linux
, alpine
or macos
we can feed that directly into the download URL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 388c47d
hooks/post-command
Outdated
local os=$(get_os) | ||
local os_array=($os) | ||
if [ -n ${os_array[1]} ] && [ ${os_array[1]} = "Alpine" ]; then | ||
apk add gnupg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does an alpine allow this install on the fly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, in my test it seems to be working:
➜ codecov-buildkite-plugin git:(codecov_uploader) docker run -it alpine:3.14
/ # gpg
/bin/sh: gpg: not found
/ # apk add gnupg
(1/28) Installing libgpg-error (1.42-r0)
(2/28) Installing libassuan (2.5.5-r0)
(3/28) Installing libcap (2.50-r0)
(4/28) Installing libffi (3.3-r2)
(5/28) Installing libintl (0.21-r0)
(6/28) Installing libblkid (2.37.4-r0)
(7/28) Installing libmount (2.37.4-r0)
(8/28) Installing pcre (8.44-r0)
(9/28) Installing glib (2.68.3-r0)
(10/28) Installing ncurses-terminfo-base (6.2_p20210612-r0)
(11/28) Installing ncurses-libs (6.2_p20210612-r0)
(12/28) Installing libgcrypt (1.9.4-r0)
(13/28) Installing libsecret (0.20.4-r1)
(14/28) Installing pinentry (1.1.1-r0)
Executing pinentry-1.1.1-r0.post-install
(15/28) Installing libbz2 (1.0.8-r1)
(16/28) Installing gmp (6.2.1-r1)
(17/28) Installing nettle (3.7.3-r0)
(18/28) Installing p11-kit (0.23.22-r0)
(19/28) Installing libtasn1 (4.17.0-r0)
(20/28) Installing libunistring (0.9.10-r1)
(21/28) Installing gnutls (3.7.1-r0)
(22/28) Installing libksba (1.5.1-r0)
(23/28) Installing gdbm (1.19-r0)
(24/28) Installing libsasl (2.1.28-r0)
(25/28) Installing libldap (2.4.58-r0)
(26/28) Installing npth (1.6-r0)
(27/28) Installing sqlite-libs (3.35.5-r0)
(28/28) Installing gnupg (2.2.31-r0)
Executing busybox-1.33.1-r7.trigger
OK: 25 MiB in 42 packages
/ # gpg --version
gpg (GnuPG) 2.2.31
libgcrypt 1.9.4
Copyright (C) 2021 Free Software Foundation, Inc.
License GNU GPL-3.0-or-later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Home: /root/.gnupg
Supported algorithms:
Pubkey: RSA, ELG, DSA, ECDH, ECDSA, EDDSA
Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH,
CAMELLIA128, CAMELLIA192, CAMELLIA256
Hash: SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224
Compression: Uncompressed, ZIP, ZLIB, BZIP2
hooks/post-command
Outdated
curl -Os https://uploader.codecov.io/latest/linux/codecov | ||
curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM | ||
curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM.sig | ||
gpgv codecov.SHA256SUM.sig codecov.SHA256SUM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes gpg a prereq for the plugin. We should name it in the plugin.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 388c47d
hooks/post-command
Outdated
[[ ${#result[@]} -gt 0 ]] || return 1 | ||
} | ||
script_path="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )" | ||
source "${script_path}/../lib/utils.bash" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm, this fails on CI:
/var/lib/buildkite-agent/plugins/github-com-peakon-codecov-buildkite-plugin-uploader/hooks/post-command: line 5: /tmp/../lib/utils.bash: No such file or directory
@joscha any idea how to do this? I moved the common code to another file, because I needed to use some of the code in the tests...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I remember solving this before. I can't find a code reference right now, but I remember two ways:
- You can stub builtins via the method outlined in Stubbing out functions sstephenson/bats#38 (comment) (local definition +
export -f
) - Or what I remember to be the better way:
set a variable in your script where you want to include the file:
REPO_DIR="$(git rev-parse --show-toplevel)"
which will always give you the plugin root (as it is a valid checkout) and then include the file via:
source "${REPO_DIR}/lib/utils.bash"
in your bats test you can then just stub out the call to git
to return the right path:
REPO_DIR="${BATS_TEST_DIRNAME}/../.."
setup() {
stub git "rev-parse --show-toplevel : echo ${REPO_DIR}"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm, I did try the second suggestion, but git rev-parse --show-toplevel
gets the path to the root of our checked out project on the agent and tries to include lib/utils.bash
which obviously doesn't exist there and it fails. I think I'll go with the first solution: bringing back the extracted code to post-command
and stubbing the required code in the tests.
@joscha would you mind doing another review? I have tested on CI and it works fine. Also tried with
|
Hi @paymand, thanks for all the updates. Looking pretty good now! If I pushed a few changes to the bash code, would you be able to run your test on the combined code by any chance? |
@joscha yes, I can. Please push your changes and I'll test it. |
hooks/post-command
Outdated
do | ||
local local_path="${TMP_DIR}/${file}" | ||
if [[ ! -e "${local_path}" ]]; then | ||
>&2 echo "Local path will be: ${local_path}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>&2 echo "Local path will be: ${local_path}" | |
debug "Local path will be: ${local_path}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 827e857.
hooks/post-command
Outdated
if [[ ! -e "${local_path}" ]]; then | ||
>&2 echo "Local path will be: ${local_path}" | ||
local remote_source="https://uploader.codecov.io/${CODECOV_VERSION}/${OS}/${file}" | ||
>&2 echo "Source is: ${remote_source}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>&2 echo "Source is: ${remote_source}" | |
debug "Source is: ${remote_source}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 827e857.
@joscha Got it tested and it works fine:
|
Nice work! |
Also tested without
Good to merge IMHO 👍 |
Support for the Bash Uploader, as you can see on the page, is deprecated on February 1st, 2022. Even though it still seems to work fine, I think it will eventually be sunset at some point.
The idea is to use Codecov Uploader.
Also see the blog post below for more info:
https://about.codecov.io/blog/introducing-codecovs-new-uploader/
I have updated the tests and they pass:
I haven't actually tested the upload, but I will try to test it on a branch.