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

Update INSTALL.md #2708

Merged
merged 3 commits into from
Jan 14, 2024
Merged

Update INSTALL.md #2708

merged 3 commits into from
Jan 14, 2024

Conversation

buggymcbugfix
Copy link
Contributor

To successfully build Idris2 and run the tests, I had to set some environment variables, see also this discussion on Discord.

For the benefit of others, I think it makes sense to add this to the Installation instructions.

@andrevidela
Copy link
Collaborator

Hi there, can you replace your link to discord with a summary of the situation? People on Github might not have access to discord, and it makes it easier to review if all the information necessary is on the PR description wihtout having to follow any links.

Thanks!

@CodingCellist
Copy link
Collaborator

CodingCellist commented Oct 10, 2022

Related: #2233
might already be addressed by: #2669 ?

@buggymcbugfix
Copy link
Contributor Author

I merely documented what worked for me. Feel free to close/amend this PR.

@joelberkeley
Copy link
Contributor

... might already be addressed by: #2669 ?

probably not. I don't think I edit any installation instructions in that, just move them about

@gallais
Copy link
Member

gallais commented Jul 31, 2023

I merely documented what worked for me.

Has someone else installed on M1/M2 since October and needed such things?
I can't test this as I don't have access to any such machines.

@mattpolzin
Copy link
Collaborator

I'll try to confirm some of these steps soon (just got access to an Apple Silicon laptop). Some of these changes were confirmed to be needed by Steve in #2233. Others of these suggestions should AFAIK not be required regardless of architecture (e.g. the LD_LIBRARY_PATH/DYLD_LIBRARY_PATH is set by the wrapper around the Idris scheme executable and should not therefore need to be set by the user prior to install).

INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
@mattpolzin
Copy link
Collaborator

Ok, I just got done doing a run-through on my aarch64-darwin machine.

I found (as did the OP of this PR and Steve Dunham) that the changes on lines 52-58 to INSTALL.md were very much needed prior to building Idris 2 and also prior to building a subsequent executable using the RefC backend. Those would be a great addition to the instructions.

I did not need to change the installation steps when I built ChezScheme, so I am not sure if the OP needed to change those steps in order to get things working or just changed them to simplify things. If it's the latter, I have not yet had the time to confirm that skipping many of the steps originally in the INSTALL.md file work on my machine and I would be tempted to say we should keep those instructions the same given that we don't have a second confirmation that the simplification works longterm (i.e. past the initial build & test of the compiler).

Finally, the other change in this PR is to put $PREFIX/lib into the LD_LIBRARY_PATH or DYLD_LIBRARY_PATH and this is not something that I found was needed when building the compiler or building a subsequent project using the RefC backend. Unless we get more details on what circumstances dictated requiring the change, I vote we do not add that bit to the instructions. If we do hear more about the circumstances requiring that change, I would be hopeful that we can make changes to the scripts or compiler themselves to obviate the need for the user explicitly adding the compiler's installed lib directory to their library path.

…need the additional DYLD_LIBRARY_PATH entry and I was not confident that there was no impact from the changes to the Chez Scheme build instructions so I am mostly reverting those changes
@mattpolzin
Copy link
Collaborator

This PR has a very important additional instruction that I want to get merged so I went ahead and made the changes I had previously suggested in my review comments.

@mattpolzin mattpolzin merged commit c4f99a8 into idris-lang:main Jan 14, 2024
3 checks passed
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.

6 participants