-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
MSBuild properties for hosted deployment options #26620
Conversation
Thank you @guardrex for taking a look. Firstly, the wording of the added section seems very comprehensible to me. There is a part of a sentence missing in the second paragraph or confusing wording though:
I think it should be either "both a framework-dependent executable and for a specific platform" or just remove the "both" (or I'm missing something). I tried removing setting the
I am very curious of this issue, so I tried modifying my CI build. On Ubuntu, this error breaks the build:
The issue itself seems very weirdly specific. With the goal of having a plaftorm-specific build that is not self-contained:
I'm not sure the reasoning of the proposed doc change is totally correct because of the behavior of the build. Seemingly the only combination that works is setting up The wording when building on Ubuntu is also different than what's happening locally on Windows. The Windows error doesn't even hint at the problem, it seems that two path variables are empty that probably shouldn't be at that point in the build. I didn't test it on other machines, so it might be worth checking out if someone's trying to tackle the issue. |
Right ... sorry about that sentence. Yes, I had it structured differently right before one of the latter commits and didn't edit the whole sentence properly.
Indeed, that's why I was asking. I didn't try to test the scenarios because Javier's explanation seemed to indicate that any MSBuild property misbehaving through the option could be passed via the MSBuild property option ... and that's apparently not the case. It would have been simpler to document. I'm going OOF for a bit, but I'll be back in a few hours to work further on this. I might go ahead with a round of local testing myself just to see the same behaviors that you're seeing. I'll ping u back later today or on Thursday. Thanks for looking and helping with the coverage on this scenario. |
I ran a series of tests, and I concur with (assuming the RID is passed as the MSBuild property (
Therefore ...
We agree on the configuration that should be stated in the new section, so I'll work on updating that now. I'll ping you back when it's ready for another look 👀. |
Ok, @yugabe ... try it now. This says what to do but not WHY, since we don't cleanly know why. Javier's remark about the RID is fine, but it clearly doesn't apply to the self-contained config. Therefore ... imo ... Javier's reasoning can't be stated as a blanket reason for why any given option may fail. In this case, BOTH WRT pinging Artak and Dan, I changed my mind on that. They're 🏃😅 for the .NET 7 release. As long as we have the guidance correct, I think we can roll with it and take further feedback on it. I'll wait until u provide feedback. Based on changes or not, we'll merge when we're happy with the coverage. |
Considering the alternative (having to go deep into the bowels of the CLI, Blazor publishing and MSBuild), I think this will have to do. Maybe one piece of information worth adding is publishing via Visual Studio works by using a publish profile, as it implicitly sets the
Also, maybe a direct link to /aspnet/core/host-and-deploy/ would be helpful as well in the closing paragraph if someone stumbles upon this. On a sidenote, my original issue is essentially resolved, and as I understand you couldn't reproduce my error message, but I wonder where that was coming from. If I got the same error message that you did ( The reasoning has to be along the lines that the Blazor WASM application can be nothing else than a self-contained app: in the browser, the app won't be able to access any .NET runtime on the host, so it has to come with the app. On the other hand, making the Server (which also includes a different, not WASM-compiled assembly of the Client, because it is referenced; enabling prerendering and such is only possible this way) framework-dependent requires that instance of the Client build be framework-dependent too. Setting the Client to Most probably because of this, the build breaks at different points, because two Client builds need to be built with different configurations. This behavior is probably undefined, and breaks the build in these cases. |
@yugabe ... I tried to shoehorn the VS pieces into this. I think it's ok. The alternative would've been to split out the VS bits, but that would make it longer. See if the latest round of updates sound good 👂. |
I think it's good as is. Thank you for your work. |
Actually, I think I will edit it again ... the sentences are a bit long and complex for machine translation into other languages. |
All the hidden things that make it worthwhile... I never would have even considered that an issue, but you're right of course. |
Ok ... I like that first bullet better for the self-contained setup. It will probably translate better this way as a list than in a L...O....N....G sentence. |
IIRC, there was a way to pass things to the |
I thought it was in the Build properties of the project, but I don't see it there. Naaaah! ... Doesn't matter. I don't know where that box is or if they removed it from VS. Anyway, the guidance on the PR now should be correct. I'm ready to merge this if you are. |
I was under the impression that the publish profile was only picked up by MSBuild and not by the dotnet CLI, but maybe I missed something regarding passing arguments through. Mixing in publishing with Visual Studio messed up the steps a bit. To be clear, when not using the CLI and only going through the VS dialogs, all's well, no errors whatsoever. This is in conflict with the two bullet points a bit. If I understand right:
The current structure seems to imply that setting it via publish profile might work with the CLI. |
Yes, I think you're correct that it will need to be split out because we will NOT further go into the details of indicating the publish profile via the CLI, which is possible to do with I'll return to this either later this afternoon or on Friday morning. Perhaps, my .... er ... FOURTH attempt 🙈 will be fruitful. |
I'm sorry this turned out to be such a pain, but I guess it's still better to document it properly than fix whatever constellation is causing this under the hood... I wouldn't even know where to start... |
No worries. Yeah, it will work out in the end. Due to the complexities of multiple tools and multiple approaches with those tools, this is a bit challenging to get just right, but I think I see what needs to be done now to finish it. WRT the why aspects on RID versus self-contained, I might ask Javier later ... much later ... like EOY or 23Q1 ... to look at this section and see if he wants to make any why remarks. I'll add that to one of my tracking issues now so that I don't forget. |
@yugabe ... I split out the VS pieces, and I decided to add a NOTE on using the publish profile with the See if I missed anything 🤞. |
Ha, yes, I see you couldn't resist to leave that part out 😁 I think this is complete, at least as far as our knowledge permits at the moment. I do "approve" (I'm not sure I should approve in the GitHub PR, I guess someone from the team will do that). In the meantime, I convinced myself that my reasoning was correct regarding the Client build having to be built for both a self-contained browser-wasm target and a framework-dependent, parameterized RID target being the root cause of the problems. Of course, this is in no way a correct response for the WHY, but it might be a good starting point when this issue resurfaces in the future. |
Ok, cool. Thanks for your help with this. Yeah, I'll catch up with Javier later on the WHY and see if he wants to add anything. No need for additional approval/sign-offs. I just need to take a vote here on it ... Here ye! Here ye! Motion to publishing the PR is on the floor. All in favor of publishing this PR? 😆They told me that if I screw up too many PRs that they'll transfer me to Microsoft's Typewriter Maintenance Division! Yikes! |
Fixes #26619
@yugabe ... Are you free to take a look 👀 at this?
btw -- You'll see three copies of the section. You only need to look at the first because they're identical. We duplicate them to cover different release versions in the same article.
The new section for the WASM deployment doc seeks to address both the general scenario for any given MSBuild property that's applied to both projects of a hosted WASM solution and specifically for your use case scenario (framework-dependent executable for a specific platform), which I show as the example.
You (and Javier) didn't state that there was a limitation on the self-contained deployment option (as
false
) to the project file. You did state that ...... but you may have meant by way of
--no-self-contained
, which seems like it would 💥 based on Javier's explanation. I came away with the impression thatSelfContained
can also be stated as an MSBuild option (/p:
) in a similar fashion to the RID to meet your needs ...However, let me know if you tried that in your testing and it failed. If so, then we'll need to figure that out as an additional complication here. I may need to work on this a bit further with Javier's help. I'm trying to avoid pinging him due to the high .NET 7 product unit workload at this time. If this all looks good, then we can proceed. I'll take further reader feedback on it.