-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add support for autogenerated binding redirects that are not source controlled #73
Comments
@julealgon This has been tried by various people, but there are a large number of downsides to this that have been encountered.
If you can get a reliable system in place for this, I'm open to a PR to add the additional mode, but I feel there would have to be a lot of documentation about the drawbacks and limitations. If you are working in a multi-developer scenario, I would hope that you are running unit tests before checking in the code/merging changes. You could set up a check to prevent accepting PRs if the build results in a binding issue. |
I don't have a great solution to this at the moment either, but the #1 problem here is over redirecting because of the commits, as well as cherry-picking back to release branches. Both of these get pretty painful. For example if you have a package that depends on X and causes a redirect for X because it ultimately bumped X to say 5.0.0, we get a redirect. That redirect is then committed to source control. Package gets updated, drops the need for X, the redirect to 5.0.0, which isn't the version we're at anymore (say it's back to 2.0.0). If no redirect is needed at all (everything else needed 2.0.0), everything is happy but the redirect is wrong and we get a runtime boom. Compared to any app.config where everything is generated to the output directory, but not checked in...this all just works, with the right redirects. But I take some the above points, doing this only in publish (basically generate, without erroring in the build) may not be for everyone. Just adding some thoughts in case it adds ideas here, I'd love to make this work as it's one of the main pain points we have left in builds/runtime at all. |
A stopgap idea might be to create a build target that runs only during a There might be one other possibility that I haven't seen yet mentioned as a solution to this situation, and that is to "treat" the project root The Pro's:
The Con's:
I haven't yet built out this idea robust enough to offer to include it with this project type, so I don't have a ready-made solution for you. But perhaps in your situation you can mitigate the negatives in order to gain the benefits. Some combination of ideas (t4 generation or TransfromXML build steps) will allow you to generate the |
Is anyone looking any further at this mechanism? I'm not adversed to having it as an option, but I don't know if there is a huge gain from the amount of work required to implement it. |
Not on my side. I was hoping someone else with more experience on MSBuild and the standard targets would take a stab at it.
I'd absolutely love for this to remain open to at least signal it is open for grabs by the community. I think it is unlikely I'll be able to dedicate any time to it myself due to priorities in the company, but this does impact us directly quite a bit, and there have been many situations already since we migrated that developers ended up merging code without rebuilding the affected projects locally after a package update that ended up generating a diff on all other devs' machines later and required a patch PR. Having this autogenerated on build would completely eliminate that concern. I can only see this same situation happening again and again with some of our projects as they have tons of dependencies that have to be upgraded fairly often which affect binding redirects. Having to build a project after a package upgrade is not a normal requirement so it naturally becomes hard to enforce. In the meantime, we might consider adding something to our build pipeline that detects the generation of any pending changes in the build agent workspace and fail the build when that happens though. |
@julealgon Having to (build and) run the project and tests after upgrading packages to ensure that nothing breaks should be a normal requirement, so I am not entirely in agreement with that. I would recommend using gated check-ins for PRs which break if there are any binding redirects detected. It does feel like this might be slightly outside the scope of the SDK though and might be something that could be in another package brought in to do just this, if indeed it should be an MSBuild thing, rather than a CI/CD thing, as this feels more like process and team specific than the SDK itself. As I said before though - if someone can create something lightweight and robust, and put it in a PR behind a flag of some sort, I'll happily add it in. |
Oh, to be clear, I'm not challenging that notion at all... I was just arguing that enforcing that behavior is harder when the build can generate further changes that are source controlled. Developers don't expect that, and they might've already pushed their changes before testing and end up merging the pr and disregarding their extra local changes. I've also hit situations in the recent past where the next build right after a package update would, for whatever reason, not update the binding redirects. Then I'd go ahead, test and merge, and on another build, when working on the next task, they would be updated "out of nowhere". I'm not sure if there is an associated bug there but I have no idea how to repro this. I suspect it might've happened with some of my colleagues as well and that's where some of the patch PRs were necessary.
Yeah, this is likely what I'll pursue as we migrate from our very legacy and inflexible build process to Github Actions.
Interesting. I'm not very fond of the CI process generating commits though, but thanks for sharing the idea.
That's fair, but I disagree that this is not a concern of the SDK. To me it is a direct responsibility of it since it is the one that generates the binding redirects in the first place. This is highly debatable though, so I won't push on this point further.
👍 |
I've been using this SDK for a while now in a bunch of our legacy projects to great success, however there is one pain point which I wanted to document here in the form of a proposal.
We are leveraging
GeneratedBindingRedirectsAction
asOverwrite
in our projects. Whenever packages are updated in our solution, we need to make sure to rebuild it fully so that binding redirects in the legacy projects are adjusted as needed, before we can commit the changes. On our solution, this is a tedious process as it takes several minutes to build the entire project. It is also error prone, since if we don't perform this step, it ends up generating pending changes for other developers later, which is not something we want of course.For modern projects, this issue has been resolved as binding redirects are "magically" taken care of: they never show up and are always up-to-date without the developer needing to do anything extra. I'd like an experience similar to that with this SDK.
Would it be possible to generate the binding redirects in a way that keeps them "decoupled" from the main
web.config
in such a way that the file can be generated and linked at build time, but doesn't need to be merged into source control? This would allow us to update the packages and don't have to worry about manually refreshing the binding redirects with a local build right after. No matter where the project was built, it would generate the redirects (probably in a separate file) and link it with the originalweb.config
, using, say, a configSource path.The text was updated successfully, but these errors were encountered: