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

registry path for scoped package tarball should include scope subdirectory under dash directory #127

Merged
merged 1 commit into from
Jun 18, 2017

Conversation

vobarian
Copy link
Contributor

The JSON sent by npm publish contains the property versions.XXX.dist.tarball which contains the scope name twice for a scoped package; for example: "http://localhost:15443/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz". The mock registry server attempts to remove the second occurrence of @scope from the file path, which causes an error if users try to create another package into which they install the package created in the workshopper using the mock server. The path should be left as-is to match the tarball property in body.json so npm can GET it. Removing this code has the additional benefit of fixing a bug on Windows because the regex does not consider backslashes in the path.

Fixes: #122
Fixes: nodeschool/discussions#1561
Fixes: nodeschool/discussions#2049

Please note this would supersede PR #68

Additional Explanation

Given a package @chad/myhowtonpm, running npm publish with npm 5.0.3 will PUT JSON something like this:

{
    "_id": "@chad/myhowtonpm",
    "name": "@chad/myhowtonpm",
    "description": "This is my package.",
    "dist-tags": {
        "latest": "1.0.0"
    },
    "versions": {
        "1.0.0": {
            "name": "@chad/myhowtonpm",
            "version": "1.0.0",
            "main": "index.js",
            "scripts": {
                "test": "node test.js"
            },
            "author": "",
            "license": "ISC",
            "description": "This is my package.",
            "dependencies": {
                "@linclark/pkg": "^1.0.2"
            },
            "repository": {
                "type": "git",
                "url": "http://www.vobarian.com/"
            },
            "readme": "This is my package. \r\n",
            "readmeFilename": "README.md",
            "_id": "@chad/myhowtonpm@1.0.0",
            "_npmVersion": "5.0.3",
            "_nodeVersion": "8.0.0",
            "_npmUser": {
                "name": "chad",
                "email": "test@example.com"
            },
            "maintainers": [
                {
                    "name": "chad",
                    "email": "test@example.com"
                }
            ],
            "dist": {
                "integrity": "sha512-8H6LDqLEk9DR3DENJh11GilaxWPwheCuxKXCiuTDESMZ4ilduTPDa4kFD2sP0iL2r7refGg0FQQupa58et0CEQ==",
                "shasum": "fc9a1b352764777a8333ef77ad2f7cc72c989c9e",
                "tarball": "http://localhost:15443/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz"
            }
        }
    },
    "readme": "This is my package. \r\n",
    "maintainers": [
        {
            "name": "chad",
            "email": "test@example.com"
        }
    ],
    "_attachments": {
        "@chad/myhowtonpm-1.0.0.tgz": {
            "content_type": "application/octet-stream",
            "data": "...",
            "length": 668
        }
    }
}

Note that the tarball property contains two subdirectories following the pattern @scope. In order for a user to be able to npm install the package the file has to be found at this location. Hence the current code which attempts to strip the second occurrence of @scope is not correct; in fact I have verified that attempting to install the package into a second package causes an error because of this.

With the new code, the variables trace out as follows:

dir = "registry/@chad/myhowtonpm"
tgzBase = "@chad/myhowtonpm-1.0.0.tgz"
tgzFile = "registry/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz"
path.dirname(tgzFile) = "registry/@chad/myhowtonpm/-/@chad"

The path tgzFile matches the tarball property and the correct directory is created in the mkdirp call. It is possible to install the package created in the workshopper into a second package later on (assuming the user copies .npmrc to use the mock server). Also, since the regex is no longer needed, the bug that breaks publishing on Windows is simply gone.

…ctory under dash directory

The JSON sent by `npm publish` contains the property `versions.XXX.dist.tarball` which contains the scope name twice for a scoped package; for example: "http://localhost:15443/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz". The mock registry server attempts to remove the second occurrence of `@scope` from the file path, which causes an error if users try to create another package into which they install the package created in the workshopper using the mock server. The path should be left as-is to match the tarball property in `body.json` so npm can GET it. Removing this code has the additional benefit of fixing a bug on Windows because the regex does not consider backslashes in the path.

See: workshopper#68
Fixes: workshopper#122
Fixes: nodeschool/discussions#1561
Fixes: nodeschool/discussions#2049
@watilde
Copy link
Member

watilde commented Jun 18, 2017

Thanks for the patch! As you said the patch including the scope name should be resolved by the path module. Will do test on my local and ship it.

@watilde watilde merged commit 7c08cb9 into workshopper:master Jun 18, 2017
@watilde
Copy link
Member

watilde commented Jun 18, 2017

This patch was released as how-to-npm@2.4.3 🎉

Thanks again for putting this together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants