Skip to content

fix: Add legacy pipenv backward compatability #742

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .github/workflows/integrate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
strategy:
matrix:
sls-version: [2, 3]
pipenv-version: ['2022.8.5', '2022.8.13']
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down Expand Up @@ -48,7 +49,7 @@ jobs:
run: python -m pip install --force setuptools wheel

- name: Install pipenv / poetry
run: python -m pip install pipenv poetry
run: python -m pip install pipenv==${{ matrix.pipenv-version }} poetry

- name: Install serverless
run: npm install -g serverless@${{ matrix.sls-version }}
Expand All @@ -67,6 +68,7 @@ jobs:
strategy:
matrix:
sls-version: [2, 3]
pipenv-version: ['2022.8.5', '2022.8.13']
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down Expand Up @@ -99,7 +101,7 @@ jobs:
run: python -m pip install --force setuptools wheel

- name: Install pipenv / poetry
run: python -m pip install pipenv poetry
run: python -m pip install pipenv==${{ matrix.pipenv-version }} poetry

- name: Install serverless
run: npm install -g serverless@${{ matrix.sls-version }}
Expand Down Expand Up @@ -147,7 +149,7 @@ jobs:
run: python -m pip install --force setuptools wheel

- name: Install pipenv / poetry
run: python -m pip install pipenv poetry
run: python -m pip install pipenv==${{ matrix.pipenv-version }} poetry

- name: Install serverless
run: npm install -g serverless@${{ matrix.sls-version }}
Expand All @@ -166,6 +168,7 @@ jobs:
strategy:
matrix:
sls-version: [2, 3]
pipenv-version: ['2022.8.5', '2022.8.13']
needs: [windowsNode14, linuxNode14, linuxNode12]
steps:
- name: Checkout repository
Expand Down
9 changes: 6 additions & 3 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
strategy:
matrix:
sls-version: [2, 3]
pipenv-version: ['2022.8.5', '2022.8.13']
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down Expand Up @@ -61,7 +62,7 @@ jobs:
run: python -m pip install --force setuptools wheel

- name: Install pipenv / poetry
run: python -m pip install pipenv poetry
run: python -m pip install pipenv==${{ matrix.pipenv-version }} poetry

- name: Install serverless
run: npm install -g serverless@${{ matrix.sls-version }}
Expand Down Expand Up @@ -94,6 +95,7 @@ jobs:
strategy:
matrix:
sls-version: [2, 3]
pipenv-version: ['2022.8.5', '2022.8.13']
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down Expand Up @@ -128,7 +130,7 @@ jobs:
run: python -m pip install --force setuptools wheel

- name: Install pipenv / poetry
run: python -m pip install pipenv poetry
run: python -m pip install pipenv==${{ matrix.pipenv-version }} poetry

- name: Install serverless
run: npm install -g serverless@${{ matrix.sls-version }}
Expand All @@ -147,6 +149,7 @@ jobs:
strategy:
matrix:
sls-version: [2, 3]
pipenv-version: ['2022.8.5', '2022.8.13']
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down Expand Up @@ -181,7 +184,7 @@ jobs:
run: python -m pip install --force setuptools wheel

- name: Install pipenv / poetry
run: python -m pip install pipenv poetry
run: python -m pip install pipenv==${{ matrix.pipenv-version }} poetry

- name: Install serverless
run: npm install -g serverless@${{ matrix.sls-version }}
Expand Down
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ custom:

Requires `pipenv` in version `2022-04-08` or higher.

If you include a `Pipfile` and have `pipenv` installed instead of a `requirements.txt` this will use
`pipenv lock -r` to generate them. It is fully compatible with all options such as `zip` and
If you include a `Pipfile` and have `pipenv` installed, this will use `pipenv` to generate requirements instead of a `requirements.txt`. It is fully compatible with all options such as `zip` and
`dockerizePip`. If you don't want this plugin to generate it for you, set the following option:

```yaml
Expand Down
94 changes: 74 additions & 20 deletions lib/pipenv.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,43 @@ const fse = require('fs-extra');
const path = require('path');
const spawn = require('child-process-ext/spawn');
const { EOL } = require('os');
const semver = require('semver');

const LEGACY_PIPENV_VERSION = '2022.8.5';

async function getPipenvVersion() {
try {
const res = await spawn('pipenv', ['--version'], {
cwd: this.servicePath,
});

const stdoutBuffer =
(res.stdoutBuffer && res.stdoutBuffer.toString().trim()) || '';

const version = stdoutBuffer.split(' ')[2];

if (semver.valid(version)) {
return version;
} else {
throw new this.serverless.classes.Error(
`Unable to parse pipenv version!`,
'PYTHON_REQUIREMENTS_PIPENV_VERSION_ERROR'
);
}
} catch (e) {
const stderrBufferContent =
(e.stderrBuffer && e.stderrBuffer.toString()) || '';

if (stderrBufferContent.includes('command not found')) {
throw new this.serverless.classes.Error(
`pipenv not found! Install it according to the pipenv docs.`,
'PYTHON_REQUIREMENTS_PIPENV_NOT_FOUND'
);
} else {
throw e;
}
}
}

/**
* pipenv install
Expand All @@ -28,31 +65,48 @@ async function pipfileToRequirements() {
}

try {
try {
await spawn('pipenv', ['lock', '--keep-outdated'], {
cwd: this.servicePath,
});
} catch (e) {
const stderrBufferContent =
(e.stderrBuffer && e.stderrBuffer.toString()) || '';
// Get and validate pipenv version
if (this.log) {
this.log.info('Getting pipenv version');
} else {
this.serverless.cli.log('Getting pipenv version');
}

const pipenvVersion = await getPipenvVersion();
let res;

if (stderrBufferContent.includes('must exist to use')) {
// No previous Pipfile.lock, we will try to generate it here
await spawn('pipenv', ['lock'], {
if (semver.gt(pipenvVersion, LEGACY_PIPENV_VERSION)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that actually going to work? It doesn't seem like pipenv is using proper semver versioning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, see the additional passing test runner matrix values running both pipenv versions: https://github.com/serverless/serverless-python-requirements/pull/742/files#diff-ed51950595c5b713cdfd2437e37b2ff83a366772307810e1cf5d2226f096d082R19

While the pipenv versions don't seem like traditional semver versions, they appear at least semver compliant, e.g. 2022.8.5 < 2022.8.13.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for clarification 🙇

// Using new pipenv syntax ( >= 2022.8.13)
try {
await spawn('pipenv', ['lock', '--keep-outdated'], {
cwd: this.servicePath,
});
} else if (stderrBufferContent.includes('command not found')) {
throw new this.serverless.classes.Error(
`pipenv not found! Install it according to the poetry docs.`,
'PYTHON_REQUIREMENTS_PIPENV_NOT_FOUND'
);
} else {
throw e;
} catch (e) {
const stderrBufferContent =
(e.stderrBuffer && e.stderrBuffer.toString()) || '';
if (stderrBufferContent.includes('must exist to use')) {
// No previous Pipfile.lock, we will try to generate it here
await spawn('pipenv', ['lock'], {
cwd: this.servicePath,
});
} else {
throw e;
}
}

res = await spawn('pipenv', ['requirements'], {
cwd: this.servicePath,
});
} else {
// Falling back to legacy pipenv syntax
res = await spawn(
'pipenv',
['lock', '--requirements', '--keep-outdated'],
{
cwd: this.servicePath,
}
);
}
const res = await spawn('pipenv', ['requirements'], {
cwd: this.servicePath,
});

fse.ensureDirSync(path.join(this.servicePath, '.serverless'));
fse.writeFileSync(
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"lodash.uniqby": "^4.7.0",
"lodash.values": "^4.3.0",
"rimraf": "^3.0.2",
"semver": "^7.3.8",
"set-value": "^4.1.0",
"sha256-file": "1.0.0",
"shell-quote": "^1.7.4"
Expand Down