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

[Fix] iOS builds on Windows with projects located at normal or long path lengths #317

Merged
merged 2 commits into from
Jul 12, 2022

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Jul 7, 2022

Description

One Line Summary

Resolve most iOS builds failing on Windows due Windows due 260 character file path limits.

Details

Motivation

It is common for Xamarin developers to use a Windows machine and a remote macOS machine to build. Builds were failing unless your project's root was an uncommonly short absolute path.

Scope

Only effects iOS builds, only the linking part to .xcframeworks.

Changes made

  • Instead of copying the .framework files into the project from the nuget cache via the .targets MSBuild task instead directly reference them.
  • Renamed Binding Resource files in .target and .nuspec files from content\OneSignalSDK.Xamarin.iOS.resources to res\iOS to shorten the absolute file paths

Example of error this addresses

Error		Unable to copy file "C:\Users\Kasten\.nuget\packages\onesignalsdk.xamarin\4.1.0\build\Xamarin.iOS\..\..\content\OneSignalSDK.Xamarin.iOS.resources\OneSignal.xcframework\ios-arm64_armv7_armv7s\OneSignal.framework\Headers\OneSignal.h" to "C:\Users\Kasten\source\repos\XamarinFormsTest2OneSignal4_1_0\XamarinFormsTest2OneSignal4_1_0\XamarinFormsTest2OneSignal4_1_0.iOS\bin\iPhone\Debug\/OneSignalSDK.Xamarin.iOS.resources/OneSignal.xcframework\ios-arm64_armv7_armv7s\OneSignal.framework\Headers\OneSignal.h". The specified path, file name, or both are too long. The fully qualified file name must be less than 260 characters, and the directory name must be less than 248 characters.	XamarinFormsTest2OneSignal4_1_0.iOS	C:\Users\Kasten\.nuget\packages\onesignalsdk.xamarin\4.1.0\build\Xamarin.iOS\OneSignalSDK.Xamarin.targets	6	

Issues this resolves

Testing

Manual testing

Window - remote mac build

  • Windows 11 Pro 21H2
  • Visual Studio 2022 Version 17.2.5
  • Remote mac on macOS 12.3.1
  • Xamarin.iOS 15.10.0.5
  • Xamarin.Forms project
  • Tested on both a real device and a simulator and ensured the OneSignal SDK prompts for notifications.

Mac

  • MacOS 12.4
  • Visual Studio 2022 Version 17.05
  • .Net 6.0.0.262
  • Xamarin.iOS 15.10.0.5
  • Xamarin.Forms project
  • Tested on both a real device and a simulator and ensured the OneSignal SDK prompts for notifications.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

Instead of copying the .framework files from the nuget cache into the
project use them directly on the NativeReference task.

This fixes an issue where the MSBuild Copy task fails on Windows due
it's 260 char limit.
@jkasten2 jkasten2 requested a review from tanaynigam July 8, 2022 17:55
Copy link
Contributor

@tanaynigam tanaynigam left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jkasten2)

Copy link
Member Author

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @tanaynigam)


OneSignalSDK.Xamarin.nuspec line 4 at r2 (raw file):

<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
    <metadata>
        <version>4.1.5</version>

Version should not be changed in this PR.

@jkasten2 jkasten2 changed the title [Fix] iOS builds on Windows with projects that use normal to longer path names [Fix] iOS builds on Windows with projects located at normal or long path lengths Jul 11, 2022
Copy link
Member Author

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @tanaynigam)


OneSignalSDK.Xamarin.nuspec line 4 at r2 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

Version should not be changed in this PR.

Last commit didn't revert this. Looks like, commit d0c0f01 reverted all the other nuspec changes instead.

@tanaynigam tanaynigam force-pushed the fix/long_paths_on_ios_builds_on_windows branch from d0c0f01 to d6f0ffb Compare July 12, 2022 18:07
* Rename Binding resource directory in `.targets` and `.nuspec` file from `content/OneSignalSDK.Xamarin.iOS.resources` to `res/iOS`
Copy link
Member Author

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jkasten2)

@jkasten2 jkasten2 merged commit 4bf5788 into main Jul 12, 2022
@jkasten2 jkasten2 deleted the fix/long_paths_on_ios_builds_on_windows branch July 12, 2022 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants