Skip to content

Log when running from cache #551

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

Merged
merged 2 commits into from
Jun 19, 2020
Merged

Conversation

atifaziz
Copy link
Contributor

@atifaziz atifaziz commented Jun 6, 2020

With this PR, I'd like to suggest adding the cache location to the dotnet script --info display. It takes out the guesswork at times.

Before this PR, dotnet script --info will print:

Version             : 0.53.0
Install location    : A:\dotnet-script\src\Dotnet.Script\bin\Debug\netcoreapp3.1
Target framework    : netcoreapp3.1
.NET Core version   : 3.1.4
Platform identifier : win
Runtime identifier  : win10-x64

After this PR, dotnet script --info will print:

Version             : 0.53.0
Install location    : A:\dotnet-script\src\Dotnet.Script\bin\Debug\netcoreapp3.1
Target framework    : netcoreapp3.1
.NET Core version   : 3.1.4
Platform identifier : win
Runtime identifier  : win10-x64
Scripts temp path   : C:\Users\johndoe\AppData\Local\Temp\dotnet-script

@filipw
Copy link
Member

filipw commented Jun 9, 2020

Thanks for the PR, however, I don't think this is needed.
Cache path is an internal implementation detail so it shouldn't show up with the main info, just like nuget restore path or package store path doesn't show up on dotnet --info. IMHO it's enough that it shows up in debug output and is documented in the documentation.

@atifaziz
Copy link
Contributor Author

atifaziz commented Jun 9, 2020

dotnet --info does not show the NuGet or package restore path because those are SDK-specific details. You can get to some of that info through more specific sub-commands like dotnet nuget locals --list all.

Also, dotnet script --info already does far more than dotnet --info, like checking for the latest version and providing instructions for updating so it seems to have deviated from its role model anyway.

it shows up in debug output

Fair point, but the problem is that the caching information is only emitted when a build takes place (when cache is missed). On subsequent runs with a cache hit, there is no way to discover or get back that information without changes the script source to cause an invalidation. All I see is:

PS> dotnet script -d test.csx
dbug: Dotnet.Script.DependencyModel.ProjectSystem.ScriptParser[0]
      Parsing A:\dotnet-script\bar.csx
Hello!

I agree it's an internal implementation detail but it's a substantial performance feature that leaks into the environment and so it's information that can potentially help with diagnostics. While the temp path is documented now, 😉 you still have to go looking for it and the README.md in the master branch, which by the way, won't necessarily reflect your deployed version. If not, you have to go looking for the README.md corresponding to the version you're using, but then that may not always have the info.

@filipw
Copy link
Member

filipw commented Jun 9, 2020

I think the best solution is to just add "running from cache... {path}" to the debug output

@atifaziz
Copy link
Contributor Author

atifaziz commented Jun 9, 2020

Okay, I can compromise with that. See 8c67160.

@atifaziz atifaziz changed the title Add cache info to "--info" mode Log when running from cache Jun 9, 2020
Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

thanks!

@filipw filipw merged commit 94741ec into dotnet-script:master Jun 19, 2020
@atifaziz atifaziz deleted the info-cache branch June 19, 2020 17:39
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.

2 participants