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

Change the way of generating new project file #1403

Open
adamsitnik opened this issue Mar 24, 2020 · 12 comments
Open

Change the way of generating new project file #1403

adamsitnik opened this issue Mar 24, 2020 · 12 comments

Comments

@adamsitnik
Copy link
Member

As of today, we generate the .csproj file by filling this text template.

The main disadvantage of this approach is that if there are any custom MSBuild settings in the project with a benchmark that we are not aware of, we sometimes fail to build the project and hence fail to run the benchmarks.

We used to follow a similar approach for generating the app.config files, but at some point in time we changed the approach to "rewrite everything from the source file unless it's on our list of settings". The code can be found here

I propose that we switch to a similar approach with the .csproj file:

  • copy the source project file
  • update all relative paths
  • replace the settings we know (TFM, optimizations enabled, project name etc)
  • add a reference to the source project

This should fix:

and a few more issues

@adamsitnik
Copy link
Member Author

@WojciechNagorski if you are still looking for some task then most probably this one would have the biggest impact (it's going to solve at least 10+ bugs)

@WojciechNagorski
Copy link
Contributor

@adamsitnik Yes, I would like to solve this task. I will find time next week.

@adamsitnik
Copy link
Member Author

Yes, I would like to solve this task. I will find time next week.

Awesome! I've assigned you to the task.

@WojciechNagorski
Copy link
Contributor

Update: I've thought for 2 weeks and I think that I invented the final solution for all cases.

@adamsitnik
Copy link
Member Author

@WojciechNagorski awesome :D

@ilohmar
Copy link

ilohmar commented Mar 17, 2021

Hi, is there any progress on the implementation? I am only asking because it sounded like the problem was basically solved. :)

We also have a custom build configuration to do out-of-tree builds, and currently use the workaround of putting a stripped-down Directory.Build.props (which does not change any output paths) into the build output dir.

@WojciechNagorski
Copy link
Contributor

@ilohmar @adamsitnik After I wanted to fix this problem I changed the project at work, then I changed work at all. So I didn't have time for this task. Currently, I have only an idea but I don't have time for it.

@timcassell
Copy link
Collaborator

@WojciechNagorski If you don't have time to work on this, could you leave your thoughts on what the solution should be so someone can work on it?

@WojciechNagorski
Copy link
Contributor

@timcassell I had some ideas and wanted to test them one by one. Two ideas are particularly promising. But I don't want to describe them because I'd have to bring back the details and that's also work and time. I would love to do this and more, even full-time, but this was only a free contribution and fun.

@timcassell
Copy link
Collaborator

timcassell commented Jul 6, 2023

update all relative paths

I'm unsure of how to reliably do this. <Reference Include=""> could be a path, or an assembly name, and it's undocumented how to determine what it is. At least I couldn't find the documentation. https://learn.microsoft.com/en-us/visualstudio/msbuild/common-msbuild-project-items?view=vs-2022 doesn't even mention the Include attribute.

[Edit] Also if the paths/assemblies include variables in them, it would be next to impossible for us to determine that without having our own mock-msbuild to resolve them.

The easiest thing to do would be to produce the generated csproj in the same directory as the original xproj. It's not ideal since it would clutter the original project directory and is separated from the generated code, but it would be able resolve all paths correctly without us having to try to guess.

Any thoughts on that @AndreyAkinshin @adamsitnik?

@ketanpkolte
Copy link
Contributor

Hi @timcassell, @adamsitnik

This issue/change involves references to a range of both old and new issues/discussions.
To clarify the scope of work, I propose creating a separate, dedicated discussion thread.

In that discussion, we should clearly define the bug/issue description and add relevant details, such as:

  • Error details
  • Reproducible tests
  • Proposed approach to fix
  • Specific changes to be applied

This will help ensure everyone has a clear understanding of the task at hand.

@timcassell
Copy link
Collaborator

Thanks for the input @ketanpkolte. I think this issue is outdated. I have already fixed most of the linked issues, and I have another PR to fix some more (#2508). Only one of the issues we haven't been able to repro so far (#1358). Also, the suggested changes introduce their own risks, and builds are already quite stable now. I think we should rather tackle the issues on a case-by-case basis. I was thinking of closing this issue after my PR is merged, or we could close it sooner if @adamsitnik is in agreement that this is no longer needed.

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

No branches or pull requests

5 participants