Skip to content
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

prevent_win32_max_path_length_error deletes files #95

Open
MegaIng opened this issue Jun 13, 2024 · 1 comment
Open

prevent_win32_max_path_length_error deletes files #95

MegaIng opened this issue Jun 13, 2024 · 1 comment
Assignees

Comments

@MegaIng
Copy link

MegaIng commented Jun 13, 2024

prevent_win32_max_path_length_error silently overwrites files that it shouldn't. This leads to mysterious compilation bugs.

            # Bare module. Module not from library dependency
            if '@s' not in item.name:
                mod_name = item.name.replace('@m', '')

            # Module from a library dependency. Find the package the module
            # belongs to (if any)
            else:
                segments = item.name.replace('@m', '').split('@s')

                for segment in reversed(segments):
                    if is_semver(segment):
                        index = segments.index(segment)
                        mod_name = '@'.join(segments[index:])
                        break

            new_name = ic(f'NIMPORTER@{mod_name}')
            item.replace(item.with_name(new_name))

If the for-loop through the segments doesn't find any semver segment, mod_name is not modified and the same mod_name as the last item is used, leading to the file being silently overwritten.

I would suggested multiple fixes:

  • before the replace, add assert not item.with_name(new_name).exists() to prevent future occurrences of this bug.
  • Add an else branch to the for loop that does the same thing as for "bare" modules, i.e. just remove the @m. This is necessary for local imports which wont include a semver.
  • Update the is_semver function to also detect if a commit hash is added to the semver in the format name-major.minor.patch-hash
@Pebaz Pebaz self-assigned this Jun 14, 2024
Pebaz added a commit that referenced this issue Jun 14, 2024
@Pebaz Pebaz mentioned this issue Jun 14, 2024
@MegaIng
Copy link
Author

MegaIng commented Jun 14, 2024

Note that I encountered this while trying to debug https://github.com/MegaIng/py_save_monger If you want, you could use it to test that your changes solve the issues (btw, thanks for the quick response!). Currently, it's setup.py monkey patches prevent_win32_max_path_length_error with a slightly different solution than what you are trying in #96

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

No branches or pull requests

2 participants