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

Working deploy with travis-ci.com for the RaspberryPi3 (arm64) support #35

Merged
merged 20 commits into from
May 7, 2020
Merged

Working deploy with travis-ci.com for the RaspberryPi3 (arm64) support #35

merged 20 commits into from
May 7, 2020

Conversation

brainstorm
Copy link
Contributor

@brainstorm brainstorm commented Apr 30, 2020

This is a followup on the failed deploy for PR #32.

The underlying issue has been resolved by travisci support folks, as explained/tracked on twitter:

https://twitter.com/braincode/status/1253981272885477376

This pullrequest should now deploy, as demonstrated on this fork:

https://github.com/brainstorm/openpnp-capture/releases
and
https://travis-ci.com/github/brainstorm/openpnp-capture/builds/162954136

As long as this particular repository is migrated to travis-ci.com instead of travis-ci.org (change to be operated by @vonnieda):

https://docs.travis-ci.com/user/migrate/open-source-repository-migration

@brainstorm brainstorm changed the title Working deploy with travis-ci.com Working deploy with travis-ci.com for the RaspberryPi3 (arm64) support Apr 30, 2020
@brainstorm
Copy link
Contributor Author

Works on XCode 11.4.1 locally, although might need some tweaks on platformcontext.mm to avoid future deprecations:

$ make -j4
[ 36%] Building CXX object CMakeFiles/openpnp-capture.dir/common/libmain.cpp.o
[ 36%] Building CXX object CMakeFiles/openpnp-capture.dir/common/logging.cpp.o
[ 36%] Building CXX object CMakeFiles/openpnp-capture.dir/common/context.cpp.o
[ 36%] Building CXX object CMakeFiles/openpnp-capture.dir/common/stream.cpp.o
[ 45%] Building CXX object CMakeFiles/openpnp-capture.dir/mac/platformcontext.mm.o
[ 54%] Building CXX object CMakeFiles/openpnp-capture.dir/mac/platformstream.mm.o
[ 63%] Building CXX object CMakeFiles/openpnp-capture.dir/mac/uvcctrl.mm.o
/Users/romanvg/dev/cchs/openpnp-capture/mac/platformcontext.mm:74:54: warning: 'devicesWithMediaType:' is deprecated: first deprecated in macOS 10.15 - Use AVCaptureDeviceDiscoverySession instead. [-Wdeprecated-declarations]
    for (AVCaptureDevice* device in [AVCaptureDevice devicesWithMediaType:AVMediaTypeVideo])
                                                     ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/AVFoundation.framework/Headers/AVCaptureDevice.h:102:1: note: 'devicesWithMediaType:' has been explicitly marked deprecated here
+ (NSArray<AVCaptureDevice *> *)devicesWithMediaType:(AVMediaType)mediaType API_DEPRECATED("Use AVCaptureDeviceDiscoverySession instead.", ios(4.0, 10.0), macos(10.7, 10.15));
^
1 warning generated.
[ 72%] Linking CXX shared library libopenpnp-capture.dylib
[ 72%] Built target openpnp-capture
[ 81%] Building CXX object mac/tests/CMakeFiles/openpnp-capture-test.dir/__/__/common/logging.cpp.o
[ 90%] Building CXX object mac/tests/CMakeFiles/openpnp-capture-test.dir/main.cpp.o
[100%] Linking CXX executable openpnp-capture-test
[100%] Built target openpnp-capture-test

@brainstorm
Copy link
Contributor Author

brainstorm commented Apr 30, 2020

Oh dear, it turns out XCode 11.4 is fine, but TravisCI still does not support Catalina?:

https://travis-ci.community/t/macos-catalina-build-environment/5608/16

https://docs.travis-ci.com/user/reference/osx/#macos-version

... sigh

Possible OS-version-dependent fix spotted here: https://github.com/opencv/opencv/pull/15517/files

Copy link
Member

@vonnieda vonnieda left a comment

Choose a reason for hiding this comment

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

Hi @brainstorm - overall I'm happy to merge this, but there are a number of changes to the .travis.yml that don't seem to be needed. I'm curious if these are these made on purpose, or if a tool made the changes. If the changes aren't required for your deployment I'd prefer if they are reverted.

deploy:
provider: releases
api_key:
token:
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this change? I don't see "token" documented on https://docs.travis-ci.com/user/deployment/releases/

file: deploy/*
skip_cleanup: true
file: deploy/*
cleanup: false
Copy link
Member

Choose a reason for hiding this comment

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

Same as above: Why is this changed? "cleanup" does not seem to be documented.

skip_cleanup: true
file: deploy/*
cleanup: false
edge: true
Copy link
Member

Choose a reason for hiding this comment

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

Okay, it looks like this selects the beta version of the provider, which explains the above questions. Is this required for your deployment to work? If it's not, I don't see a good reason to switch to a beta provider at this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, all the changes above are informed by TravisCI support, travis lint's CLI and web linter too:

https://config.travis-ci.com/explore

See how your current .travis.yml on your master looks like against the official linter:

Skärmavbild_2020-05-07_kl__09_17_17

I really just wanted to future-proof this a little bit, but happy to revert back to the old syntax.

on:
all_branches: true
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to create a new release for every branch. Is there some reason this is added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this was added since I was getting "Not deploying because rpi3_arm64 is not an authorized branch". In my experience, I've found that when combined with tags: true, it conserves the original behaviour (only release on tags) and at the same time, it allows for easier deployment testing on forks (my case), since I didn't want to pollute my forked master to test whether it deployed well or not. Happy to remove it anyway now that the work is done.

@vonnieda
Copy link
Member

vonnieda commented May 7, 2020

Thanks @brainstorm - all sounds good. I appreciate the detailed response. Let's give it a try!

@vonnieda vonnieda merged commit 4a9bd22 into openpnp:master May 7, 2020
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.

2 participants