Skip to content
This repository has been archived by the owner on Mar 18, 2021. It is now read-only.

Build failing on jenkins server #13

Closed
not-an-aardvark opened this issue Oct 26, 2017 · 14 comments
Closed

Build failing on jenkins server #13

not-an-aardvark opened this issue Oct 26, 2017 · 14 comments
Labels

Comments

@not-an-aardvark
Copy link
Member

Example: https://jenkins.eslint.org/job/eslint-canary/266/console

This seems to be a build-related issue and not a regression in ESLint. It might have something to do with the fact that I updated to Node 8 on the Jenkins server yesterday when I deployed eslint-github-bot.

@aladdin-add
Copy link
Member

I have met it before, when using dont-break (eslint/eslint#9508). maybe related to npm v5 which shipped with Node 8, it is very buggy! ╮(╯▽╰)╭

@platinumazure
Copy link
Member

Is this still an issue? Curious if either npm v5 has released some fixes or if there's an npm v6 that has avoided all these issues.

@platinumazure
Copy link
Member

@not-an-aardvark Is this still an issue? I'd like to look into setting up a nightly build for eslint-canary in the near future.

@not-an-aardvark
Copy link
Member Author

This was still an issue as of 21 hours ago. There has been a nightly build running on jenkins here. I'm not sure if your fixes from today have resolved the issue, but we should find out in 3 hours if it's still a problem.

@platinumazure
Copy link
Member

Oh, thanks, I missed that there was a nightly build running. I'll check out the results tomorrow.

@not-an-aardvark
Copy link
Member Author

I tried clicking the "build now" button, and it seems like the build is still broken: https://jenkins.eslint.org/job/eslint-canary/453/console

@platinumazure
Copy link
Member

Thanks. Really weird-- looks like the projects' dependencies aren't getting installed... I'm happy to try to dig in this weekend if nobody else gets a chance to look sooner.

(Also, seems I misspoke earlier-- looks like this job is cloning the latest ESLint and using that, which is awesome.)

@not-an-aardvark
Copy link
Member Author

I suspect the issue has something to do with npm versions. The travis build uses Node 6 and 7, both of which ship with npm 3 by default. The server runs npm 5, which changed some things about how dependencies are arranged.

@platinumazure
Copy link
Member

platinumazure commented May 5, 2018

I'm looking into this (in between other tasks, so others can feel free to jump in as well).

I've received the same results as Jenkins after upgrading locally to npm@6, so I agree it has something to do with recent npm/the dependency tree. I'm hoping to find out more and maybe write a fix at some point.

@platinumazure
Copy link
Member

I've taken a look. I think I found the area where the issue is, but unfortunately it's raised more questions than it has answered.

The logic that appears to be causing the problem is in lib/config/config-file.js:

function getBaseDir(configFilePath) {

    // calculates the path of the project including ESLint as dependency
    const projectPath = path.resolve(__dirname, "../../../");

    if (configFilePath && pathIsInside(configFilePath, projectPath)) {

        // be careful of https://github.com/substack/node-resolve/issues/78
        return path.join(path.resolve(configFilePath));
    }

    /*
     * default to ESLint project path since it's unlikely that plugins will be
     * in this directory
     */
    return path.join(projectPath);
}

This code basically checks whether the ESLint project path is within the configFilePath (in other words, is ESLint a dependency of the consuming project). If so, it will return the consuming project's configFilePath (and later append node_modules and then search that directory for shareable configs needed for the linting run). Otherwise, it will assume the directory won't have ESLint plugins/configs installed and will instead return the ESLint project path (i.e., assume the config file path should be resolved in ESLint itself). This results in the config or plugin not being found, since eslint-canary installed the packages in the downloaded project but ESLint itself is not being run from within the downloaded project.

It seems that we need to tell this function to trust the passed-in configFilePath somehow, without breaking the intended use cases. (Or maybe we need to make a change higher in the call stack which prevents this method from being called.)

That said, I have no idea how this logic didn't fail the same way when I ran with npm@3. All I can think of is if npm install actually installed the dependencies in the ESLint project itself, despite the CWD being inside the downloaded project. So I think there's more we need to figure out here.

(cc @not-an-aardvark in case any of this triggers a brainwave of some kind)

@not-an-aardvark
Copy link
Member Author

I'm not sure whether it's related to this case, but I think it was discovered in eslint/eslint#9897 that the logic in that function is pretty much completely broken. As you mentioned, the intent is to check whether the config file is inside the consuming project, but the code actually checks whether the config file is inside the consuming project's node_modules folder. This is very rarely the case, so the result is that dependencies are effectively always loaded from the ESLint package.

This doesn't answer the question of why eslint-canary's behavior is different with npm@3, but maybe it's useful for figuring out what's going on.

@platinumazure
Copy link
Member

platinumazure commented May 11, 2018

@not-an-aardvark I'll keep digging this weekend, thanks for that insight.

There were concerns about breaking changes in eslint/eslint#9897. I'm afraid to saturate the 5.0.0 project, but is this something we should take on? I'm only asking because otherwise we probably won't be able to fix this for 6-9 months.

(Edit: Of course this is really a question for the TSC, but I'm asking your thoughts personally on whether you think this could be feasible or if it would unacceptably delay 5.0.0.)

@not-an-aardvark
Copy link
Member Author

I'm currently of the opinion that we should try to fix all of the config/plugin-loading problems described in eslint/eslint#10125 in the near future (although not for 5.0). Increasing the complexity around how shareable configs are loaded beforehand might get in the way of creating that solution, so I was thinking it would be better to fix everything at the same time rather than doing partial fixes that only change config loading without changing plugin loading.

@not-an-aardvark
Copy link
Member Author

This was fixed by 4edcba1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants