CommentFileSizeAuthor
#2 composer-2770355-2.patch414 bytesShreya Shetty
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Shreya Shetty created an issue. See original summary.

Shreya Shetty’s picture

FileSize
414 bytes
Shreya Shetty’s picture

Status: Active » Needs review
mikeker’s picture

Status: Needs review » Postponed (maintainer needs more info)

@Shreya Shetty, thank you for the patch. Can you explain the need of a composer.json file if there are no dependencies to include?

And my apologies for the delay getting to this issue.

dawehner’s picture

IMHO this patch would just be needed if bef would have some sort of dependency on another PHP library, or wouldn't be hosted on d.o. I think both conditions are wrong.

mikeker’s picture

Status: Postponed (maintainer needs more info) » Closed (won't fix)

Since BEF currently has no dependencies and the d.o packaging script builds a default composer.json file based on what's in your module's info.yml file, I'm going to close this as won't fix. Otherwise, we have to maintain things like the module description in two places instead of one.

If someone can show me a use case where a composer.json is needed, please feel free to reopen this issue.

Lukas von Blarer’s picture

Status: Closed (won't fix) » Active

Ok, maybe I don't understand the Composer workflow yet, but if I create a fork of a module, it will need a composer.json file since Github or any other Git hosting will not build a composer.json. So wouldn't it make sense if every module provided a composer.json by default?

mikeker’s picture

@Lukas von Blarer, wouldn't it be the responsibility of the fork to add the composer.json file if they're going to change hosting locations?

Honestly I don't really care one way or the other -- it's a pretty small addition after all -- I'm just concerned about adding things that leave me with multiple places to update information such as the project description.

I also saw a thread that raised the concern about too many composer.json files impacting Composer's performance, but I can no longer find it...

Lukas von Blarer’s picture

I don't know about best practices how to handle this. I understand your concern about updating stuff in multiple places. Drupal core has a soft dependency on Composer, but it is needed in most cases. Does anyone know more about this?

When it comes to forking, it would be quite inconvenient having to add composer.json ass well since this would mess up pull requests. I've encountered a lot of modules without Composer dependencies that have one.

mansspams’s picture

Priority: Normal » Major

If you do not add this file, we get "The requested package drupal/better_exposed_filters could not be found in any version, there may be a typo in the package name." when following https://www.drupal.org/docs/develop/using-composer/using-composer-to-man....

dawehner’s picture

@mansspams
Are you sure you have the drupal.org bit in your composer.json file?

{ 
    "repositories": { 
        "drupal": {
            "type": "composer",
            "url": "https://packages.drupal.org/8" 
        }
    }
} 
mikeker’s picture

Priority: Major » Normal

I am able to run composer require drupal/better_exposed_filters and have it download the latest release of the module. The Drupal composer endpoints will create the composer.json if one doesn't exist (just pulls info from the module's .info.yml file).

I'm guessing that either you were missing the Drupal composer endpoint as @dawehner mentioned in #11, or you have your min stability set too high (BEF only has an alpha release on D8).

$ composer require drupal/better_exposed_filters
Using version ^3.0@alpha for drupal/better_exposed_filters
./composer.json has been updated
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
  - Installing drupal/better_exposed_filters (3.0.0-alpha2): Downloading (100%)
Writing lock file
Generating autoload files
> DrupalProject\composer\ScriptHandler::createRequiredFiles
allella’s picture

@mikeker Interesting bit about the https://packages.drupal.org/8 endpoint generating a composer.json from a module's .info.yml information. However, there may be another reason to consider adding a composer.json.

Both drupal/drupal and drupal-composer/drupal-project require composer-installers, which can read a "type:" from each module's composer.json to allow for easily defining custom install paths.

I explained this in more detail to the entityqueue maintainer and he agreed and decided to add a composer.json.

allella’s picture

It turns out the packages.drupal.org Composer endpoint will read .info.yaml files and create a composer.json with a type: drupal-module when contrib modules don't provide a composer.json.

This means it's not necessary to have a composer.json unless the module has 3rd party dependencies. However, there's no harm in having one since Composer is likely to be a bigger part of Drupal and PHP going forward. Also, this is dependent on an endpoint being smart enough to generate a file, which may not be a great long term expectation if endpoints change.

jeremyr’s picture

Status: Active » Closed (outdated)

Because of the age of this issue and really no requirement to have a composer.json in the module I'm setting this to wont fix. Please reopen if anyone feels otherwise.