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

resolve watch issues with empty siteContents; remove unneeded Option #103

Merged
merged 1 commit into from
May 7, 2022

Conversation

bigjonroberts
Copy link
Contributor

This resolves #96 by only initializing SiteContents once when in watch mode.

I also removed an option for config in Generator.fs, as we're already potentially raising another exception in that let binding, so loading out and option and then immediately raising an exception on None didn't seem to add any value but to clutter up the code.

@rdipardo
Copy link
Contributor

rdipardo commented Apr 1, 2022

I was looking forward to trying this, but had to downgrade my targeting packs first (*). (Are we sure that fake-cli is the best choice for dependency management?)


(*) Linux does clean dotnet installs by default, so a simple feature upgrade leaves fake-cli completely borked:

$ FAKE_SDK_RESOLVER_CUSTOM_DOTNET_PATH=/usr/share/dotnet dotnet fake build -t TestTemplate
The last restore is still up to date. Nothing left to do.
Performance:
 - Cli parsing: 967 milliseconds
 - Packages: 314 milliseconds
   - Creating Runtime Graph: 232 milliseconds
   - Retrieve Assembly List: 4 seconds
 - Runtime: 7 seconds
There was a problem while setting up the environment:
-> Could not find referenced assemblies in path: '/usr/share/dotnet/packs/Microsoft.NETCore.App.Ref/6.0.0/ref/net6.0', please check installed SDK and runtime versions
Hint: If you just upgraded the fake-runner you can try to remove the .fake directory and try again.

$ dotnet --list-sdks
3.1.417 [/usr/share/dotnet/sdk]
5.0.406 [/usr/share/dotnet/sdk]
6.0.201 [/usr/share/dotnet/sdk]

$ ls -l /usr/share/dotnet/packs/Microsoft.NETCore.App.Ref
total 12
drwxr-xr-x 4 root root 4096 Mar  7 20:34 3.1.0
drwxr-xr-x 4 root root 4096 Mar  7 01:46 5.0.0
drwxr-xr-x 5 root root 4096 Mar 11 06:13 6.0.3

The "solution" (for deb-based distros):

sudo aptitude install dotnet-targeting-pack-6.0=6.0.0-1 aspnetcore-targeting-pack-6.0=6.0.0-1
sudo aptitude hold dotnet-targeting-pack-6.0 aspnetcore-targeting-pack-6.0

@bigjonroberts
Copy link
Contributor Author

@rdipardo i just tested it locally using dotnet build and dotnet publish

@rdipardo
Copy link
Contributor

rdipardo commented Apr 3, 2022

@rdipardo i just tested it locally using dotnet build and dotnet publish

That works! Bravo!

@bigjonroberts
Copy link
Contributor Author

I plan to test this week on a file deletion. That might be the only thing left to do?
If that works, I might make a separate PR to address incremental builds, as I could see the build time being annoying if you had a large site and were working on it.

@rdipardo
Copy link
Contributor

I might make a separate PR to address incremental builds

Please do!

@bigjonroberts
Copy link
Contributor Author

bigjonroberts commented Apr 25, 2022

Is there something else that needs to be tested before this could be merged?

@bigjonroberts
Copy link
Contributor Author

bigjonroberts commented Apr 26, 2022

I tested it with deletes: it removes any links as expected, but you need to run a clean to remove any other items. This is not unexpected behavior.

Editing or adding files works fine.

@Krzysztof-Cieslak Is there any other testing or documentation you would expect?
This is not a feature change, but resolves a known bug.
Any objections to including it in the next release?

@bigjonroberts
Copy link
Contributor Author

@baronfel This fixes a known bug. Do you see any issues that this shouldn't be put into a bugfix release?

@baronfel
Copy link
Contributor

baronfel commented May 7, 2022

Thanks for implementing @bigjonrobers, and for testing @rdipardo! I'll merge and make a bugfix release.

@baronfel baronfel merged commit bafe619 into ionide:master May 7, 2022
@bigjonroberts bigjonroberts deleted the empty-sitecontents branch December 27, 2022 22:28
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.

Uninitialized SiteContents being passed to post generator in watch mode
3 participants