这是indexloc提供的服务，不要输入任何密码
Skip to content

fix(scripts/lint-packages): use package directory for version diff check #26750

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 2, 2025
Merged

fix(scripts/lint-packages): use package directory for version diff check #26750

merged 1 commit into from
Oct 2, 2025

Conversation

@TomJo2000
Copy link
Member

@TomJo2000 TomJo2000 commented Oct 2, 2025

and try to work out the issue causing git merge-base to not find the base commit of a PR branch.

@TomJo2000
fix(scripts/lint-packages): get master branch head from refs.
0859e3b 
- also use package directory instead of build.sh for version diff check
@robertkirkman robertkirkman self-requested a review October 2, 2025 19:22
owokitty added a commit to owokitty/termux-packages that referenced this pull request Oct 2, 2025
@owokitty
test PR termux#26750
de7782d
@owokitty owokitty mentioned this pull request Oct 2, 2025
Closed
robertkirkman
robertkirkman approved these changes Oct 2, 2025
View reviewed changes
Copy link
Member

@robertkirkman robertkirkman left a comment

Choose a reason for hiding this comment

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

I tested this in

I think this is working.

@TomJo2000
Copy link
Member Author

TomJo2000 commented Oct 2, 2025 •
edited
Loading

I did a similar test above (based on #26744's initial issue) but I had already cleaned up the commits on this branch.
https://github.com/termux/termux-packages/actions/runs/18201628698/job/51821761405?pr=26750#step:5:26

@TomJo2000 TomJo2000 merged commit f7f0300 into termux:master Oct 2, 2025
2 checks passed
@TomJo2000 TomJo2000 deleted the 2lint2furious branch October 2, 2025 19:36
@robertkirkman
Copy link
Member

robertkirkman commented Oct 2, 2025

I was testing what would happen if the name of the PR branch was "master", but I didn't see anything wrong happen as far as I can tell.

@TomJo2000
Copy link
Member Author

TomJo2000 commented Oct 2, 2025 •
edited
Loading

Ah yeah that makes sense.
That's accounted for since we're reading .git/refs/remotes/origin/master.
Whereas the PR's branch is at .git/refs/remotes/pull/${PR_number}.
And local master is at .git/refs/heads/master.

@robertkirkman
Copy link
Member

robertkirkman commented Oct 4, 2025

I have determined in wild (downstream project's GitHub Actions CI) that this change has a dependency on this line, because this line uses git fetch origin master. After this change, if this lint-packages.sh is ever used in a CI workflow that was in a PR but did not have git fetch origin master already somewhere in it, there will be an error.

termux-packages/.github/workflows/packages.yml

Line 85 in 5abfc71

if [[ "${{ github.event_name }}" == "pull_request" && -n "$(git rev-list --merges "$(git fetch origin master >&2; git merge-base origin/master $HEAD_COMMIT)..$HEAD_COMMIT")" ]]; then

maybe that's not very important, since official termux-packages CI has that line, but I thought it important to write this note in case this change confuses anyone else or anyone else encounters the problem.

@robertkirkman robertkirkman mentioned this pull request Oct 5, 2025
Merged
@robertkirkman robertkirkman mentioned this pull request Oct 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robertkirkman robertkirkman robertkirkman approved these changes

@Grimler91 Grimler91 Awaiting requested review from Grimler91

@thunder-coding thunder-coding Awaiting requested review from thunder-coding

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@TomJo2000 @robertkirkman