fix(pip_setup): Run pip setup extract in the correct directory #8320

hisener · 2021-01-16T07:36:30Z

Changes:

Changes pip setup extract script's working directory to "dirname" of setup.py.

Context:

Fixes #8319.

Documentation (please check one with an [x])

I have updated the documentation, or
No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

Code inspection only, or
Newly added unit tests, or
Unit tests + ran on a real repository

I run it against hisener/renovate-tests, see hisener/renovate-tests#32. The hosted app closes the PR because it fails to extract (See the issue for the error message).

viceice

Isn't it easier to simply change cwd and leave the rest as is?

hisener · 2021-01-16T09:08:50Z

Isn't it easier to simply change cwd and leave the rest as is?

Then, we need to use the absolute path of the extract.py. Is config.localDir always an absolute path?

I've just tried but it didn't create a PR. The script seems to run though. 🤔

DEBUG: Executing command (repository=hisener/renovate-tests)
       "command": "docker run --rm --name=renovate_pip --label=renovate_child -v \"/tmp/renovate/repos/github/hisener/renovate-tests\":\"/tmp/renovate/repos/github/hisener/renovate-tests\" -v \"/tmp/renovate/cache\":\"/tmp/renovate/cache\" -w \"/tmp/renovate/repos/github/hisener/renovate-tests/pip-setup-test\" docker.io/renovate/pip bash -l -c \"python \\\"/tmp/renovate/repos/github/hisener/renovate-tests/renovate-pip_setup-extract.py\\\" \\\"setup.py\\\"\""
DEBUG: exec completed (repository=hisener/renovate-tests)
       "cmd": "docker run --rm --name=renovate_pip --label=renovate_child -v \"/tmp/renovate/repos/github/hisener/renovate-tests\":\"/tmp/renovate/repos/github/hisener/renovate-tests\" -v \"/tmp/renovate/cache\":\"/tmp/renovate/cache\" -w \"/tmp/renovate/repos/github/hisener/renovate-tests/pip-setup-test\" docker.io/renovate/pip bash -l -c \"python \\\"/tmp/renovate/repos/github/hisener/renovate-tests/renovate-pip_setup-extract.py\\\" \\\"setup.py\\\"\"",
       "durationMs": 1062,
       "stdout": "",
       "stderr": ""
DEBUG: Found pipenv package files (repository=hisener/renovate-tests)
DEBUG: Found 1 package file(s) (repository=hisener/renovate-tests)

hisener · 2021-01-16T09:22:47Z

It's because it generates renovate-pip_setup-report.json in the current working directory so I need to either change extract.py or keep passing directory to parseReport function. 🤔 I think I prefer the current approach since there might be multiple directories with setup.py. WDYT?

viceice · 2021-01-16T10:41:20Z

🤔 I think we should move to cache aproach like in #8083 so we write our python file to the cache folder and the extracted json to the repo cache folder (which will be deleted after repo processing). The json files can have the same relative paths for easier referencing.

hisener · 2021-01-16T13:05:14Z

The extracted json is already in the repo cache. Added a commit to move extract.py to the cache folder, PTAL.

hisener · 2021-01-16T13:09:09Z

data/extract.py

-  # Inserting the parent directory of the target setup.py in Python import path:
-  sys.path.append(dirname(realpath(sys.argv[-1])))


It's not necessary because we are already running the script in the setup.py's directory.

hisener · 2021-01-16T13:09:50Z

data/extract.py

  # This is setup.py which calls setuptools.setup
-  load_source('_target_setup_', sys.argv[-1])
+  load_source('_target_setup_', basename(sys.argv[-1]))


We could also do this in extract.ts#49.

hisener · 2021-01-16T13:11:49Z

lib/manager/pip_setup/index.spec.ts

@@ -105,7 +108,7 @@ describe(getName(__filename), () => {
      expect(
        await extractPackageFile(
          'raise Exception()',
-          '/tmp/folders/foobar.py',
+          'folders/foobar.py',


To prevent confusion; otherwise cwd would be /tmp/github/some/repo/tmp/folders.

hisener · 2021-01-16T13:12:53Z

lib/manager/pip_setup/util.ts

+export async function parseReport(packageFile: string): Promise<PythonSetup> {
+  const data = await readLocalFile(join(dirname(packageFile), REPORT), 'utf8');


Here we assume the report json file is next to the package file.

rarkins · 2021-01-18T08:22:03Z

@hisener can you also test it against a real repo with setup.py in the root directory?

hisener · 2021-01-18T12:32:08Z

Rebased and tested after adding a setup.py to the root directory.

Renovate Ignore - Update dependency pytest to v6 hisener/renovate-tests#36 (root directory)
Renovate Ignore - Update dependency bugsnag to v4 hisener/renovate-tests#39 (sub-directory)

renovate-release · 2021-01-18T14:52:51Z

🎉 This PR is included in version 24.22.3 🎉

The release is available on:

GitHub release
24.22.3

Your semantic-release bot 📦🚀

viceice reviewed Jan 16, 2021

View reviewed changes

hisener commented Jan 16, 2021

View reviewed changes

hisener requested a review from viceice January 16, 2021 13:15

hisener added 3 commits January 18, 2021 13:20

fix(pip_setup): Run pip setup extract in the correct directory

4b701b1

PR comments

858269a

Polish

d89a709

hisener force-pushed the hsener/gh-8319 branch from f09d802 to d89a709 Compare January 18, 2021 12:32

rarkins merged commit 6da2f1d into renovatebot:master Jan 18, 2021

hisener deleted the hsener/gh-8319 branch January 18, 2021 13:18

This was referenced Jan 25, 2021

setup.py parser unable to import files from its own package #8415

Closed

fix(pip_setup): Allow setup.py to import from its own package #8417

Merged

github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(pip_setup): Run pip setup extract in the correct directory #8320

fix(pip_setup): Run pip setup extract in the correct directory #8320

hisener commented Jan 16, 2021 •

edited

viceice left a comment

hisener commented Jan 16, 2021

hisener commented Jan 16, 2021

viceice commented Jan 16, 2021

hisener commented Jan 16, 2021

hisener Jan 16, 2021 •

edited

hisener Jan 16, 2021

hisener Jan 16, 2021

hisener Jan 16, 2021

rarkins commented Jan 18, 2021

hisener commented Jan 18, 2021

renovate-release commented Jan 18, 2021

		# Inserting the parent directory of the target setup.py in Python import path:
		sys.path.append(dirname(realpath(sys.argv[-1])))

		export async function parseReport(packageFile: string): Promise<PythonSetup> {
		const data = await readLocalFile(join(dirname(packageFile), REPORT), 'utf8');

fix(pip_setup): Run pip setup extract in the correct directory #8320

fix(pip_setup): Run pip setup extract in the correct directory #8320

Conversation

hisener commented Jan 16, 2021 • edited

Changes:

Context:

Documentation (please check one with an [x])

How I've tested my work (please tick one)

viceice left a comment

Choose a reason for hiding this comment

hisener commented Jan 16, 2021

hisener commented Jan 16, 2021

viceice commented Jan 16, 2021

hisener commented Jan 16, 2021

hisener Jan 16, 2021 • edited

Choose a reason for hiding this comment

hisener Jan 16, 2021

Choose a reason for hiding this comment

hisener Jan 16, 2021

Choose a reason for hiding this comment

hisener Jan 16, 2021

Choose a reason for hiding this comment

rarkins commented Jan 18, 2021

hisener commented Jan 18, 2021

renovate-release commented Jan 18, 2021

hisener commented Jan 16, 2021 •

edited

hisener Jan 16, 2021 •

edited