feat: allow specifying external commit from submodule #1026
Conversation
mistydemeo
force-pushed
the
github-releases-repo-submodule
branch
3 times, most recently
from
374d178
to
c8f9863
Compare
| @@ -577,6 +577,7 @@ jobs: | |||
| owner: {{{ github_releases_repo.owner }}} | |||
| repo: {{{ github_releases_repo.repo }}} | |||
| token: ${{ secrets.GH_RELEASES_TOKEN }} | |||
| commit: ${{ fromJson(needs.host.outputs.val).ci.github.external_repo_commit }} | |||
There was a problem hiding this comment.
This needs to be wrapped in some kind of if, since it should only be done if the submodule config is set
There was a problem hiding this comment.
I believe if it's passed as null, it's functionally the same as if it weren't passed at all - that's why I didn't do any kind of "if" here. https://github.com/ncipollo/release-action/blob/1cbdc80532ba923d3d81ebd1cece305c437b8d66/src/Inputs.ts#L110-L116
There was a problem hiding this comment.
Will that still work right if we make external_repo_commit skip_serializing_if?
There was a problem hiding this comment.
Right, I guess I'm passing it as an empty string if not set - should fix that
There was a problem hiding this comment.
After we did a little looking, we've settled on that this should work as-is.
cargo-dist/src/tasks.rs
Outdated
| // history that aren't reflected by the submodule commit history, | ||
| // won't be reflected here. | ||
| fn submodule_head(submodule_path: &Utf8PathBuf) -> DistResult<String> { | ||
| let output = Cmd::new("git", "fetch expected latest commit for a submodule") |
There was a problem hiding this comment.
This isn't the format of the error string, it's an explanation of what we were trying to do (gets prefixed with "failed to"). So it should be like "get submodule commit".
There was a problem hiding this comment.
Oh yeah, sorry, "expected" wasn't meant as an error message - it was because this fetches the cached/configured (eg "expected") commit of the submodule rather than what's currently there on disk
There was a problem hiding this comment.
Switched to the technical git term, "cached".
| .arg(submodule_path) | ||
| .output() | ||
| .map_err(|_| DistError::GitSubmoduleCommitError { | ||
| path: submodule_path.to_string(), |
There was a problem hiding this comment.
Should this wrap the axoprocess error? Oh I see, the other path doesn't have that.
|
|
||
| let line = String::from_utf8_lossy(&output.stdout); | ||
| // Format: one status character, commit, a space, repo name | ||
| let line = line.trim_start_matches([' ', '-', '+']); |
mistydemeo
force-pushed
the
github-releases-repo-submodule
branch
2 times, most recently
from
b607671
to
ed4330b
Compare
This is used with `github_releases_repo` to specify that the commit to tag is the commit that's the current HEAD of submodule within this repo.
mistydemeo
force-pushed
the
github-releases-repo-submodule
branch
from
ed4330b
to
4323899
Compare
This is the companion to
github-releases-repo; when targeting creating a commit in an external repo, where that repo is also checked out as a submodule of the repository in which the release is being run, this lets you consult the latest commit of that submodule and use that commit as the tag instead of whatever happens to be the latest commit in that repository.