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

[mono] Improve iOS sample Makefile #105316

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Jul 23, 2024

Fixes an issue where we were still harcoding the Debug config for tasks even though that changed with #84931.
Also align the Makfile to make it more similar to the iOS-NativeAOT one.

Fixes an issue where we were still harcoding the Debug config for tasks even though that changed with dotnet#84931.
Also align the Makfile to make it more similar to the iOS-NativeAOT one.
@akoeplinger akoeplinger requested a review from ivanpovazan July 23, 2024 14:18
@akoeplinger akoeplinger requested a review from marek-safar as a code owner July 23, 2024 14:18
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 23, 2024
@akoeplinger akoeplinger added area-Build-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 23, 2024
@ivanpovazan
Copy link
Member

Overall looks good, but we will need to update the perf jobs to pass the new make variables, e.g.:

- script: make build-appbundle TARGET=ios MONO_ARCH=arm64 MONO_CONFIG=Release AOT=True USE_LLVM=False DEPLOY_AND_RUN=false STRIP_DEBUG_SYMBOLS=false HYBRID_GLOBALIZATION=${{ parameters.hybridGlobalization }}

@akoeplinger
Copy link
Member Author

@ivanpovazan thanks, fixed

/p:MonoEnableLLVM=false \
/p:MonoForceInterpreter=true \
/p:DeployAndRun=$(DEPLOY_AND_RUN) \
/p:EnableAppSandbox=$(APP_SANDBOX) \
/bl

clean:
rm -rf bin
rm -rf obj bin *.binlog
Copy link
Member

Choose a reason for hiding this comment

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

nit: I might be wrong, but if I remember correctly the intermediate dir for the Mono sample will be in artifacts folder.
For NativeAOT iOS sample we did not set it up correctly so we are creating both bin and obj next to the project file.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked and we have the folders in the current directory too.

Copy link
Member

@ivanpovazan ivanpovazan left a comment

Choose a reason for hiding this comment

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

I left a minor note. Otherwise LGTM! Thanks!

@akoeplinger akoeplinger merged commit f6994c7 into dotnet:main Jul 24, 2024
66 of 69 checks passed
@akoeplinger akoeplinger deleted the fix-makefile branch July 24, 2024 10:54
@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants