Auto-insert 'use Traits' into files via .build.php during build process

[raamdev]

See wpsharks/comet-cache#709

[raamdev]

@jaswsinc A review here when you get a chance, please. I've tested this with Phing and it works as expected.

Note that this PR depends on wpsharks/phings#73 to work properly while building the Lite version. Right now the generated Lite builds include Pro-only Trait files that should be excluded before the -dotbuilds target is run.

[jaswrks]

@raamdev This all looks great to me. The only thing I'm realizing now is that the order of our Phing targets could prevent this from working as expected. Currently (and I don't see a way around this), the -lite target in our Phing script runs last—after the pro targets have completed, and that would include the pro -dotbuilds target as well.

So that means whenever it copies files over to create the lite version, it's going to be copying over files that have already had Traits inserted into them from the pro build that came before the lite build begins. Whenever the -dotbuilds target runs again in the lite version, it won't find the comment marker that you have, because the replacement will have already been done. Does that sound correct?

What I'd suggest is that you have an open comment marker:

• /*[.build.php-auto-generate-use-Traits]*/

and then a closing comment marker:

• /*[/.build.php-auto-generate-use-Traits]*/

Whenever the replacements are done, leave the markers, but change what is inside them. This way it can be run over and over again without issue, and if it runs during the lite build, it will overwrite was done run for the pro build.

[raamdev]

So that means whenever it copies files over to create the lite version, it's going to be copying over files that have already had Traits inserted into them from the pro build that came before the lite build begins.

Ah, tricky tricky. Good catch.

Whenever the replacements are done, leave the markers, but change what is inside them.

Yep, that sounds like a better idea. I'll work on that and let you know when I'm ready for another review. :-)

[raamdev]

@jaswsinc writes...

So that means whenever it copies files over to create the lite version, it's going to be copying over files that have already had Traits inserted into them from the pro build that came before the lite build begins.

Actually, that's not true.

The build script copies the repo files into the Lite staging area, not the built Pro files from ~.build/. So when the -lite target runs and eventually calls the -dotbuilds target, it's acting on the same files that the Pro build acted on (the files from the repo, which contain the marker and not the use Trait lists).

So, I'm not seeing that any changes are necessary, are you?

[jaswrks]

Noting that wpsharks/phings#73 has been implemented and can be tested from the dev branch.

[jaswrks]

The build script copies the repo files into the Lite staging area, not the built Pro files from ~.build/. So when the -lite target runs and eventually calls the -dotbuilds target, it's acting on the same files that the Pro build acted on (the files from the repo, which contain the marker and not the use Trait lists).

I think you make a good argument for that being the way it should be done, or at least much closer to that idea. The Phing build system is turning into something now where that seems like a more desirable behavior. However, as it exists right now, everything before the packages.xml and distros.xml is run from the ${project.basedir} and not from ${project.basedir}/.~build.

See: https://github.com/websharks/phings/blob/160410/src/psr4/targets.xml#L21
See also: https://github.com/websharks/phings/blob/160410/src/psr4/targets/dotbuilds.xml#L16

[raamdev]

@jaswsinc writes...

However, as it exists right now, everything before the packages.xml and distros.xml is run from the ${project.basedir} and not from ${project.basedir}/.~build.

I think you're making my point. The -dotbuilds target runs on... Ah, I see. The -lite target calls another Phing process to run the build again, for the Lite version:

${project.basedir}/.~build/${project_lite_slug}/build.xml

So after that point, ${project.basedir} is actually equal to comet-cache-pro/.~build/comet-cache/. (I was thinking ${project.basedir} always referred to the comet-cache-pro/ repo directory.)

EDIT: See next comment.

I think you make a good argument for that being the way it should be done, or at least much closer to that idea.

Hmm, yeah. That recursion is confusing. It's not that big of a deal. I already knew about the recursion, but it always takes me some extra time to think about the program flow when I'm doing something to the Lite build process.

I'll open a GitHub issue so that we can look into separating out the -lite target. It would be nice to be able to run that target by itself, so that, for example, when testing some work that is specific to the Lite version we could build a Lite zip without also doing the extra work of building the Pro version. A -build-lite target would be cool to have.

[raamdev]

Umm, LOL. Here I go again, agreeing with you and then immediately reversing course in my next comment.

@jaswsinc writes...

So that means whenever it copies files over to create the lite version, it's going to be copying over files that have already had Traits inserted into them from the pro build that came before the lite build begins. Whenever the -dotbuilds target runs again in the lite version, it won't find the comment marker that you have, because the replacement will have already been done. Does that sound correct?

That is not correct. As it stands right now, the .build.php files will find the /*[.build.php-auto-generate-use-Traits]*/ comment marker, during both the Pro and the Lite builds.

Here's why:

The -lite target copies the repo files into the Comet Cache Lite staging area.

${project.basedir} (i.e., the comet-cache-pro/ repo directory—remember, we're still running from the context of the first build process, so ${project.basedir} is still referring to the base repo directory) gets copied into ${project.basedir}/.~build/${project_lite_slug} (i.e., comet-cache-pro/.~build/comet-cache/).

At that point, we have a fresh set of unmodified files from the repo directory in our Comet Cache Lite staging area, files that have not been affected by the Pro build process and therefore have not yet been touched by .build.php.

So, when we run .build.php a second time—on the files in comet-cache-pro/.~build/comet-cache/—they will find the /*[.build.php-auto-generate-use-Traits]*/ markers and do their job as expected.

---

To confirm all of this you can run a test with the way things are now and you'll see. After running a phing build from the feature/709 branch of the Comet Cache Pro repo, inspect the following files to confirm they include the use Traits lists as expected:

comet-cache-pro/.~build/comet-cache/.~build/comet-cache/comet-cache/src/includes/classes/AdvancedCache.php
comet-cache-pro/.~build/comet-cache/.~build/comet-cache/comet-cache/src/includes/classes/AbsBaseAp.php
comet-cache-pro/.~build/comet-cache/.~build/comet-cache/comet-cache/src/includes/classes/Plugin.php

---

So, I'm back to feeling that nothing needs to change in this PR. 😄

[raamdev]

@jaswsinc In d36924d I updated the .build.php scripts to look for start and end markers and update what's inside them, as you suggested. Here's why I realized that makes a lot more sense:

We're discussing in wpsharks/phings#76 how the -dotbuilds target should not modify the working directory because that would result in needing to commit the changes that the .build.php file makes. However, with the change in d36924d that doesn't matter.

By simply having the .build.php file replace what's in between the markers we can commit the changes that it makes the first time (as I did in d36924d) and now whenever the build system runs it will modify those files and Git will see that nothing has changed (unless something has changed and .build.php generates a different list).

If you checkout this feature/709 branch and run phing after my last commit, you'll see that even though the .build.php file is generating and updating the use Traits lists, Git doesn't report any modifications because the list hasn't changed.

[jaswrks]

Git will see that nothing has changed (unless something has changed and .build.php generates a different list).

Right. This looks great to me.