Skip to content

Fix custom-module copies inheriting read-only permissions#45686

Merged
Rocketknight1 merged 1 commit intohuggingface:mainfrom
nurpax:fix/dynamic-module-utils-readonly-permissions
Apr 29, 2026
Merged

Fix custom-module copies inheriting read-only permissions#45686
Rocketknight1 merged 1 commit intohuggingface:mainfrom
nurpax:fix/dynamic-module-utils-readonly-permissions

Conversation

@nurpax
Copy link
Copy Markdown
Contributor

@nurpax nurpax commented Apr 28, 2026

What does this PR do?

Fixes #45684.

shutil.copy is copyfile + copymode, so when source files are read-only (common with version-control systems like Perforce that check files out as r--r--r-- until edit), the destination inherits those permissions. This breaks post-save tooling that wants to rewrite the saved module file, and leaves users with read-only files in their saved-model directories.

The fix swaps shutil.copy for shutil.copyfile at all five call sites in dynamic_module_utils.py (one in custom_object_save, four in get_cached_module_file). copyfile copies file contents only; the destination, when newly created, gets standard umask-based permissions, which is what callers of save_pretrained expect.

A regression test is added to tests/utils/test_dynamic_module_utils.py: it makes a source module read-only, calls custom_object_save, and asserts the destination is writable. The test fails on main and passes with this fix.

Code Agent Policy

  • I confirm that this is not a pure code agent PR.
    • AI was used for making the change, but I hand-edited, reviewed and added tests myself.

Before submitting

Who can review?

@Cyrilvallez (model loading)

@nurpax
Copy link
Copy Markdown
Contributor Author

nurpax commented Apr 28, 2026

AI was used to author some of this code but I reviewed & hand-edited the contribution myself.

@nurpax nurpax force-pushed the fix/dynamic-module-utils-readonly-permissions branch from a6a6423 to 30f65e4 Compare April 28, 2026 17:02
@nurpax nurpax force-pushed the fix/dynamic-module-utils-readonly-permissions branch from 30f65e4 to a9728da Compare April 29, 2026 06:20
@Rocketknight1
Copy link
Copy Markdown
Member

Looks good, but we can skip the test I think! The custom saving code doesn't get updated too often, so we should be fine without a regression test. Can you revert the test file and ping me to merge the fix?

…e#45684)

`shutil.copy` is `copyfile` + `copymode`, so when source files are
read-only (common with version-control systems like Perforce that
check out files as `r--r--r--`), the destination inherits those
permissions. This breaks post-save tooling and leaves users with
read-only files in their saved-model directories.

Switch to `shutil.copyfile` at all five call sites in
`dynamic_module_utils.py`. New files then get standard umask-based
permissions, matching what callers of `save_pretrained` expect.
@nurpax nurpax force-pushed the fix/dynamic-module-utils-readonly-permissions branch from a9728da to d19b012 Compare April 29, 2026 10:17
@nurpax
Copy link
Copy Markdown
Contributor Author

nurpax commented Apr 29, 2026

Thanks @Rocketknight1! I removed the test case in the latest commit version. The change should be ready to merge.

Copy link
Copy Markdown
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Yep, LGTM now. Thank you!

@Rocketknight1 Rocketknight1 enabled auto-merge April 29, 2026 10:38
@Rocketknight1 Rocketknight1 added this pull request to the merge queue Apr 29, 2026
@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Merged via the queue into huggingface:main with commit 1ca0be5 Apr 29, 2026
28 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.

save_pretrained (with register_for_auto_class`) propagates read-only permissions from custom-model source files

3 participants