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

Add support for public repos on travis-ci.com #310

Merged
merged 33 commits into from
Nov 30, 2018
Merged

Conversation

asmeurer
Copy link
Member

@asmeurer asmeurer commented Aug 6, 2018

Fixes #309

This is still not fully tested, and I have a few things that need to be fixed (like the url in the commit message).

I'm unsure if the logic here is correct. I test travis-ci.org first and then travis-ci.com. The reason is that travis-ci.com always requires authentication, so .org is preferred. However, maybe it should be the other way around, as apparently it's possible to enable both.

@asmeurer
Copy link
Member Author

asmeurer commented Aug 7, 2018

I think I need to fix the logic to check if both .com and .org exist and ask in that case. Otherwise we have situations like doctr where the .com "exists" but isn't working https://travis-ci.com/drdoctr/doctr, or conversely, repos like the one in the issue, or drdoctr/travis-ci-com-testing that exist and are run on both (https://travis-ci.com/drdoctr/travis-ci-com-testing, https://travis-ci.org/drdoctr/travis-ci-com-testing). I don't think we need to actually support both, as assumedly this situation won't last forever.

@asmeurer
Copy link
Member Author

asmeurer commented Aug 7, 2018

It doesn't seem easy to detect whether or not we are running in .com or .org for the commit message (travis-ci/travis-ci#8935, travis-ci/travis-ci#7552). I'll play with it some more and see if it's possible to extract it.

@asmeurer
Copy link
Member Author

asmeurer commented Aug 7, 2018

I'm not seeing a way, other than saving it somehow in the configure stage, which is annoying considering it's just for the commit message.

@asmeurer
Copy link
Member Author

I guess this is ready for review. I still need to do a lot more testing of it.

Unfortunately, Travis never got back to me on my questions, so for now:

  • Configuring travis-ci.com will require authentication.
  • The URLs in the commit messages for travis-ci.com builds will be wrong (this has already the case for private repo builds on .com ever since they have been supported). You can fix them by replacing .org with .com.

@asmeurer
Copy link
Member Author

Travis now has TRAVIS_JOB_WEB_URL so that fixes the URL issue. Still no word on authentication (travis-ci/travis-ci#9954).

Also remove travis_tld(), which is no longer needed (and didn't actually work
anyway).
Also print it in green so it is noticeable.
--no-authenticate implies --no-upload. --no-upload no longer doesn't ask for
authentication (use --no-authenticate instead). This is because authentication
is now needed for other things than uploading the key (particularly, making
API requests to travis-ci.com).
@asmeurer
Copy link
Member Author

I think this is ready to go. The travis-ci.com testing is at https://travis-ci.com/drdoctr/travis-ci-com-testing.

I've also split out a new --no-authenticate flag for doctr configure, which implies --no-upload. --no-upload no longer doesn't ask for authentication without --no-authenticate. If --no-authenticate is used with a private repo or with a travis-ci.com repo, it is an error. Hopefully Travis can re-enable the ability to get the public key without authenticating, as it isn't a particularly user friendly process. Not only does it make it impossible for someone to add doctr to a repo they don't have access to, it requires temporarily creating a personal access token, which results in a notification email from GitHub (we delete it immediately, but it's still confusing to users).

I'll let this sit for a bit in case anyone wants to review/test. Otherwise I'll merge and get a release out, probably tomorrow or Thursday. I'm still unsure but likely #332 should also go in the release.

@asmeurer
Copy link
Member Author

Also I have not physically tested this on a private repo. That might be a good thing to do.

@asmeurer asmeurer merged commit 3d81f52 into master Nov 30, 2018
@asmeurer asmeurer deleted the travis-ci-dot-com branch November 30, 2018 22:16
@asmeurer
Copy link
Member Author

asmeurer commented Dec 3, 2018

Oh I forgot to add some flags to specify which to use from the command line.

Also it looks like there is a bug for repos that aren't activated on either yet (KeyError).

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.

Issues with new repositories on travis-ci.com
1 participant