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

Reduce nuget path length #190

Merged
merged 3 commits into from
Jan 11, 2022
Merged

Conversation

khmMouna
Copy link
Contributor

@khmMouna khmMouna commented Jan 5, 2022

Use the format UAModuleName.resources instead of AirshipBindings.iOS.Automation.resources to reduce the nuget length path. As suggest @marc-scig I implemented the fix from this xamarin/xamarin-macios#10819

@@ -15,7 +15,7 @@

<files>
<file src="src/AirshipBindings.iOS.Basement/bin/release/AirshipBindings.iOS.Basement.dll" target="lib/Xamarin.iOS10" />
<file src="src/AirshipBindings.iOS.Basement/bin/Release/AirshipBindings.iOS.Basement.resources/**" target="content/AirshipBindings.iOS.Basement.resources" />
<file src="src/AirshipBindings.iOS.Basement/bin/Release/AirshipBindings.iOS.Basement.resources/**" target="content/Basement.resources" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we risk having conflicts with other libs? should we add a ua prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure but we can add the prefix to avoid any conflit just in case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a good idea. "Core" or "Basement" could be anything.

Copy link
Contributor

@marc-scig marc-scig left a comment

Choose a reason for hiding this comment

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

It seems to work ok on my machine. Aside from ryan's suggestion about adding a prefix, it might be a good idea to try to verify the maximum file path length for a typical installation.

For example, in the open GitHub issue there's the path C:\Users\Alex\.nuget\packages\urbanairship.ios.automation\14.4.1\content\AirshipBindings.iOS.Automation.resources\AirshipAutomation.xcframework\ios-arm64_x86_64-simulator\AirshipAutomation.framework\PrivateHeaders\UAScheduleTriggerContextTransformer+Internal.h

With your current change this would be shortened from 261 to 241 characters. I'm not sure about whether there are longer ones though, especially with an added prefix.

@khmMouna
Copy link
Contributor Author

khmMouna commented Jan 6, 2022

I just noticed that unfortunately this fix will not work with the service extension and content extension. we are going to have something like that:
C:\Users\xxxxx.nuget\packages\urbanairship.ios.notificationcontentextension\16.1.2\content\NotificationContentExtension.resources\AirshipNotificationContentExtensionr.xcframework\ios-arm64_x86_64-simulator\AirshipPreferenceCenter.framework\Headers\AirshipNotificationContentExtension.h which exceed the 255 (it's about 299) unless we want to reduce the xcframework and the package name length but I am not sure we want to that for the moment.

@rlepinski
Copy link
Collaborator

I think we should continue with this change and plan to further reduce names later

@khmMouna khmMouna merged commit 208e904 into main Jan 11, 2022
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.

4 participants