Use "composer install --no-dev" to create tagged core packages

It's not entirely clear (certainly to me, but I think generally) whether this would be correct. AFAIK we have put a bunch of things into require-dev that Simpletest module depends on, because testing is generally considered a "dev" operation not a "prod" operation. However, that means that in the tarballs generated with --no-dev Simpletest module would be unusable even though you can install it from the UI. I haven't verified that, but if it is the case, then I think we shouldn't do this as of yet.

It would be even better IMO to be able to detect whether the dependencies are available and if not mark Simpletest as uninstallable and hide it from the UI. Then I think this would be a great idea. This would require a core patch, though.

If simpletest properly defines it's own dependencies, #2494073: Prevent modules which have unmet Composer dependencies from being installed should help with #2.

Yes, thanks, I was thinking of that issue as well (but couldn't find it).

In this case I think it might actually make sense to hide the module alltogether instead marking it as uninstallable for UX purposes, but I guess we can discuss that when we get there.

I'd like core maintainers to decide this, then it can be moved to the drupal.org customizations project. Composer for packaging is run at http://cgit.drupalcode.org/drupalorg/tree/drupalorg_project/plugins/rele...

#2756187: Don't include dev dependencies in tarball has a patch for this.

It looks like this is a generally good idea. I'd like to know if there are any blockers to deployment, or followups that can be done later. Such as putting Simpletest in an okay place.

Didn't alexpott suggested that in #2756187: Don't include dev dependencies in tarball, which was duplicate as this issue?

This issue is a bit of a followup to #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core

The main standout issue here is that simpletest requires phpunit, so in core we need to either move phpunit to non-dev requires, or make sure #2494073: Prevent modules which have unmet Composer dependencies from being installed disallows simpletest from being installed without a composer install --dev.

We might also turn simpletest into a special case that checks whether its dependencies are present: #2780093: Have simpletest, run-tests.sh enforce their dependency on PHPUnit

Are we already using the --prefer-dist option when packaging core? That is also important, since it will exclude all test files of our dependencies. These test files might contain security issues and do not belong on production environments.

Yes, we already use --prefer-dist for the composer command, see also my patch at #2756187: Don't include dev dependencies in tarball.

#2780093: Have simpletest, run-tests.sh enforce their dependency on PHPUnit is in, as of 8.2.x.

But... If we're packaging all versions of D8 with code changed here, then we might end up packaging 8.1.x (or even 8.0.x) without phpunit, and a broken simpletest. I think we're still packaging 8.1.x, right? So here's an issue to fix it, which is a copy of the other one: #2804663: Have simpletest, run-tests.sh enforce their dependency on PHPUnit in 8.1.x

We no longer need to be concerned about 8.1.x being an issue. I think we can move forward with this.

This needs feedback from a core committer right?

Yup. #2804663: Have simpletest, run-tests.sh enforce their dependency on PHPUnit in 8.1.x is CWF, and 8.2.x is the current release.

If a maintainer could please evaluate this idea and set it back to the infrastructure project as per #5. Thanks.

Patchy patchy :)

/me is confident that core committers will approve this :)

Could we do this only for tagged releases, and include the dev dependencies in the dev tarballs?

That’s certainly doable.

That would still allow someone to run tests without cli access if they really needed to, and it solves the main point of this issue by making the actual releases a lot lighter. So big +1 if we do that.

Could we do this only for tagged releases, and include the dev dependencies in the dev tarballs?

That sounds like a good idea to me, but it will maybe come as a surprise to some projects in the future if they need to deploy a dev release for some reason. Anyway I think people will simply have to adapt their deploy processes. I think it is reasonable for a dev release to contain dev dependencies.

Moving back to drupalorg for implementation.

In this section of code, a tagged release is $this->release_node->field_release_build_type[LANGUAGE_NONE][0]['value'] === 'static'

Thanks @drumm for the hint! I updated the patch. Note that this is untested.

Looks good. Committed & deploying.

I just downloaded the 8.2.2 tarball and it has phpunit in it... I guess either it's not in production yet or something has to kick a rebuild of the tarball.

This will only affect future tagged releases.

We don't rebuild previously-packaged tagged releases. That would change the file hashes and confuse Drush and anything else keeping track. And potentially confuse people with two packages of Drupal running around with the same label.

Whew, I did not know we were jumping in and doing this for 8.2.x releases. I had tagged it "8.3.0 release notes".

If there is an 8.2.3, it will have --no-dev.

Cool, thanks @drumm.

#2830880: Warn site admins when composer dev dependencies are installed inside of docroot