search

fsprojects / Paket

A dependency manager for .NET with support for NuGet packages and Git repositories.
https://fsprojects.github.io/Paket/
MIT License
1.99k stars 520 forks source link

"redirects: off" still sets AutoGenerateBindingRedirects to true #4158

Open jmeland opened 2 years ago

jmeland commented 2 years ago

Description

The paket install command keeps trying to change the AutoGenerateBindingRedirects flag to true in .NET Framework project files, even when explicitly told to not use redirects. This happens on both the initial install command and any subsequent ones.

Repro steps

  1. Clone the repo found at https://github.com/jmeland/PaketBindingRedirects - note how line 13 of the project file is \<AutoGenerateBindingRedirects>false\</AutoGenerateBindingRedirects>
  2. Go to the folder at the level of the solution/dependencies/lock files in a command prompt.
  3. Run .paket\paket.exe install. (Substitute your own copy of the bootstrapper in magic mode for paket.exe if desired, but make sure to put it in the same location as mine is, and remove mine if you're using your own.)
  4. Observe line 13 of the project file again.

Expected behavior

I expect the AutoGenerateBindingRedirects flag to remain false when specifying redirects: off, meaning line 13 of the project file should not change at all, and if it's not the initial install command, the project shouldn't need to get checked out since nothing changed.

Actual behavior

Paket has changed the flag to true, making the line become \<AutoGenerateBindingRedirects>true\</AutoGenerateBindingRedirects>. This also constitutes a change to the file, leading to Git checking the project out. In a private and much larger repo using TFS where this issue was discovered, attempting such a modification without having already checked the project out will cause Paket to error on the project file (read-only when not checked out from TFS), stopping it from making this modification or any modifications that would have been done after it!

Known workarounds

Specifying redirects: off at the group level is definitely not a workaround, even though it seemed like the ideal way to solve this entirely. No functioning workaround is currently known.

jcmrva commented 2 years ago

I would think binding redirects should be off in .NET (Core) by default, but I can confirm this behavior. Also, it will add <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects> if that property is missing.

I noticed it adds REDIRECTS: OFF to paket.lock but I'm not sure if that does anything.

jmeland commented 2 years ago

I would think binding redirects should be off in .NET (Core) by default, but I can confirm this behavior. Also, it will add <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects> if that property is missing.

I noticed it adds REDIRECTS: OFF to paket.lock but I'm not sure if that does anything.

This is not .NET Core, this is .NET Framework. It has binding redirects true by default, but you can turn them off by going into Properties for the project file, or by manually editing it using an external editor.

jcmrva commented 2 years ago

Whoops, I didn't read that closely. But Paket seems to be working the same for both here?

jmeland commented 2 years ago

Yeah, the behavior you described for .NET Core matches what happens for .NET Framework, aside from the part about binding redirects being off by default. For my use case, the main issue is .NET Framework, since a decision was made to leave any .NET Core projects in the private larger repo alone.

jmeland commented 1 year ago

Just checking in again, Paket team. This is still happening and still an issue for my use case, and the only response I got failed to resolve my issue, followed by a long period of inactivity. Again, despite saying redirects: off for all groups in the paket.dependencies file, it tries to set AutoGenerateBindingRedirects to true in .Net Framework (NOT .Net Core) projects. Could the team take another look over this?

DalekBaldwin commented 1 year ago

This happens in .Net Core/5.0+ as well.

Ironically it seems this started happening again in version 7.1.3, whose rationale was "Do not set binding redirects all the time".

jmeland commented 1 year ago

Can this be assigned to be fixed? A rough estimate of the timeframe for a fix would be nice as well.

isaacabraham commented 1 year ago

Hey @jmeland . I can't speak for the rest of the Paket team - but currently this isn't a blocker for my folks and I'm not sure when we're have time to look into this.

I'd probably be able to allocate some time to review a PR that fixed the issue and had some automated tests alongside it though - just let me know if that's of interest to you.

Thanks!

DalekBaldwin commented 1 year ago

I expect it's not a blocker for anyone really, but I'm willing to bet there are a plenty of teams who have this experience:

A dev checks out a branch and starts their work by running paket install instead of paket restore, because hey, it's at least as thorough as a restore, right? A bunch of <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects> elements end up strewn about the project files. Sometimes they remember to revert them, sometimes a code reviewer catches them, but often they get checked in, then removed as cleanup work in a future PR, re-added, removed, re-added, removed...

hyazinthh commented 1 year ago

This still happens, a workaround is to use Paket <= 7.0.2

Credit goes to: https://github.com/fsprojects/Paket/issues/4139#issuecomment-1289145487