At some point in time in the past composer make "--dev" default option for install command.
This means that package that we creating for the Drupal core contains development components that should not be there, also this means that that package is bigger then it should be.

Proposed solution

Use composer install --no-dev to create package on Drupal.org.

Files: 
CommentFileSizeAuthor
#24 2745355-24.patch1.68 KBpfrenssen

Comments

RoSk0 created an issue. See original summary.

tstoeckler’s picture

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.

borisson_’s picture

If simpletest properly defines it's own dependencies, #2494073: Mark modules with unmet composer dependencies uninstallable should help with #2.

tstoeckler’s picture

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.

drumm’s picture

Project: Drupal.org infrastructure » Drupal core
Version: » 8.2.x-dev
Component: Packaging » install system
Status: Active » Needs review

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...

drumm’s picture

#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.

dawehner’s picture

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

Mile23’s picture

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: Mark modules with unmet composer dependencies uninstallable disallows simpletest from being installed without a composer install --dev.

Mile23’s picture

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

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pfrenssen’s picture

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.

klausi’s picture

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

Mile23’s picture

#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

pfrenssen’s picture

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?

Mile23’s picture

Status: Needs review » Reviewed & tested by the community

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.

pfrenssen’s picture

FileSize
1.34 KB

Patchy patchy :)

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

cilefen’s picture

Issue tags: +8.3.0 release notes
catch’s picture

Status: Reviewed & tested by the community » Needs review

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

drumm’s picture

That’s certainly doable.

catch’s picture

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.

pfrenssen’s picture

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.

RoSk0’s picture

Great idea! Another +1 from me.

drumm’s picture

Title: Use "composer install --no-dev" to create core package » Use "composer install --no-dev" to create tagged core packages
Project: Drupal core » Drupal.org customizations
Version: 8.3.x-dev » 7.x-3.x-dev
Component: install system » Code
Status: Needs review » Needs work

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'

pfrenssen’s picture

Status: Needs work » Needs review
FileSize
1.68 KB

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

  • drumm committed 3f80c24 on 7.x-3.x, dev authored by pfrenssen
    Issue #2745355 by pfrenssen: Use "composer install --no-dev" to create...
drumm’s picture

Status: Needs review » Fixed

Looks good. Committed & deploying.

Mile23’s picture

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.

drumm’s picture

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.

cilefen’s picture

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".

drumm’s picture

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

Mile23’s picture

Cool, thanks @drumm.

xjm’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

cilefen’s picture