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

Idempotent build: manual.xml.in #210

Merged
merged 6 commits into from
Jan 20, 2025
Merged

Idempotent build: manual.xml.in #210

merged 6 commits into from
Jan 20, 2025

Conversation

alfsb
Copy link
Member

@alfsb alfsb commented Dec 16, 2024

This PR removes the last non-PhD related, non idempotency of manual. All other are PhD related and will need coordination.

Tested on all languages, with and without CHM enabled. Few languages have broken XML files inside chmonly/. This PR need to be tested on Windows, in particular, by builders of CHM manuals, to see if nothing got broken in this process. The tests showed identical XML output, but changes on DTD area.

Opened to review, but I'm not expecting merging this before 2025. There is nothing blocking this, really, only to avoid injecting possible breakages near new year festivities.

The broken chmonly/ files I found are:

  • fr/chmonly/search.xml
  • ja/chmonly/usingchm.xml

@alfsb alfsb mentioned this pull request Dec 16, 2024
32 tasks
@alfsb alfsb requested review from Girgias and mumumu December 18, 2024 11:31
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Wouldn't it be better to use process instruction for the inclusion of the translation layer and CHM?

@alfsb
Copy link
Member Author

alfsb commented Dec 21, 2024

The PI alternative for this would be at XML parse, or as after parse fixup?

@alfsb
Copy link
Member Author

alfsb commented Dec 28, 2024

Wouldn't it be better to use process instruction for the inclusion of the translation layer and CHM?

I do not think that processing instructions will work, for the same reason that XInclude wont work in general: entities. Files at chmonly/ have entity references. And entity declarations do not survive parse time, nor libxml appear to share entity declarations between files (this last bit is something I will try more experiments latter on).

This PR only focuses on making manual building as idempotent as possible, and for this, it's only necessary to avoid writing/changing files outside temp/.

@alfsb
Copy link
Member Author

alfsb commented Jan 6, 2025

I plan to merge this and #207 mid January, so it will possible to start working in pushing PnD stuff at the end of configure.php, and some other optimizations.

@alfsb alfsb force-pushed the stable-manual-xml branch from 694e927 to 8f5b45b Compare January 6, 2025 15:37
@Girgias
Copy link
Member

Girgias commented Jan 14, 2025

Sorry for the delayed response.

Well if PIs are not going to work then this is fine.

@alfsb
Copy link
Member Author

alfsb commented Jan 15, 2025

Well if PIs are not going to work then this is fine.

I do not think PIs after parse time are gone to work, because files with undefined entity references are impossible to load (so there is nothing valid to patch or append). There is a way use PIs at parse time, returning raw CDATA while loading a XML file? @nielsdos

@nielsdos
Copy link
Member

There is a way use PIs at parse time, returning raw CDATA while loading a XML file? @nielsdos

I don't think so.

@alfsb
Copy link
Member Author

alfsb commented Jan 20, 2025

Merging. There is still a few points where configure.php leaves intermediary files behind, outside temp/, but their appear not affect XML generation or validation. Focusing on performance now.

@alfsb alfsb merged commit ffa8870 into php:master Jan 20, 2025
11 of 12 checks passed
@alfsb
Copy link
Member Author

alfsb commented Jan 23, 2025

There is a way use PIs at parse time, returning raw CDATA while loading a XML file? @nielsdos

I don't think so.

I cannot find one, either. We are so accustomed to using around <php code(); ?> around output, but I do not think it's possible to have libxml "run" and receive the output of processing instructions as PHP does.

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.

3 participants