search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.34k stars 9.98k forks source link

Documentation : Guide for building the project requires some additional steps #37240

Open CarlSargunar opened 3 years ago

CarlSargunar commented 3 years ago

As part of following the guide from https://github.com/dotnet/aspnetcore/blob/main/docs/BuildFromSource.md, I was unable to complete installing the JDK because I was receiving the following error

` ./eng/scripts/InstallJdk.ps1 : File D:\Work\dotNET\aspnetcore\eng\scripts\InstallJdk.ps1 cannot be loaded because running scripts is disabled on this system. For more information, see about_Execution_Policies at https:/go.microsoft.com/fwlink/?LinkID=135170. At line:1 char:1

CarlSargunar commented 3 years ago

I will be submitting a PR with the steps I had to take to get around this shortly

CarlSargunar commented 3 years ago

Looks like this is managed a little earlier in the guide, but I missed it. Perhaps it could do with re-wording a little.

Would this guide be better split into separate guides for

As it's quite a long guide and steps can be missed (as I did). Happy to take that on if it would help, let me know :-)

dougbu commented 2 years ago

@captainsafia what are your thoughts on the "split into separate guides" idea❔ If we don't move forward w/ that, we should close this issue since #37241 is now in.

dougbu commented 2 years ago

/ping @captainsafia on whether there's work you think we should do here

dougbu commented 2 years ago

Adding @vitek-karas's comments in #41158:

Installing nodejs + yarn + JDK - not a great experience based on the docs. On Windows I was able to do all of this via winget eventually (after lot of searching) and probably a lot faster then following the various websites. On Linux it feels even worse - ended up searching how to do it via apt-get mostly. But I must admit I never worked with nodejs and so on, so maybe this is just me doing it for the first time. Lastly - I was trying this on Windows and Linux WSL. The WSL seems to cause issues for the Selenium based tests - I think I installed everything per the doc (but I could be wrong) and the Selenium tests were still failing for me. I guess there's a bit of a weird dichotomy to the document - it starts explaining git in relative detail. But then the section on installing pre-reqs is often very short - especially nodejs which is basically just a link. I like that at least on Windows Java can be installed by a script - that helps a lot. Would be nice to have the same for Linux. Very last minor nit: I use PowerShell even on Linux, but the PS scripts in the repo fail in weird ways when used on Linux (didn't investigate why). For example the activate.ps1 misbehaves - and the shell script is not a replacement since it won't do me any good when in PS. P.S.: I find it "funny" that the repo requires two competing stacks to build (NodeJS and Java). Maybe a short sentence on what they're used for?

captainsafia commented 2 years ago

/ping @captainsafia on whether there's work you think we should do here

Need to revive and update https://github.com/dotnet/aspnetcore/pull/37362

vitek-karas commented 2 years ago

Couple of additional observations:

vitek-karas commented 2 years ago

Some more observations:

wtgodbe commented 2 years ago

@captainsafia is there a different issue we should transfer the feedback to BuildFromSource.md to? Assigning this issue to you for now

captainsafia commented 2 years ago

@wtgodbe Nope! Good to use this issue. This is something I was hoping to look into after preview7.

wtgodbe commented 2 years ago

@captainsafia any update on this one?

captainsafia commented 2 years ago

@wtgodbe Not yet. I think this will be 7.0 GA work for me. :/

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

captainsafia commented 2 years ago

Would this guide be better split into separate guides for

With the merge of #44127, the guide is now more scenario focused. It's not split by operating system but by editor/platform choice which ends up having a bigger impact on setup.

One has to use the InstallVisualStudio.ps1 even if only to update existing install. The .vsconfig in the repo root is not enough for even the restore.cmd to work.

This is clarified in Step 2 of the VS on Windows docs but we can make it a little clearer with a warning call out.

It's necessary to install NodeJS 16 LTS, installing the latest 18 will not work it seems

Yeah, the rule is that at least NodeJS LTS is required but higher versions are allowed. We've avoided documenting the specific version in the docs since it is easy for it to grow out of sync with the version that is required in the various package.json files.

For building a specific project some team members suggested to use just .dotnet\dotnet build - that doesn't work in many cases (for example the IIS subtree doesn't build this way). One has to use the .\eng\build.cmd -Projects command to build.

I think the IIS is the exception, not the norm, so it might be sufficient to document that it specifically needs ./build.cmd always in its individual README.

Selenium tests require Chrome, which will typically install latest version (at the time of writing this comment it is 101). Then they use chrome driver which must match the version of the installed Chrome. This means editing src\Shared\E2ETesting\selenium-config.json - also note that the version of the chrome driver typically doesn't match exactly the version of Chrome. I was able to find the matching version here: https://chromedriver.chromium.org/downloads. For Chrome 101 it is 101.0.4951.41. So edit the json file to contain that version number. Then rerun yarn in the E2ETest directory to install the new chrome driver.

It feels like the right fix here is to make sure that our selenium-config is pointing to the right build so that users don't have to manually edit the file. We could probably add this verification as an additional validation for our existing build-ops task around keeping these Selenium dependencies up-to-date.

When using the InstallJdk.ps1 the PATH must point to something like \aspnetcore.tools\jdk\win-x64\bin, so it has to have the bin appended unlike the JAVA_HOME which should point to \aspnetcore.tools\jdk\win-x64.

For this feedback, I think we can include it as a note in the new JDK dependency section at the bottom.

dougbu commented 2 years ago

It feels like the right fix here is to make sure that our selenium-config is pointing to the right build so that users don't have to manually edit the file. We could probably add this verification as an additional validation for our existing build-ops task around keeping these Selenium dependencies up to date.

We should treat out-of-date Selenium dependencies as a bug and address the problem before it affects any developer.

Our regular updates are intended to do this, but we sometimes forget to update the versions in servicing branches. That means we sometimes don't notice versions are out of date until branding PRs hit the problem ☹️

I think we can include it as a note in the new JDK dependency section at the bottom.

Good idea. I normally scan eng/build.ps1 to find how to set $env:JAVA_HOME and extend $env:Path for Java. That or hope my command history contains

$env:JAVA_HOME = "$PWD\.tools\jdk\win-x64\"
$env:PATH = "$(Join-Path $env:JAVA_HOME bin);${env:PATH}"
dougbu commented 2 years ago

I think the IIS is the exception, not the norm

To be clear, we also have some native code in src/Installers/Windows/AspNetCoreModule-Setup/. that's IIS-related of course. The general issue is that we build as much as we can with dotnet msbuild but have to use desktop msbuild for any native C++ projects.

On dotnet msbuild versus the eng/build.* scripts, note one run of the scripts is usually necessary to ensure RepoTasks.csproj and GenerateFiles.csproj are correct for the current branch and eng/Versions.props content.

captainsafia commented 2 years ago

Good idea. I normally scan eng/build.ps1 to find how to set $env:JAVA_HOME and extend $env:Path for Java. That or hope my command history contains

Does it make sense to update the InstallJdk script to do this in the same way the install-dotnet scripts automatically update your path? It would reduce the burden on the user to set up these environment variables correctly.

dougbu commented 2 years ago

Does it make sense to update the InstallJdk script to do this in the same way the install-dotnet scripts automatically update your path?

The InstallJdk script is rarely run and isn't dot-sourced. Probably better to update activate.ps1 to make the changes if the script notices .tools/jdk/ exists and contains files.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.