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

Fixes generated paths folders from get too long #4056

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

andrueastman
Copy link
Member

Fixes #3854

In summary, changes include

  • Prevents the namespace folder name from getting too long
  • Normalized paths if they are longer than windows MAX_PATH similar to Go

@andrueastman andrueastman marked this pull request as ready for review January 25, 2024 13:42
@andrueastman andrueastman requested a review from a team as a code owner January 25, 2024 13:42
@andrueastman andrueastman force-pushed the bugfix/pathsTooLong branch 2 times, most recently from 3e591a5 to 3291486 Compare January 26, 2024 10:55
@andrueastman andrueastman requested a review from baywet January 26, 2024 10:55
auto-merge was automatically disabled January 26, 2024 11:09

Pull Request is not mergeable

Copy link

@andrueastman andrueastman requested a review from baywet January 26, 2024 14:19
@andrueastman andrueastman merged commit 9fc77f3 into main Jan 29, 2024
185 of 186 checks passed
@andrueastman andrueastman deleted the bugfix/pathsTooLong branch January 29, 2024 17:21
}
return fullPath;
}
internal const int MaxFilePathLength = 32767;

Choose a reason for hiding this comment

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

PR description mentions MAX_PATH, which is 260

32767 is max length of an extended-length path, which requires \\?\ prefix.

This constant should be 260. I'm I mistaken?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for double-checking.
Let's start by defining the question precisely, which has historically been the challenge when working on path lengths:
which maximum path length should kiota generate files at?

This has two main components:

  • what does the local file system support as a maximum path length?
  • what will the compiler receiving those files support?

We SHOULD NOT limit the generation to an outdated requirement pushed from older OS configurations:

  • windows supports 32K characters and has for a while, yes, it's still not turned on by default, but it's in some enterprises, and it's easy to do so if needed. And this without having to specify the prefix.
  • linux has supported ~4k characters for a long time as well now.

We could consider 4k our upper-bound for any language out there as a safety measure, but this is something that's likely to change in the future, also this is something user can change either by configuration or by targeting a shorter path.

As for the compiler receiving those files, there are situations where in order to be packaged, the file path relative to the package directory cannot exceed a specific length (npm, ruby...) , or the compiler will impose limitations (go), in which case to maintain compatibility with the ecosystem being targeted, we should (and have) implement file path length limits when possible.

I hope this addresses your concern?

Choose a reason for hiding this comment

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

Makes sense. Thanks!

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.

Generated servicenames get too long
3 participants