Skip to content

Debug XML check failure - #790

Closed
alfsb wants to merge 1 commit into
php:masterfrom
alfsb:xml-check-path-debug
Closed

Debug XML check failure#790
alfsb wants to merge 1 commit into
php:masterfrom
alfsb:xml-check-path-debug

Conversation

@alfsb

@alfsb alfsb commented Aug 6, 2026

Copy link
Copy Markdown
Member

No description provided.

@alfsb

alfsb commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

@lacatoire , here is a question for you: the checkout of doc-en as en inside the checkout of translation at root, is intentional?

Note the output of steps "Initial state", "After checkout, without: repository, path" and "After checkout, with: repository, path", in: https://github.com/php/doc-pt_br/actions/runs/31059766682/job/92484939615

In particular, note that in the last case, an en entry is listed after docbookcs.xml, which does not show before.

@lacatoire

Copy link
Copy Markdown
Member

@lacatoire , here is a question for you: the checkout of doc-en as en inside the checkout of translation at root, is intentional?

Note the output of steps "Initial state", "After checkout, without: repository, path" and "After checkout, with: repository, path", in: https://github.com/php/doc-pt_br/actions/runs/31059766682/job/92484939615

In particular, note that in the last case, an en entry is listed after docbookcs.xml, which does not show before.

Yes, intentional: the check script expects the translation at the working directory root and doc-en under en/, as noted in its header comment. It works because en/ is untracked, so it never appears in the diff the check consumes.

It is not the sibling layout the rest of the tooling uses, though. Happy to move the translation to path: pt_br and adjust the script accordingly if you prefer the consistency.

@alfsb

alfsb commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

If it is intended, it is ok. I would only ask, in future and if this yaml is changed for another reason, to also annotate this tree layout at yaml level.

But I asked because of the strange results of Structure/Check XML results of https://github.com/php/doc-pt_br/pull/789/checks .

In that PR, only one file was modified, not a .xml one, and even so the check is listing (and failing) some .xml files, for errors (xi:fallback) that does not exist anymore, in any files of either repository.

So are two possible problems here. The listing of xml files, and the PR inspecting the past, aven after the PR is pushed. At first I suspected that was because of the unusual checkout inside the checkout layout.

@alfsb

alfsb commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Oh, I think I got it. The HEAD of "this" repository is a merge commit. So it has two parents, and so, listing from BASE to HEAD will list all files changes in both branches.

So check is listing files not changed by the PR, but files also changed in main/master. This is intended?

To avoid listing files outside the PR, the checkout or diff need to be using the ref/HEAD as ${{ github.event.pull_request.head.sha }}, so "BASE to HEAD" will only consider a path inside the PR.

@lacatoire

Copy link
Copy Markdown
Member

@alfsb nice catches, i've done the PRs

@alfsb

alfsb commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Thanks. Closing here, applying there.

@alfsb alfsb closed this Aug 7, 2026
@alfsb
alfsb deleted the xml-check-path-debug branch August 7, 2026 12:13
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