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 automake for macos GHA. #1096

Merged
merged 2 commits into from
Apr 18, 2020
Merged

Install automake for macos GHA. #1096

merged 2 commits into from
Apr 18, 2020

Conversation

ni4
Copy link
Contributor

@ni4 ni4 commented Apr 11, 2020

Looks like something changed in GHA macos runner, preventing correct installation of json-c. Attempting to fix it.

@ni4 ni4 marked this pull request as ready for review April 11, 2020 10:29
@ni4 ni4 requested a review from dewyatt April 11, 2020 10:29
@ni4
Copy link
Contributor Author

ni4 commented Apr 11, 2020

@dewyatt Is this a correct way and place in script to install automake? And, still have issues with ruby-rnp.

@ronaldtse
Copy link
Contributor

@ni4 it seems that the automake part worked, but these two errors remain:

	 55 - rnp_tests.test_ffi_signatures_dump (Failed)
2554
	167 - ruby-rnp (Failed)
2555

I assume the latter is to be fixed by @dewyatt but any idea where the first comes from?

@ni4
Copy link
Contributor Author

ni4 commented Apr 12, 2020

@ronaldtse First one is because of test key expiration, I will make a separate PR fixing this. The second one is something on the ruby side, I guess. Did a PR in attempt to fix that here: rnpgp/ruby-rnp#67

@codecov
Copy link

codecov bot commented Apr 12, 2020

Codecov Report

Merging #1096 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1096      +/-   ##
==========================================
- Coverage   83.20%   83.17%   -0.03%     
==========================================
  Files          76       76              
  Lines       29070    29070              
==========================================
- Hits        24187    24180       -7     
- Misses       4883     4890       +7     
Impacted Files Coverage Δ
src/lib/utils.h 47.36% <0.00%> (-26.32%) ⬇️
src/lib/crypto/elgamal.cpp 78.35% <0.00%> (-2.24%) ⬇️
src/librepgp/stream-write.cpp 79.03% <0.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16ecb28...a1aba8d. Read the comment docs.

@@ -5,6 +5,7 @@ set -ex

macos_install() {
[ "${CI-}" = true ] || brew bundle
brew install automake
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of this just add it to the top-level Brewfile

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait I just realized it's already in there -_-. Must be something else going on here...

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been working on getting CI stable again with regards to the ruby-rnp failures, so I'll add this to the list. Hopefully I can get it finished today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @dewyatt !

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay I see what's up here: https://github.blog/changelog/2020-04-15-github-actions-sets-the-ci-environment-variable-to-true/

Anyways, I'll get this stuff all fixed up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really sure why I wrote it that way originally anyways, it would only install the bundle if CI was not true (which worked because it used to be blank until yesterday). That'll teach me to comment my code I guess. I'll probably just replace that with brew bundle. Hmm.

@dewyatt dewyatt force-pushed the ni4-fix-macos-aclocal branch 2 times, most recently from 7d293aa to 05477b7 Compare April 18, 2020 18:00
@dewyatt
Copy link
Contributor

dewyatt commented Apr 18, 2020

I modified this to just a simple brew bundle and I'm going to go ahead and merge. I also added a workaround for an issue in Cirrus CI. Windows appears to be timing out but it shouldn't be related.

@dewyatt dewyatt merged commit 0238d53 into master Apr 18, 2020
@dewyatt dewyatt deleted the ni4-fix-macos-aclocal branch April 18, 2020 18:57
@ni4
Copy link
Contributor Author

ni4 commented Apr 19, 2020

Thanks, @dewyatt!

@ni4 ni4 added this to the v0.14.0 milestone Jan 4, 2021
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