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

-dotbuilds should not modify working tree #76

Closed
raamdev opened this issue Apr 10, 2016 · 10 comments
Closed

-dotbuilds should not modify working tree #76

raamdev opened this issue Apr 10, 2016 · 10 comments
Assignees

Comments

@raamdev
Copy link
Contributor

raamdev commented Apr 10, 2016

The -dotbuilds target currently modifies repo files. It should instead only modify files in the .~build/ directory.

Here's why:

The -dotbuilds target runs .build.php files, which are designed to make some portion of the codebase change during the build process. However, the changes that are made by the .build.php files should never be committed to the repo—the whole point of the .build.php files are to avoid committing code that should be dynamically generated during the build process.

As it stands now, you end up with a dirty working tree after the -dotbuilds target runs, because files in the repo have changed. If -dotbuilds was modifying files in .~build/ instead, those changes would not make the working tree dirty.

Yes, all this means is that we'd need to reset those files after running Phing, but that shouldn't be necessary. Also, more importantly, modifying the repo files makes it possible to accidentally commit those changes, which would silently break the .build.php files.

Note that other targets, like -rtokens, do modify the repo files, but that's OK because it's expected that those changes will be committed to the repo as part of the release process (e.g., incrementing version numbers, etc.).

@raamdev raamdev changed the title -dotbuilds modifies repo files -dotbuilds should not modify working tree Apr 10, 2016
@raamdev
Copy link
Contributor Author

raamdev commented Apr 10, 2016

Oh, I see now that the .~build/ directory isn't even populated until after the zip file has been generated in -packages. I guess I misunderstood how much the .~build/ directory was being used (i.e., it's not used for anything other than unzipping the package towards the end of the build process).

I still feel this is an issue we need to figure out, because running the -dotbuilds target is required to build a distributable for testing purposes. If it was just something that happened during a release process, I could deal with resetting those modified files and not worry too much about accidentally committing them. But as it stands now, anybody who wants to build a distributable for testing a feature branch needs to make sure they reset the files modified by the -dotbuilds target before committing their changes.

Which leads me to another issue with the way things are now: What if the feature branch being worked on modifies one of the files that the -dotbuilds target modifies? If you're doing a build as part of testing your changes locally and you haven't committed your changes, you won't even see that the -dotbuilds target made additional modifications, unless you do a diff and look for those changes.

@raamdev
Copy link
Contributor Author

raamdev commented Apr 10, 2016

Noting that the new -js-css target added by #77 ↑ is affected by the same issue described above, i.e., it will leave the working tree modified in a way that should never be committed.

@jaswrks jaswrks self-assigned this Apr 10, 2016
jaswrks pushed a commit that referenced this issue Apr 10, 2016
@jaswrks jaswrks mentioned this issue Apr 10, 2016
@jaswrks
Copy link
Contributor

jaswrks commented Apr 11, 2016

Hmm. Well, typically a "build" (in the truest sense) is to compile a binary or compile from one language to another, or from one set of files to another set of files that are derived from the original. In most of our projects the closest thing we have to a true build (in that sense) would be the lite variation.

I have been giving this some thought today and for me it comes down to what our end goal is for the main project repo. Since we are using Composer and following other PSR guidelines, our goal (which is also the goal for most PHP projects like ours) is to have a repo that:

  • Contains a composer.json file that satisfies dependencies.
  • The composer.json also allows other developers to use our project as a dependency; e.g., if you are building a WordPress site and want to include Comet Cache, you can add us to your composer.json file and have it pulled in by Composer. Important: the parent project in that case will expect that the repo contains the files in a ready state; i.e., Composer doesn't offer any concept of a build itself, other than to load dependencies recursively and place binaries in a vendor/bin directory.
    • Another big advantage when it comes to having a self-contained repo is that it makes it much easier to work on a project, because you can make changes and test them out without needing to rebuild each time; i.e., it makes it possible to "tweak something and test it"; instead of "tweak something, rebuild, and then test it".

      We only rebuild when there is a need to modify the project files in some automated way after changes occur that would require our build process to run again; e.g., changing the list of Traits, or preparing a release where the version numbers are going to change.

Now, because we also use Phing in our projects, it makes it possible for us to automate some things above and beyond what is offered by Composer alone. For instance, we can:

  • Automate the injection of things like Traits via .build.php files.
  • Build a ZIP file for redistribution that is stripped down to reduce filesize and improve security.
  • Generate POT translation files for WordPress.org distribution.
  • Generate a codex that helps to explain how the software works, based on docBlocks.
  • Update tokens in the software by altering the version and other minimum versions that are subject to change with each formal release of the software that we tag at GitHub.
  • Run a lint checker against the software to help everyone catch problems with code style.
  • Run a suite of tests provided by the repo.
  • Build a lite variation and carry out the same operations again in that variation.
  • Eventually, to help us automate the entire release process by uploading/pushing files.

Of those items in the list currently, I break those apart into two classifications:

  • 1. Modifies or enhances the software in some way.
    • Automate the injection of things like Traits via .build.php files.
    • Generate POT translation files for WordPress.org distribution.
    • Update tokens in the software by altering the version and other minimum versions that are subject to change with each formal release of the software that we tag at GitHub.
  • 2. Utilities in one form or another.
    • Build a ZIP file for redistribution that is stripped down to reduce filesize and improve security.
    • Generate a codex that helps to explain how the software works, based on docBlocks.
    • Run a lint checker against the software to help everyone catch problems with code style.
    • Run a suite of tests provided by the repo.
    • Build a lite variation and carry out the same operations again in that variation.
    • Eventually, to help us automate the entire release process by uploading/pushing files.

Here are the items (anything that modifies the software) that I feel should be performed in the base directory for the repo itself, so as to always keep the repo in a ready state; i.e., ready to be used as-is. In other words, changes introduced by these routines should be committed in my view.

  • 1. Modifies or enhances the software in some way.
    • Automate the injection of things like Traits via .build.php files.
    • Generate POT translation files for WordPress.org distribution.
    • Update tokens in the software by altering the version and other minimum versions that are subject to change with each formal release of the software that we tag at GitHub.

Narrowing that down a bit further, we can separate what should be run by anyone working on the project, and what should only be run by the project lead. This is a list that is derived from conversations we have had in the past, and it reflects the current difference between running just phing (or phing build) and running phing build-all as the project lead.

Running phing or phing build (default target):

  • Automate the injection of things like Traits via .build.php files.

Running phing build-all as the project lead:

  • Automate the injection of things like Traits via .build.php files.
  • Generate POT translation files for WordPress.org distribution.
  • Update tokens in the software by altering the version and other minimum versions that are subject to change with each formal release of the software that we tag at GitHub.

What remains are all things I consider to be utilities in one form or another:

  • 2. Utilities in one form or another.
    • Build a ZIP file for redistribution that is stripped down to reduce filesize and improve security.
    • Generate a codex that helps to explain how the software works, based on docBlocks.
    • Run a lint checker against the software to help everyone catch problems with code style.
    • Run a suite of tests provided by the repo.
    • Build a lite variation and carry out the same operations again in that variation.
    • Eventually, to help us automate the entire release process by uploading/pushing files.

@jaswrks
Copy link
Contributor

jaswrks commented Apr 11, 2016

On the other hand, if our goal is to have a base set of files in the repo that are not necessarily in a ready state; i.e., a build is absolutely required for the repo to work as intended in every way, then I'd say that almost anything we do (with just a couple of exceptions) should be run from the .~build directory instead. That way the base directory (i.e., the repo) is a set of source files only.

I'm not totally against doing that, but I would hesitate to do it due to the fact that it would break the intended functionality offered by Composer. For WordPress plugins, that's not such a big deal, because I suspect very few people will use composer.json to build their WordPress site. Some do, but not that many. However, when we take the same model and try to apply that to a project like the HTML Compressor or JS Minifier (i.e., if those required a separate build process) then suddenly it becomes much harder for us to use Composer as intended.

For that matter, even if we leave Composer out of the equation for a moment, do we really want to have a GitHub repo for a WordPress plugin that you can't just clone and use as-is? I think it could be a mistake to go down that path, because eventually WordPress and Git are going to meet in one way or another. They already have in a few smaller projects, but eventually a Git repo could be used to source the installation of a plugin. If that repo contains files that are not in a ready state, then site owners will have a problem.

