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

Fix for empty namespaces. #13459 #13460

Merged
merged 2 commits into from
Sep 13, 2019
Merged

Fix for empty namespaces. #13459 #13460

merged 2 commits into from
Sep 13, 2019

Conversation

johncook
Copy link
Contributor

@johncook johncook commented Sep 13, 2019

Fixes #13459

Changes proposed in this Pull Request:

If the namespace is an empty string then NULL is passed into ClassMapGenerator::createMap().

Testing instructions:

Create a composer.json file with the following content:

{
    "description": "Tests automattic/jetpack-autoloader",
    "keywords": [
        "automatic jetpack autoloader"
    ],
    "repositories": [
        {
            "type": "path",
            "url": "/path/to/your/jetpack/packages/autoloader",
            "options": {
                "symlink": true
            }
        }
    ],
    "require": {
        "automattic/jetpack-autoloader": "@dev",
        "wp-cli/wp-cli": "^1.0"
    }
}

Then run composer install.

Once you see the bug, rm -rf vendor composer.lock, apply the patch above, and then run composer install again. The error should disappear.

Proposed changelog entry for your changes:

  • General: avoid conflicts when using Jetpack alongside other plugins or services that rely on an Autoloader.

This is a patch to address issue Automattic#13459 

If the namespace is an empty string then NULL is passed into `ClassMapGenerator::createMap()`.
@johncook johncook requested a review from a team September 13, 2019 11:38
@jetpackbot
Copy link

jetpackbot commented Sep 13, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: October 1, 2019.
Scheduled code freeze: September 24, 2019

Generated by 🚫 dangerJS against 67d2177

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Type] Bug When a feature is broken and / or not performing as intended [Focus] Jetpack DNA labels Sep 13, 2019
@jeherve jeherve added this to the 7.8 milestone Sep 13, 2019
@jeherve
Copy link
Member

jeherve commented Sep 13, 2019

Could you provide testing instructions so we can try to reproduce the issue and check that the patch fixes the problem?

Thank you!

@johncook
Copy link
Contributor Author

@jeherve

The easiest way to test is to create a composer.json file with the following content:

{
  "description": "Tests automattic/jetpack-autoloader",
  "keywords": ["automatic jetpack autoloader"],
  "repositories": [
    {
      "type": "composer",
      "url": "https://wpackagist.org"
    }
  ],
  "require": {
    "automattic/jetpack-autoloader": "^1.2",
    "wp-cli/wp-cli": "^1.0"
  }
}

Then running composer install or composer update.

@johncook johncook removed their assignment Sep 13, 2019
@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 13, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works well in my tests.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 13, 2019
Copy link
Contributor

@gravityrail gravityrail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeherve jeherve merged commit 3d69319 into Automattic:master Sep 13, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 13, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
kraftbj pushed a commit that referenced this pull request Dec 9, 2019
* Fix for empty namespaces. #13459 

This is a patch to address issue #13459 

If the namespace is an empty string then NULL is passed into `ClassMapGenerator::createMap()`.

* Coding standards fixes


Co-authored-by: Jeremy Herve <jeremy@tagada.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Jetpack DNA [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: "strpos(): Empty needle" in Autoloader
5 participants