-
Notifications
You must be signed in to change notification settings - Fork 807
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
Add Makefile for installing framework #214
Conversation
Chisel/Makefile
Outdated
DSTROOT="$(TEMPORARY_FOLDER)" \ | ||
INSTALL_PATH=/ \ | ||
SKIP_INSTALL=NO | ||
install -d "$(PREFIX)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this do anything beyond mkdir -p
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed with above changes
Chisel/Makefile
Outdated
-sdk iphonesimulator \ | ||
install \ | ||
DSTROOT="$(TEMPORARY_FOLDER)" \ | ||
INSTALL_PATH=/ \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about Xcode's install action. Why install into a temp path and then copy from there into the true destination?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, nice I didn't re-think this after I made some other changes, updated
I should hopefully be able to accept this soon. In the mean time, I have a couple questions:
I'm indifferent about the fat binary, I'm fine with being lazy and waiting for a request/need. I don't know the ins and outs of code signing, but I hit some issues that frustrated me during development of this library. If I loaded the library from where it was originally built and code signed (within the build directory) and then later moved or copied the framework to some install location, then I would no longer be able to load the library from the installed location. The error messages were non-existent or cryptic at best, nothing that I could solve from google. Eventually I figured out that if I re-codesigned the installed library, then I wouldn't have those same loading problems. I guess what I'm asking is, is signing supposed to be post-install or can it be pre-install? Presumably pre-install, but the issues tell me that I don't really know how path comes into play with code signing verification. |
This actually is building both right now, since it's a release build,
Good question. The
|
I should be able to merge this next week. |
Chisel/Makefile
Outdated
-sdk iphonesimulator \ | ||
install \ | ||
DSTROOT="$(PREFIX)" \ | ||
INSTALL_PATH=/ \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the docs, the above two should be flipped. DSTROOT=/
and INSTALL_PATH=$(PREFIX)
.
Chisel/Makefile
Outdated
install \ | ||
DSTROOT="$(PREFIX)" \ | ||
INSTALL_PATH=/ \ | ||
SKIP_INSTALL=NO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be set to NO
in the .xcodeproj
? I'm fine with it here either way, but I'm wondering why not have it default to this anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll set it in the project since there's no reason to archive it anyways
This allows you to run `make install` with optional environment variables in order to build and install Chisel.framework.
Addressed your feedback! |
Thanks! |
You're welcome, thanks for the pull request. I'll create a pull request to add an implementation for the |
This allows you to run
make install
with optional environment variablesin order to build and install Chisel.framework.