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

Remove project.json support #526

Closed
adamsitnik opened this Issue Aug 13, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@adamsitnik
Member

adamsitnik commented Aug 13, 2017

As reported by @peter-takacs here the way we used to determine if project.json or .csproj should be used can cause some bugs when user interrupts benchmark run with ctrl+c.

Hi,

I recently encountered a similar issue using .NET Core 2 preview, Visual Studio 2017 15.3 preview 6 and BenchmarkDotNet 0.10.9.

I managed to track it down to the fact that during a previous run of BDN that was aborted, no cleanup was performed, so project.json files generated by BDN remained within the directory tree of the solution.
This made BDN think that I am using .json files for my projects, whereas I am using .csproj.

I looked at the source and saw how BDN determines whether we're using .json or .csproj and I think it's extremely brittle, since it relies on preview being present in the name of the runtime and recursively (!) scans the directory structure for project.json files.

I think that this solution is less than ideal, it took me quite a while to figure out what was going wrong. I propose two things to make it easier for people in the future:

During build indicate what the automatically detected toolchain was, optionally how BDN reached that conclusion.
Make the choice of toolchain something that needs to be explicitly configured while setting up the benchmark. I understand the need for easy setup and sane defaults in configuration, but I believe that is only a feasible route when we can determine the runtime with certainty.
Thoughts?

I asked on Twitter and only single person is still using it, but he is fine with the removal.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment