Skip to content

Add PEP 517-compliant pip install workflow#4828

Open
gamesh411 wants to merge 3 commits intoEricsson:masterfrom
gamesh411:pip-install-modernization
Open

Add PEP 517-compliant pip install workflow#4828
gamesh411 wants to merge 3 commits intoEricsson:masterfrom
gamesh411:pip-install-modernization

Conversation

@gamesh411
Copy link
Copy Markdown
Contributor

Rewrite setup.py to discover packages from source tree instead of requiring pre-built build_dist/ directory. Enables pip install . and pip install -e . as alternative to Makefile build.

Remove the ldlogger C Extension that was in the original setup.py. It used setuptools Extension to build an .so file, but the runtime needs a logger binary as well (which the setuptools Extension did not produce). Build logging on linux still requires make package (which builds ldlogger correctly).

Makefile build, Thrift API packages, and frontend build unchanged.

Rewrite setup.py to discover packages from source tree instead of
requiring pre-built build_dist/ directory. Enables `pip install .`
and `pip install -e .` as alternative to Makefile build.

Remove the ldlogger C Extension that was in the original setup.py.
It used setuptools Extension to build an .so file, but the runtime
needs a logger binary as well (which the setuptools Extension did
not produce). Build logging on linux still requires `make package`
(which builds ldlogger correctly).

Makefile build, Thrift API packages, and frontend build unchanged.
Makefile build does this via extend_version_file.py but
rewritten setup.py missed it — rc builds would report
6.28.0 instead of 6.28.0-rc1.
Copy link
Copy Markdown
Collaborator

@gulyasgergely902 gulyasgergely902 left a comment

Choose a reason for hiding this comment

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

LGTM apart from a few minor things.

tu_collector --help

- name: Verify data files installed
run: |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd move this part to a separate file to make it more readable and invoke it here. Also given the simplicity of this task, couldn't it be done with bash instead of python?


- name: Test analyze and parse
run: |
WORK="$RUNNER_TEMP/analyze-test"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ideally I'd imagine this in a separate file too because of the length of the script. Also if JQ is available, counting reports could be done with it: CodeChecker parse "$WORK/reports" -e json | jq ".reports | length".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants