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.

Sign up for GitHub

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

Jump to bottom

Copy PackageReferences to generated csproj #2347

Merged
merged 3 commits into from
Jul 3, 2023
Merged

Copy PackageReferences to generated csproj #2347

merged 3 commits into from
Jul 3, 2023

Conversation

timcassell
Copy link
Collaborator

@timcassell timcassell commented Jun 29, 2023 •
edited
Loading

Fixes #2344 and fixes #2259

@timcassell timcassell marked this pull request as draft June 29, 2023 00:14
@timcassell timcassell force-pushed the csproj-packagereference branch 2 times, most recently from 5af1cfc to d50f562 Compare June 29, 2023 01:11
@timcassell
Copy PackageReferences to generated csproj.
85befa1 
Explicitly specify program entry point.
@timcassell timcassell force-pushed the csproj-packagereference branch from d50f562 to 85befa1 Compare June 29, 2023 01:40
@timcassell timcassell marked this pull request as ready for review June 29, 2023 02:56
@AndreyAkinshin
Copy link
Member

@timcassell could you please provide some explanations on why we actually have to copy the PackageReferences? Why the default NuGet transitive reference mechanism is not good enough for our use cases?

@timcassell
Copy link
Collaborator Author

timcassell commented Jun 29, 2023 •
edited
Loading

Tbh, I'm not sure why. This was just an experiment I tried to fix the issue, and it worked for me, so I sent the PR. My guess is because the csproj is an output exe instead of dll, so the transitive references are skipped over.

@AndreyAkinshin
Copy link
Member

@timcassell I believe we should understand the source of the problem before merging any fixes. Could you please share a repro?

@timcassell
Copy link
Collaborator Author

Could you please share a repro?

Can you repro with my description in #2344? I didn't do anything fancy, just a simple BenchmarkRunner.Run<Benchmarks>();.

@AndreyAkinshin
Copy link
Member

@timcassell this issue is investigated, see my report in #2344 (comment)

@timcassell timcassell force-pushed the csproj-packagereference branch 2 times, most recently from f4c02c1 to 66a04c4 Compare June 30, 2023 21:13
@timcassell timcassell force-pushed the csproj-packagereference branch from 66a04c4 to e31672a Compare June 30, 2023 21:17
@timcassell
Parse CsProj with XmlDocument instead of TextReader.
fd937b4 
Added `PackageReference` to `SettingsWeWantToCopy`.
@timcassell timcassell mentioned this pull request Jul 1, 2023
Closed
@AndreyAkinshin AndreyAkinshin merged commit a519b6c into dotnet:master Jul 3, 2023
@AndreyAkinshin
Copy link
Member

@timcassell thanks!

@AndreyAkinshin AndreyAkinshin added this to the v0.13.6 milestone Jul 3, 2023
@timcassell timcassell deleted the csproj-packagereference branch July 3, 2023 10:10
@radical
Copy link
Member

radical commented Jul 12, 2023

This breaks if the Version attribute of the PackageReference uses a property, as the property is never copied.

@timcassell
Copy link
Collaborator Author

This breaks if the Version attribute of the PackageReference uses a property, as the property is never copied.

Yeah... that was reported in #2364

@cincuranet cincuranet mentioned this pull request Jul 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreyAkinshin AndreyAkinshin AndreyAkinshin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.13.6
3 participants
@timcassell @AndreyAkinshin @radical