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

Perform domain substitution before patching #1185

Closed
Luka-Filipovic opened this issue Sep 13, 2020 · 5 comments
Closed

Perform domain substitution before patching #1185

Luka-Filipovic opened this issue Sep 13, 2020 · 5 comments

Comments

@Luka-Filipovic
Copy link

Is your feature request related to a problem? Please describe.
Patches can't be reverted without reverting domain substitution process, which would reset the whole incremental build.

Describe the solution you'd like
Simply apply domain substitution before patches.
All patches should be updated to work on source that is already domain substituted. This could easily be automated.

@Luka-Filipovic Luka-Filipovic changed the title Apply domain substitution before patching Perform domain substitution before patching Sep 13, 2020
@Eloston
Copy link
Member

Eloston commented Sep 19, 2020

Why is this necessary when the domain substitution utility can revert the changes?

@Eloston Eloston added the need info Need feedback to proceed label Sep 19, 2020
@Luka-Filipovic
Copy link
Author

Luka-Filipovic commented Sep 19, 2020

Correct me if I'm wrong, but here's one example to show my point.
Do a full build with all patches and domain sub after patches, and afterward decide to do a new build without some a.patch. That would require reverting domain sub, reverting a.patch, applying domain sub and starting a new build.
By redoing domain sub, timestamps of all files in domain sub list are changed. Ninja incremental build checks only timestamps and even if files remain the same, they are complied once again, and basically that is like doing a full build instead of only a few actions affected by removal of a.patch.

Or looking at it just from philosophy pov, domain substitution is something that is always done and is never changed, while patches can differ from user to user and can always be experimented with, so it makes more sense to do them last.

@Eloston
Copy link
Member

Eloston commented Sep 19, 2020

How is this issue different from #849? Does 1a0e163 address your issue?

@Luka-Filipovic
Copy link
Author

I wasn't aware domain_substitution.py was modified to keep a track on timestamps, that definitely solves the problem listed.
Nevertheless, it would be simpler to only revert patches without having to redo domain sub each time
Another advantage would be ability to fix some necessary links domsub brakes with patches.
But it's really not something of a high priority right now.

@Eloston
Copy link
Member

Eloston commented Sep 19, 2020

Right now, domain substitution is a pretty crude and inefficient tool. There are proposals like #1026 that are steps in the right direction, but there still needs to be more work on this tool.

Closing as duplicate since the timestamp manipulation logic solves this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants