Skip to content

fix: call make_file_executable with a Path#3764

Merged
frostming merged 1 commit intopdm-project:mainfrom
maxyz:fix_make_file_executable_with_str
Apr 10, 2026
Merged

fix: call make_file_executable with a Path#3764
frostming merged 1 commit intopdm-project:mainfrom
maxyz:fix_make_file_executable_with_str

Conversation

@maxyz
Copy link
Copy Markdown
Contributor

@maxyz maxyz commented Apr 10, 2026

The make_file_executable function provided by pypa/installer only works for Path objects. as it uses it's chmod method directly.

Both uses in pdm are passing an str, thus hitting this code produces tracebacks similar to:

Traceback (most recent call last):
  File "/usr/lib/python3.13/concurrent/futures/thread.py", line 59, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3/dist-packages/pdm/installers/synchronizers.py", line 50, in update_candidate
    self.manager.overwrite(dist, can)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/pdm/installers/manager.py", line 63, in overwrite
    installed = self.install(candidate)
  File "/usr/lib/python3/dist-packages/pdm/installers/manager.py", line 32, in install
    dist_info = install_wheel(
        prepared.build(),
    ...<4 lines>...
        requested=candidate.requested,
    )
  File "/usr/lib/python3/dist-packages/pdm/installers/installers.py", line 191, in install_wheel
    dist_info_dir = install(source, destination=destination, additional_metadata=additional_metadata)
  File "/usr/lib/python3/dist-packages/pdm/installers/installers.py", line 203, in install
    _install(source, destination, additional_metadata=additional_metadata or {})
    ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/installer/_core.py", line 89, in install
    record = destination.write_script(
        name=name,
    ...<2 lines>...
        section=section,
    )
  File "/usr/lib/python3/dist-packages/installer/destinations.py", line 235, in write_script
    entry = self.write_to_fs(
        Scheme("scripts"), script_name, stream, is_executable=True
    )
  File "/usr/lib/python3/dist-packages/pdm/installers/installers.py", line 130, in write_to_fs
    make_file_executable(target_path)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/installer/utils.py", line 272, in make_file_executable
    path.chmod(0o777 & ~_current_umask() | 0o111)
    ^^^^^^^^^^
AttributeError: 'str' object has no attribute 'chmod'

This can be solved either changing the invocations of make_file_executable in pdm or adding or own version. The updated version of this pr adds the make_file_executable function to pdm.utils.

@frostming
Copy link
Copy Markdown
Collaborator

Oh, we should use our own version of that function instead of relying on installer internals. Can you make that change?

@maxyz maxyz force-pushed the fix_make_file_executable_with_str branch from 5d81412 to f807612 Compare April 10, 2026 09:09
@maxyz
Copy link
Copy Markdown
Contributor Author

maxyz commented Apr 10, 2026

Oh, we should use our own version of that function instead of relying on installer internals. Can you make that change?

Updated, ptal.

@maxyz maxyz force-pushed the fix_make_file_executable_with_str branch from f807612 to fb21e45 Compare April 10, 2026 09:12
Avoid using the installer.utils internal implementation.

An implementation detail, the implemented make_file_executable works
similar to invoking chmod +x $file, while the previously used
installer's version was similar to invoking chmod 755 $file.
@maxyz maxyz force-pushed the fix_make_file_executable_with_str branch from fb21e45 to 59011ab Compare April 10, 2026 09:22
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.14%. Comparing base (0765ad9) to head (59011ab).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3764   +/-   ##
=======================================
  Coverage   86.13%   86.14%           
=======================================
  Files         118      118           
  Lines       12417    12420    +3     
  Branches     2075     2075           
=======================================
+ Hits        10696    10699    +3     
  Misses       1150     1150           
  Partials      571      571           
Flag Coverage Δ
unittests 85.99% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@frostming frostming merged commit a101c24 into pdm-project:main Apr 10, 2026
26 checks passed
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