@jaswrks
Copy link
Contributor

jaswrks commented Apr 11, 2016

One thing I didn't include in the lists above would be the new JS/CSS target, and just looking at what that does now it probably should be fit into a 3rd classification: Cleanups. That's a routine that does something that I agree should not be committed to the repo, because it would remove something that developers working on the project would like to have, and it doesn't necessarily need to be removed for someone to use the repo as-is.

So in the case of the new JS/CSS target, I think an exception should be made where that (as you suggest) could be run from the .~build directory so that what it changes is not committed.

@raamdev
Copy link
Contributor Author

raamdev commented Apr 11, 2016

@jaswsinc writes...

For that matter, even if we leave Composer out of the equation for a moment, do we really want to have a GitHub repo for a WordPress plugin that you can't just clone and use as-is? I think it could be a mistake to go down that path

That is already the case. The Pro repo can't be cloned and used as-is--you need to run phing so that Composer runs. The Lite repo is somewhat unique in that it just contains a copy of whatever the -lite target generates. We don't use the Lite repo for anything other than GitHub issues and tagging releases.

On the other hand, if our goal is to have a base set of files in the repo that are not necessarily in a ready state; i.e., a build is absolutely required for the repo to work as intended in every way, then I'd say that almost anything we do (with just a couple of exceptions) should be run from the .~build directory instead. That way the base directory (i.e., the repo) is a set of source files only.

I'm not totally against doing that, but I would hesitate to do it due to the fact that it would break the intended functionality offered by Composer. For WordPress plugins, that's not such a big deal, because I suspect very few people will use composer.json to build their WordPress site. Some do, but not that many. However, when we take the same model and try to apply that to a project like the HTML Compressor or JS Minifier (i.e., if those required a separate build process) then suddenly it becomes much harder for us to use Composer as intended.

I think we need to step back for a moment. Phing is a project build system. Composer is a dependency manager. They're two different things designed to solve two very different problems.

I see Phing as something that becomes necessary after a project grows to a certain size (e.g., when it starts having several dependencies, which may be managed with Composer). All projects could use Phing, but for some projects it would be overkill. (I'm not advocating that we don't use it for all projects—I'm just making a point that Phing is not designed to be something that you set up once and then use for all your projects, as we're trying to do—it's designed to be project-specific.)

Not all projects are equal. The HTML Compressor and the JS Minifier are not the same type of project as Comet Cache. Comet Cache is larger, requires many dependencies, has multiple variations (Pro vs Lite), is published to two places (WordPress.org via SVN and GitHub via Git), has releases that must be uploaded to multiple places (GitHub, WordPress.org, and CometCache.com), has translation files that need to be generated, and so on.

The Comet Cache project is a great example of where something like Phing can be used to automate all of the tasks necessary to build a project. Note that my definition of "build" here is nothing like your earlier definition of build "in the truest sense": When I think of building Comet Cache I think of "what's required to release Comet Cache". That's what I see Phing being used for: We use it to help automate the steps necessary to produce a release ready for public consumption. (The only reason it's used during development is that using Phing makes it easier to build an installable WordPress plugin, which is necessary for testing changes.)

The Phing website even reinforces this thinking:

If you find yourself writing custom scripts to handle the packaging, deploying, or testing of your applications, then we suggest looking at the Phing.

That's not to say that we can't use Phing for smaller projects like the HTML Compressor or JS Minifier, but I think it's wrong to design Phing build scripts with a one-size-fits-all mentality. Different projects have different requirements. The more different the projects, the more likely they are to have build processes that are incompatible (again, using 'build processes' here to refer to 'tasks that are required to produce a public release').


I feel like the issues that we're discussing here with doing stuff in .~build/ vs changing the working directory, can be directly attributed to trying to create a "one size fits all" set of Phing build scripts. The reality is that the HTML Compressor and JS Minifier will be built differently than a WordPress plugin (and by 'built', I mean 'tasks that are required to produce a public release').

