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

Installation instructions need updates for Windows #723

Closed
jordanpadams opened this issue Oct 5, 2023 · 5 comments · Fixed by #725 or #904
Closed

Installation instructions need updates for Windows #723

jordanpadams opened this issue Oct 5, 2023 · 5 comments · Fixed by #725 or #904
Assignees
Labels
B14.1 bug Something isn't working i&t.done

Comments

@jordanpadams
Copy link
Member

jordanpadams commented Oct 5, 2023

Checked for duplicates

Yes - I've already checked

🐛 Describe the bug

Per user, numerous issues identified by user for windows installation.

🕵️ Expected behavior

I expected installation instructions would be valid

📜 To Reproduce

PATH
In https://nasa-pds.github.io/validate/install/index.html#windows-environment, the command to append Path is given as:   

C:\> set PATH = %PATH%;C:\Program Files\validate-3.2.0\bin

but would be best given as:

C:\> set Path=%Path%;C:\Program Files\validate-3.2.0\bin

The variable name is case-sensitive and must be specified as Path, there should be no space between the variable name and =, and all spaces after = are treated as part of the value. Strictly, the (case-insensitive) interpolation could use %PATH%, but %Path% seems clearer.

JAVA_HOME
In https://nasa-pds.github.io/validate/install/index.html#windows-environment, the command to set JAVA_HOME is given as:

C:\> set JAVA_HOME = C:\path\to\java\home

but should be:

C:\> set JAVA_HOME=C:\path\to\java\home

Also, someone passingly acquainted with Windows commands might expect that double-quotes are required for paths with spaces (as they are for some commands), but that is not the case here. To clue the user to this, it might be helpful to choose an example path with spaces, such as:

C:\> set JAVA_HOME=C:\path to java\home

or an example related to the default Java directory:

C:\> set JAVA_HOME=C:\Program Files\Java\some-jre-version

Windows 64-bit JRE:
The current instructions explicitly direct the user to install 64-bit JRE, which is helpful. However, I note that the 32-bit JRE is the default on Windows. Indeed, the online installer for Windows on the All Operating Systems page (https://www.java.com/en/download/manual.jsp) only installs the 32-bit JRE, even though this is not explicitly stated on the download page. Worse yet, the only JRE on the Windows-only download page (https://www.java.com/download/ie_manual.jsp) is 32-bit (though this is not stated), and that page is the first hit on a Google search for "java download" (at least, from my system). Therefore, it might help to highlight the 64-bit caveat in the installation instructions and include a reference to https://nasa-pds.github.io/validate/operate/errors.html#Invalid_maximum_heap_size under https://nasa-pds.github.io/validate/install/index.html#Verifying_the_Installation.

🖥 Environment Info

Windows OS

📚 Version of Software Used

No response

🩺 Test Data / Additional context

No response

🦄 Related requirements

No response

⚙️ Engineering Details

No response

I&T

TestRail Test ID: T8681189

@miguelp1986
Copy link

@jordanpadams should this be i&t.skip?

@jordanpadams
Copy link
Member Author

@miguelp1986 I don't think so since we definitely want to make sure the installation instructions are valid for Windows, but this is probably validated by executing any of our smoke tests on Windows.

@miguelp1986
Copy link

miguelp1986 commented May 2, 2024

@jordanpadams Looks like changes from #725 were reverted somehow.
Screenshot 2024-05-02 at 12 12 17 AM

Screenshot 2024-05-02 at 12 14 37 AM

@miguelp1986 miguelp1986 reopened this May 6, 2024
@nutjob4life
Copy link
Member

@miguelp1986 thanks for noticing this; yes, it looks like the documentation got reverted in pull request #734 … and the lengthy copyright notices got added back too 😅

@jordanpadams
Copy link
Member Author

@miguelp1986 ☝️ this should be fixed. it is not deployed yet online, but you should see it with mvn site

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B14.1 bug Something isn't working i&t.done
Projects
None yet
3 participants