Skip to content

Fix for #676 #677

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix for #676 #677

wants to merge 1 commit into from

Conversation

tuxmaster5000
Copy link
Contributor

Fix for #676

@tdstein
Copy link
Collaborator

tdstein commented Jun 2, 2025

Hi @tuxmaster5000, thanks for the pull request.

Can you say a bit more about what scenario you are fixing with this change? I do not see any errors in my local environment when running...

uv run --with rsconnect-python==1.26.0 python -m rsconnect.subprocesses.inspect_environment

@tdstein tdstein requested review from amol-, tdstein and edavidaja June 2, 2025 14:30
@tuxmaster5000
Copy link
Contributor Author

When I try to build it as an rpm package. It looks like Fedora 41 is using an modern version of setuptools, because on the EOL version 40 it was not happens. As I understand the documentation of setuptools, when using this option, than all modules must listed by hand. Or don't use this option.
https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html

Copy link
Contributor

@amol- amol- left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

It would probably make sense to switch to a tool.setuptools.packages.find directive so that packages don't have to be manually listed and it's a bit more future-proof.

Also, I suggest in the PR we change https://github.com/posit-dev/rsconnect-python/blob/main/.github/workflows/main.yml#L40 to install in non-editable mode (actually make a dist and then install the dist) so that we can confirm that the changes do actually fix the issue before committing them and that problems like this can be caught in the future.

@tuxmaster5000
Copy link
Contributor Author

I have played with find, but this will require an complete reorganisation of the directory structure.
See: https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#finding-simple-packages
The source code must be placed in an sub folder. But I don't know what then happens with the tests and the rest.

[tool.setuptools.packages.find]
where = [ "rsconnect" ]
will fails.

@amol-
Copy link
Contributor

amol- commented Jun 4, 2025

I have played with find, but this will require an complete reorganisation of the directory structure. See: https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#finding-simple-packages The source code must be placed in an sub folder. But I don't know what then happens with the tests and the rest.

[tool.setuptools.packages.find]
where = [ "rsconnect" ]
will fails.

[tool.setuptools.packages.find]
include = ["rsconnect"]
namespaces = false

should be what we want. If where is not specified, it defaults to . out of which we only want to include the rsconnect directory.

See https://github.com/pypa/setuptools/blob/c3f486f0f7ebf8fa141dfd7314cbcaba7370db0b/setuptools/discovery.py#L109-L110

@tuxmaster5000
Copy link
Contributor Author

I have try it, but this will break the build process. But I don't know why:

+ /usr/bin/python3 /usr/lib/rpm/redhat/pyproject_save_files.py --output-files /builddir/build/BUILD/python-rsconnect-1.26.0-build/python-rsconnect-1.26.0-2.fc41.x86_64-pyproject-files --output-modules /builddir/build/BUILD/python-rsconnect-1.26.0-build/python-rsconnect-1.26.0-2.fc41.x86_64-pyproject-modules --buildroot /builddir/build/BUILD/python-rsconnect-1.26.0-build/BUILDROOT --sitelib /usr/lib/python3.13/site-packages --sitearch /usr/lib64/python3.13/site-packages --python-version 3.13 --pyproject-record /builddir/build/BUILD/python-rsconnect-1.26.0-build/python-rsconnect-1.26.0-2.fc41.x86_64-pyproject-record --prefix /usr rsconnect
Traceback (most recent call last):
  File "/usr/lib/rpm/redhat/pyproject_save_files.py", line 905, in <module>
    main(cli_args)
    ~~~~^^^^^^^^^^
  File "/usr/lib/rpm/redhat/pyproject_save_files.py", line 846, in main
    file_section, module_names = pyproject_save_files_and_modules(
                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        cli_args.buildroot,
        ^^^^^^^^^^^^^^^^^^^
    ...<7 lines>...
        cli_args.varargs,
        ^^^^^^^^^^^^^^^^^
    )
    ^
  File "/usr/lib/rpm/redhat/pyproject_save_files.py", line 827, in pyproject_save_files_and_modules
    generate_file_list(paths_dict, globs, include_auto)
    ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/rpm/redhat/pyproject_save_files.py", line 625, in generate_file_list
    raise ValueError(f"Globs did not match any module: {missed_text}")
ValueError: Globs did not match any module: rsconnect
error: Bad exit status from /var/tmp/rpm-tmp.MJFrec (%install)
    Bad exit status from /var/tmp/rpm-tmp.MJFrec (%install)

@amol-
Copy link
Contributor

amol- commented Jun 5, 2025

Ah! when you say "build process" you mean the rpm build process, not the python distribution build process.

Building the wheel works as expected with find:

% python -m build
...
adding 'rsconnect/__init__.py'
adding 'rsconnect/actions.py'
adding 'rsconnect/actions_content.py'
...
adding 'rsconnect/subprocesses/__init__.py'
adding 'rsconnect/subprocesses/inspect_environment.py'
...
adding 'rsconnect_python-1.26.1.dev1+g1108457.d20250605.dist-info/METADATA'
adding 'rsconnect_python-1.26.1.dev1+g1108457.d20250605.dist-info/RECORD'

rsconnect and rsconnect.subprocess are both included in the wheel.

I haven't been building RPMs in years, but that sounds like a bug in pyproject_save_files.py and it's out of the scope of rsconnect-python to address it.
If pyproject_save_files.py behaves differently than the official PyPa packages like build I think it probably needs to be aligned.

I have try it, but this will break the build process. But I don't know why:

+ /usr/bin/python3 /usr/lib/rpm/redhat/pyproject_save_files.py --output-files /builddir/build/BUILD/python-rsconnect-1.26.0-build/python-rsconnect-1.26.0-2.fc41.x86_64-pyproject-files --output-modules /builddir/build/BUILD/python-rsconnect-1.26.0-build/python-rsconnect-1.26.0-2.fc41.x86_64-pyproject-modules --buildroot /builddir/build/BUILD/python-rsconnect-1.26.0-build/BUILDROOT --sitelib /usr/lib/python3.13/site-packages --sitearch /usr/lib64/python3.13/site-packages --python-version 3.13 --pyproject-record /builddir/build/BUILD/python-rsconnect-1.26.0-build/python-rsconnect-1.26.0-2.fc41.x86_64-pyproject-record --prefix /usr rsconnect

@amol-
Copy link
Contributor

amol- commented Jun 5, 2025

I tried to verify things with this test script: https://gist.github.com/amol-/b1ce860fb645a731a8ff4588895041e5

It seems in both cases with

[tool.setuptools.packages.find]
include = ["rsconnect"]
namespaces = false

or

[tool.setuptools]
packages = [
   "rsconnect",
   "rsconnect.subprocesses"
]

it works as expected, because pyproject_preprocess_record.py parses the wheel .dist-info and thus finds all modules that the wheel did include.

The output of the script for me is

RESULT
rsconnect
rsconnect.actions
...
rsconnect.subprocesses
rsconnect.subprocesses.inspect_environment
...
rsconnect.version

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.

3 participants