search

heal-research / SimSharp

Sim# is a .NET port of SimPy, process-based discrete event simulation framework
MIT License
126 stars 30 forks source link

Enable SourceLink #41

Closed Irubataru closed 2 years ago

Irubataru commented 2 years ago

SourceLink is quite useful for developing dependent applications as it enables users to step into the source code and open definitions of SimSharp through nuget packages. E.g. when opening the definition to a SimSharp class in a dependent project one would then get the code from github instead of getting the decompiled version from the IDE.

I am still quite new to nugets and SourceLink, but I added similar changes that the Newtonsoft repo did when adding this feature (see this pull request).

Irubataru commented 2 years ago

I am not entirely sure why the unit test fails, it doesn't on my computer. The new warnings seem to stem from compiling an older version of the standard with a newer compiler.

abeham commented 2 years ago

Ok thanks, the test can fail when the processes is suspended for a longer time. It's not the best test, but it's a bit difficult to test parallel processes and giving some guarantee on execution time. I restarted the build. Seems to be working.

abeham commented 2 years ago

Now, I also believe the reason why upgrading from netcoreapp2.1 to net5 failed on appveyor recently. I didn't remember that the build image was declared in the appveyor config 🤦‍♂️. I think we can upgrade to net5 the sample, benchmark, and test projects now (the library should remain net45 and netstandard2.0).

Anyway, I also have only very little experience with nuget and deployment related issues. But I did a little bit of research. This source mentions that we shouldn't add ContinuousIntegrationBuild (deterministic builds) unconditionally:

These should not be enabled during local dev or the debugger won’t be able to find the local source files.

We should do ContinuousIntegrationBuild on appveyor only and then also configure appveyor so that the artifact can be downloaded and be uploaded to nuget for distribution (I was publishing local builds so far). Do you happen to know the condition that we need to check when it's running on appveyor?

Irubataru commented 2 years ago

Good catch. From the appveyor docs it seems like the flag would be APPVEYOR (or simply CI). I'll add it to the config.

Irubataru commented 2 years ago

Maybe it would be a good idea to make the realtime unit tests not run on Appveyor. Could e.g. be a global config that checks the APPVEYOR environment flag and skips the test if it is set, using e.g. Xunit.SkippableFact. Can either define something in the PropertyGroup or just have a simple static method.

Irubataru commented 2 years ago

Would it be possible to publish an updated nuget package so I could see if source link works?

abeham commented 2 years ago

I uploaded it to nuget as prerelease version: https://www.nuget.org/packages/SimSharp/3.4.1-pre1

Also, the build server will from now on produce the nupkg as artifacts (retention period is one month I think): https://ci.appveyor.com/project/abeham/simsharp/build/artifacts

Can you also check the experience of debugging models? E.g. stepping through a process in the debugger? I hope that if you say "Just My Code" it will overstep the Sim# library stuff. Because for me that makes a huge difference. If I hit F10 and the debugger shows me the next line of model code, then that's good. But if your thrown into the Process class and Step() logic then stepping through processes with the debugger is annoying. I am not sure how VS treats this when the pdb is present.

Irubataru commented 2 years ago

I am having a bit of trouble getting source link to work in VS all together. However, it works well in Rider (which is the C# IDE I actually use). I can't test the Just My Code settings yet.

However, I have two observations. First it seems like the symbol package wasn't uploaded to nuget.org. If you look at e.g. the Newtonsoft.Json page there is a link to download symbols that is not present for SimSharp. Second, there seems to be an issue with the config still. If you look at the checks in the Nuget Package Explorer there are warnings about untracked sources, so maybe we also need the <EmbedUntrackedSources>true</EmbedUntrackedSources> flag in the project file. Also, it seems like <AllowedOutputExtensionsInPackageBuildOutputFolder> might not be necessary if you publish the symbols file to nuget (see this comment).

abeham commented 2 years ago

Okay, I have no idea how this all works, but Newtonsoft.Json produces a .snupkg file and not a symbols.nupkg, because it seems like the .snupkg is an add-on to the regular .nupkg while the .symols.nupkg is an either-or to the regular that contains just the source code in addition. I changed the settings as you mentioned and added the tags to produce snupkg instead of the other. I uploaded 3.4.1-pre2 to nuget.org. Please check that one out.

Irubataru commented 2 years ago

I also do not fully know how this works, I have been trying to figure out source link for a while now and it all seems a bit like a mess. I hope these things get more standardized because it is not good as it is now.

Anyway, I tested it out and it works great now! I am able to step into the SimSharp code with F11, and if I enable Just My Code it doesn't as expected.

Thanks a lot! <3