[push:code] build artifacts

[danepowell]

Motivation
Currently, ACLI supports pulling code but not pushing it. Additionally, it has no awareness of the distinction between "source" and "build" repositories. Best practice for Drupal 8+ using Composer is to keep a "source" repo (without dependencies committed, i.e. Github) and an "artifact" repo (with dependencies committed, i.e. Acquia Git). The only way to support this workflow today is with blt deploy, which is overly complicated for most users.

Proposed changes
Replicate the blt deploy command in ACLI, but in a much more streamlined form that's more tightly coupled with Acquia environments, so it's a true deployment manager (not just an artifact builder): allow users to select a specific environment to push to (as long as that environment has a branch checked out).

Alternatives considered
Creating a standalone CLI tool to manage deployments so that Acquia CLI remains decoupled from the guts of a Drupal codebase. But this would involve duplicating so much of Acquia CLI, I think it belongs here.

Testing steps

1. Follow the contribution guide to set up your development environment.
2. Clear the kernel cache to pick up new and changed commands: acli ckc
3. Clone a Drupal project where vendor dependencies have not been committed. Or create a new project using composer create-project acquia/drupal-recommended-project, which should ignore vendor dependencies.
4. Run acli push:code and verify that usable code (with production dependencies committed) is deployed to your Acquia environment.

Merge requirements

• Bug, enhancement, or breaking change label applied
• Manual testing by a reviewer

[danepowell]

@grasmash @anavarre would you mind taking this for a spin and providing preliminary feedback on the functionality and code? If you're good with the general approach, I'll clean it up and add tests.

[grasmash]

When Acquia Cloud Platform eventually supports tasks that perform Composer assembly, I'd like that task to simply run an Acquia CLI command.

For that reason, it's likely best to allow a user to generate a deployment artifact without pushing it. E.g., run acli push:code --dry-run or acli push:code --build-only.

[danepowell]

Let me know if you have any other feedback, or if it's time to write tests

[grasmash]

I'm concerned that this command does a lot more than the name implies. acli push:code does not imply a "build and push" operation. I think we need to make that very clear so that the user knows what process they are initiating.

[danepowell]

I'm concerned that this command does a lot more than the name implies

It does, but I think the alternative (just pushing code without any sort of intelligent build process or deployment) is likely to lead to more confusion. This is what killed BLT over and over: when people want to deploy, they mostly want to deploy to some environment. Making deployment a two- or three-step process (building, committing to Git, deploying Git to a specific environment), while technically somewhat cleaner, generated endless frustration, as measured by support tickets.

And given that there is no other canonical way of building a Drupal deploy artifact, I see no alternative but to handle this in ACLI.

I agree transparency is important, we don't want to surprise anyone. But given how hard it's been to build support for the two-repo "source" and "build" system, I don't think trying to shove it in people's faces will be very productive. Much easier to build it in a way that "just works" for 95% of people without having to think about it.

[danepowell]

Alright, this is good to go if y'all approve.

Based on your own understanding of blt deploy

This is a pretty major improvement over blt deploy in several ways. It's a lot simpler at every step, and smarter when it comes to including vendor/scaffold files in the artifact, no longer requiring giant hardcoded lists of files to add, exclude, or ignore. It sacrifices a little flexibility in the sanitization step but we can deal with that later if it's really in demand.

[grasmash]

We should use IO

[danepowell]

I can change it. But... between $this->logger, $this->io, $output, and $output_callback, all of which largely overlap, I am completely lost as to what to use when.

Since you seem to have strong opinions on this, would you mind leaving an explanation here or in our style guide so we can get it right the first time next time?

[grasmash]

Use IO

[danepowell]

I don't see how to use IO here. When I tried, it wasn't interpreting <options=bold></>. In other areas of the code where we have a warning and font styles, we use $this->logger->warning().

[grasmash]

This is another new system dependency

[danepowell]

Au contraire, we use Composer in pull and new. As with Git, I agree we'd ideally check for it, but we can't do that before fixing commandExists() (it's super common for composer to be an alias, in fact Acquia recommends it as a workaround for Composer 1/2 issues)

[grasmash]

You may want to include the branch information here too

[grasmash]

The repository in the current working directory will be left in a strange state after this command executes. There will be a new upstream commit resulting from this command, but the local repository won’t have it. Perhaps we should pull it down? Otherwise, we could at least inform the user.

[danepowell]

I don't understand. Are you confusing the source and artifact repos? These commands all execute in a dedicated artifact directory (/tmp/acli-build-artifact) that the user will never see. We never modify the source repo (i.e. the user's working directory).

[grasmash]

This is a possibly strange assumption. Why do we care about what is checked out on the environment? Are we pushing to remote or are we pushing to an environment? If we are pushing to a remote and it doesn’t matter. If we are pushing to an environment, why can’t we check out a different branch or tag as part of that push?

[grasmash]

we probably should check out the pushed branch on the remote environment. I’m fine with preventing that for environments that have tags, but it only makes sense if we are otherwise going to be potentially deploying a new branch.

[danepowell]

I want the user to think of deploys as being to an environment, not to a particular branch or tag. This was the biggest point of confusion with blt deploy, people didn't understand that they had to deploy to git first, and then deploy git to an environment.

This is easy to accomplish technically if an environment is running a branch, we just push to the branch. It's less clear what we should do if an environment runs a tag. I'm kicking that decision down the road in the spirit of getting this out as an MVP.

[grasmash]

I think there’s a scenario in which this is problematic. If there is a new upstream change to the repository which has not yet been pulled locally, and then a user runs this command, it will effectively wipe out those changes. What is the desired behavior in that case?

it would be nice to check whether the local repository is up-to-date before initiating this build.

[danepowell]

Per discussion in Slack, this is okay to merge with the follow-up tickets we've created:

• [push:artifact] ensure Git and Composer exist #530
• Git and Composer helpers #525

I wish we could mark a command as "beta" or something. I think it's really important to get this out to start getting feedback, but I agree that it's a little more complex and likely to cause confusion if we don't get it right.