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

With github pullrequests work on target repo rather than the source #816

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11546,10 +11546,13 @@ done`;
// being built.
let repoFullName = process.env.GITHUB_REPOSITORY;
if (github.context.payload.pull_request) {
repoFullName = github.context.payload.pull_request.head.repo.full_name;
repoFullName = github.context.payload.pull_request.base.repo.full_name;
}
const headRef = process.env.GITHUB_HEAD_REF;
const commitRef = headRef || github.context.sha;
let commitRef = headRef || github.context.sha;
if (github.context.payload.pull_request) {
commitRef = github.context.sha;
}
const repoFilePath = path.join(rosWorkspaceDir, "package.repo");
// Add a random string prefix to avoid naming collisions when checking out the test repository
const randomStringPrefix = Math.random().toString(36).substring(2, 15);
Expand Down
7 changes: 5 additions & 2 deletions src/action-ros-ci.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,10 +555,13 @@ done`;
// being built.
let repoFullName = process.env.GITHUB_REPOSITORY as string;
if (github.context.payload.pull_request) {
repoFullName = github.context.payload.pull_request.head.repo.full_name;
repoFullName = github.context.payload.pull_request.base.repo.full_name;
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, the git commit for the "PR merge" commit that we are trying to check out is only available on the target repo (i.e., the original repo against which the PR was opened), correct?

Copy link
Author

Choose a reason for hiding this comment

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

correct. But even more critically, the token which GH generates automatically in the target repo is not valid in the source repo (which is not public).

Copy link

Choose a reason for hiding this comment

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

@christophebedard since there is no one maintaining vcstool, and this tool is maintained.
Would you consider dropping usage of vctools and clone repos in a different way?
Might be easier then trying to fix it in 2 repos

Copy link
Member

Choose a reason for hiding this comment

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

But even more critically, the token which GH generates automatically in the target repo is not valid in the source repo (which is not public).

Got it.

since there is no one maintaining vcstool, and this tool is maintained.

The goal is for OSRF to get maintainer access eventually, but that's going to take some time.

Would you consider dropping usage of vctools and clone repos in a different way?
Might be easier then trying to fix it in 2 repos

For the repo being tested, yes, that's a good idea. Running plain old git commands instead of using vcstool would just be simpler here. Would you be willing to make this change? I wouldn't change anything about the vcs-repo-file-url option (for other repos), though.

}
const headRef = process.env.GITHUB_HEAD_REF as string;
const commitRef = headRef || github.context.sha;
let commitRef = headRef || github.context.sha;
if (github.context.payload.pull_request) {
commitRef = github.context.sha;
}
const repoFilePath = path.join(rosWorkspaceDir, "package.repo");
// Add a random string prefix to avoid naming collisions when checking out the test repository
const randomStringPrefix = Math.random().toString(36).substring(2, 15);
Expand Down