What we really need, IMO, is to think about the intended use for each of our projects and group them according to similarities in the steps necessary to produce a public release. Then we need to design our build targets to apply to those types of projects. You might have a better idea for high-level grouping, but here's what comes to my mind:

  • WordPress plugins/themes
  • WebSharks libraries

With that in mind, we could either maintain a single Phings repo and have build targets with names like -build-wp, -build-all-wp, -build-lib, -build-all-lib, etc., or we could maintain two separate repos, one intended for WordPress plugins/themes (e.g., phings-wp) and one intended for WebSharks libraries (e.g., phings-lib).

I'd normally be in favor of separate repos, but I feel like we're so far along with the current phings repo that separating them out would result in too much duplicated work.

We've come a long way with our Phing build system without hitting this divergence point so I'd say that we should create -build-wp and -build-lib targets, and let those contain collections of other targets that apply to the appropriate project type (e.g., we could have a -dotbuilds-wp specifically for .build.php files intended to run for a WordPress project, and have it apply the changes to the .~build/ directory; there could be another -dotbuilds-lib that is intended to be used with the -build-lib target).

@raamdev
Copy link
Contributor Author

raamdev commented Apr 11, 2016

Noting that a move towards more independence within the Phing build system would be in alignment with the intention of this issue: #75

@raamdev
Copy link
Contributor Author

raamdev commented Apr 11, 2016

Noting that my latest commit and comment in wpsharks/comet-cache-pro#240 (comment) removes the need for the -dotbuilds target to apply only to the .~build/ directory.

That said, I still feel that my point above about creating some separation between project types applies. The "Cleanups" classification you mentioned earlier makes it clear that some projects will need to generate a .~build/ directory, make some changes to its content, and then package that directory for distribution. (Currently packaging happens before .~build/ is populated and the packages are sourced from the repo directory, not the .~build/ directory.)

@jaswrks
Copy link
Contributor

jaswrks commented Apr 11, 2016

That is already the case. The Pro repo can't be cloned and used as-is--you need to run phing so that Composer runs. The Lite repo is somewhat unique in that it just contains a copy of whatever the -lite target generates. We don't use the Lite repo for anything other than GitHub issues and tagging releases.

Ah, right. Good point. I think my choice of words there, "if we leave Composer out of the equation for a moment", is where I went wrong. Composer is to PHP what NPM is to Node projects; i.e., Composer is the dependency manager for PHP. So whenever I think about a PHP project being in a ready state, I think of it either being:

  • Not dependent on Composer at all; i.e., all self-contained.
  • Or, it uses Composer and comes with a composer.json file.

So in terms of it being in a ready state, my feeling is that a project which depends on Composer and has a composer.json file is also in a ready state, because if a system is going to allow for PHP projects to be pulled it from a Git repo it would need to also be capable of running Composer to satisfy the dependencies that a project has. That is a standard for PHP projects. It would be nice if Phing were too, and that does seem to be a growing trend, but my feeling is that Phing is not nearly as widespread.

I have seen that Composer makes it possible for us to define a custom installer, and they even have WordPress installers that we could base from. It might be worth it for us to explore ways that we could connect Composer dependency management with Phing in some way in the future; i.e., to instruct Composer to run Phing whenever anyone uses it to pull one of our repos in as a dependency.

Still, I think the main reason for Phing not being nearly as widespread, is because PHP projects don't typically need to be built. Sure, there are some special cases where you might have a PHP project that does need to be "built", but those seem to be few and far between, and I don't consider a WordPress plugin to be something that anyone would expect needs to be built in order to use.

WordPress plugins/themes
WebSharks libraries

so I'd say that we should create -build-wp and -build-lib targets, and let those contain collections of other targets that apply to the appropriate project type

Cool idea. No objection there.

@raamdev
Copy link
Contributor Author

raamdev commented Apr 11, 2016

@jaswsinc Cool. I'm going to close this GitHub issue and fork it into a new one specifically geared towards improving the separation of targets for specific types of projects.

The original reason this issue was opened was to satisfy a problem I came across with Comet Cache Pro .build.php files, but since that has been resolved I'm not seeing a reason for this specific issue to remain opened.

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