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 - Using Junctions on Windows for Seamless Builds #3198

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

hellerim
Copy link

Added description of how to build Idris2 on Windows using junctions.

Description

Should this change go in the CHANGELOG?

  • If this is a fix, user-facing change, a compiler change, or a new paper
    implementation, I have updated CHANGELOG_NEXT.md (and potentially also
    CONTRIBUTORS.md).

Added description of how to build Idris2 on Windows using junctions.
mattpolzin
mattpolzin previously approved these changes Jan 22, 2024
Copy link
Collaborator

@mattpolzin mattpolzin left a comment

Choose a reason for hiding this comment

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

I don't have access to a Windows machine but I can imagine how this additional tip could play out so if no one chimes in to suggest otherwise then this looks like a good addition.

INSTALL.md Outdated
built on windows is to create a junction without spaces for the ChezScheme base folder, e.g.

```sh
mklink /j C:\Chez "C:\Program Files\Chez Scheme 9.6.4"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to have the version be an environment variable, or even better: extracted from chez --version or similar? Just so we don't risk people mistyping the version, and so we don't have to update this whenever a new Chez version releases : )

Copy link
Author

@hellerim hellerim Jan 22, 2024

Choose a reason for hiding this comment

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

probably we are bound to manual handling. Chez Scheme appears to work through several nested cmd calls. I didn't manage to pipe its output into a file; scheme --version > chezvers.dat leaves the file empty; equally I didn't succeed to capture the output in an environment variable. But even if this worked, the junction to the old version should be removed.

In my opinion trying to fully automate updates is not worth the effort. The mklink command is well documented by Microsoft (including the command's delete option), and there is no magic: A junction is a small file which points to a folder; although the dir command produces a listing, a junction cannot be used by programs to run through the folder entries normally.
My first idea was to modify the ProgramFiles environment variable to include double quotes. It turned out that this does not work. Anyway, to keep risk low, I used a Windows virtual machine. (You can get a legal copy of Windows 11 for about 12 $.) Wouldn't this be a possibility to produce a distributable binary copy of the current version and to include this on the download page? I have no idea how much work this would mean in addition to what I experienced - apart from starting the build, the rest takes a couple of minutes which don't require anyone's presence. Running a test suite could be automated as well.

Copy link
Author

@hellerim hellerim Jan 22, 2024

Choose a reason for hiding this comment

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

Here is a solution: Copy these two lines to the file mkChezLink.cmd:

@rd C:\Chez
@for /f "tokens=1" %%i in ('dir "%ProgramFiles%\Chez*" /B /O-D') do mklink /j C:\Chez "%ProgramFiles%\%%i"

Running it will delete an existing junction and create a new one. If Chez Scheme is not installed it does nothing but display a file not found message. In case there are multiple versions it will use the latest one. If the junction does not exist, it will display an error.
Anyway, C:\Chez must be added to the system path (one-time action).

Copy link
Author

@hellerim hellerim Jan 23, 2024

Choose a reason for hiding this comment

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

In the forcommand, a delimiter needs to be added. Otherwise, folder names are extracted only up to the first space. Hence:

@for /f "tokens=1 delims=:" %%i in ('dir "%ProgramFiles%\Chez*" /B /O-D') do mklink /j C:\Chez "%ProgramFiles%\%%i"

I corrected this in my fork.

@CodingCellist
Copy link
Collaborator

Would a simple shortcut file work? I am slightly cautious of asking people to create fancy file-system things, especially for stuff that will probably need updating/removing (new Chez release)...

Copy link
Collaborator

@CodingCellist CodingCellist left a comment

Choose a reason for hiding this comment

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

Thank you for the explanations and for putting this together! : )
LGTM, but I think we'll give it a week or so before merging, just to give people with a Windows machine a chance to try it out and report back any errors : )

@hellerim
Copy link
Author

hellerim commented Jan 24, 2024 via email

Made up a complete workflow for Windows since too many things can go wrong and information helpful to remedy the situation is scattered across the web.
@hellerim
Copy link
Author

hellerim commented Jan 24, 2024 via email

Copy link
Collaborator

@mattpolzin mattpolzin left a comment

Choose a reason for hiding this comment

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

The newest changes to this PR are a bit more of what I would expect to see in a third party resource that walks through installation step by step for a particular target audience. I think we should keep the INSTALL.md file a bit more high level so that it is easy to use as a resource for those already experienced with build tools and easier to maintain.

I don't mean we shouldn't be supportive of all skill levels, but just like we don't go into detail on installing individual prerequisites for Linux I don't think we want to go into details of installing MSYS or Mingw; I imagine there are resources elsewhere online that detail getting Mingw running in MSYS on Windows. Stating that one must have Mingw running in MSYS should suffice. Similarly, the new changes repeat some previous information (e.g. the need to specify the name of the Chez executable with the SCHEME environment variable when building).

TLDR; I think comprehensive walkthroughs aimed at various levels of familiarity with tooling are really great resources, but (and this is my personal opinion, other maintainers may disagree) the INSTALL.md file should give all necessary information without repeating itself or readily available information on installing dependencies.

@mattpolzin
Copy link
Collaborator

I think I'll close this PR soon as it seemed to stall out. Early iterations of this PR felt really good to me, but then as I noted above later change sets seemed to repeat already stated information and go into a degree of depth that felt more in line with a third party walk through tailored to a specific audience rather than a concise list of required steps to get Idris installed.

@melted
Copy link
Collaborator

melted commented Jun 12, 2024

Looks good to me as a Windows user.

@mattpolzin
Copy link
Collaborator

To be clear, I was going to close without merging in line with my comments about things I thought should be changed before merging it. If general consensus is that we should merge this as-is, that’s ok with me.

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