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

adding provide to php composer.json #13141

Closed
wants to merge 1 commit into from

Conversation

brettmc
Copy link
Contributor

@brettmc brettmc commented Jun 25, 2023

Adding composer config to allow the native protobuf extension to provide ext-protobuf.

This allows libraries to require at least one protobuf implementation. If the extension is not available, it can be provided by the native package. If the extension is available but the native package is required, the native will be installed.

Importantly, for libraries which require at least one of them to be installed, composer will complain if neither is available.

Adding composer config to allow the native protobuf extension to provide ext-protobuf.
This allows libraries to require at least one protobuf implementation. If the extension
is not available, it can be provided by the native package. If the extension is available
but the native package is required, the native will be installed.
Importantly, for libraries which require at least one of them to be installed, composer
will complain if neither is available.
@brettmc brettmc requested a review from a team as a code owner June 25, 2023 01:56
@brettmc brettmc requested review from haberman and removed request for a team June 25, 2023 01:56
@brettmc
Copy link
Contributor Author

brettmc commented Jun 25, 2023

My testing:

{
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/brettmc/protobuf-php"
        }
    ],
    "require": {
        "ext-protobuf": "*"
    }
}

No extension:

$ php --ri protobuf
Extension 'protobuf' not present.

$ composer update
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires PHP extension ext-protobuf * but it is missing from your system. Install or enable PHP's protobuf extension.
 
### install forked branch with provide:
$ composer require google/protobuf dev-provides
./composer.json has been updated
Running composer update google/protobuf
Loading composer repositories with package information
Updating dependencies                                 
Lock file operations: 1 install, 0 updates, 0 removals
  - Locking google/protobuf (dev-provides 18e3096)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
  - Installing google/protobuf (dev-provides 18e3096): Extracting archive
1 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating autoload files
No security vulnerability advisories found

$ composer check-platform-reqs
Checking platform requirements for packages in the vendor dir
ext-protobuf *        success provided by google/protobuf 
php          8.0.27   success

Extension installed:

$ php --ri protobuf

protobuf

Version => 3.21.6

$ composer update
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Generating autoload files
No installed packages - skipping audit.

$ composer check-platform-reqs
No vendor dir present, checking platform requirements from the lock file
ext-protobuf 3.21.6   success

Extension installed, require native anyway:

$ php --ri protobuf

protobuf

Version => 3.21.6


$ composer require google/protobuf dev-provides
./composer.json has been updated
Running composer update google/protobuf
Loading composer repositories with package information
Updating dependencies                                 
Lock file operations: 1 install, 0 updates, 0 removals
  - Locking google/protobuf (dev-provides 18e3096)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
  - Installing google/protobuf (dev-provides 18e3096): Extracting archive
1 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating autoload files
No security vulnerability advisories found

$ composer check-platform-reqs
Checking platform requirements for packages in the vendor dir
ext-protobuf *        success provided by google/protobuf 
php          8.0.27   success

@mkruskal-google mkruskal-google added php 🅰️ safe for tests Mark a commit as safe to run presubmits over and removed 🅰️ safe for tests Mark a commit as safe to run presubmits over labels Jun 26, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 26, 2023
@fowles fowles added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 7, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 7, 2023
@copybara-service copybara-service bot closed this in d603b41 Jul 7, 2023
@brettmc
Copy link
Contributor Author

brettmc commented Jul 7, 2023

@fowles - I don't think running the tests was supposed to close the PR. I couldn't see in any of the jobs why that might have happened.

@fowles
Copy link
Contributor

fowles commented Jul 8, 2023

@brettmc marking it "safe for tests" caused GHA to run, when those were green, copybara brought it inside google where a changelist was created in piper, that was then code reviewed internally, and submitted. Copybara then merged that submission externally as d603b41. For reasons I do not understand, sometimes copybara can do this by merging a PR and sometimes it generates an identical commit in github but does not merge it. In this case, it did the latter.

You should still see your change in our git repo properly creditted to you.

@brettmc brettmc deleted the php-provides-ext branch October 19, 2023 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants