Fix SingleFile regression in client configuration

[am11]

This PR attempts to fix a 3.1 to 5.0 observable regression in ConfigurationManager APIs.

Starting with 5.0System.Reflection.Assembly.GetEntryAssembly().ManifestModule.Name in app published with PublishSingleFile=True returns <Unknown> string.
With 3.1 it returns a pseudoname: MyApp.dll with the singlefile executable with name MyApp (without .dll).

This change adjusts ClientConfigPaths implementation such that it stays compatible with 3.1 behavior.

Fixes #42161

cc @vitek-karas, @jkotas, I tested locally that it fixes the reported 3.1 -> 5.0 regression in #42161; but I am not sure if there is enough infra to exercise SingleFile publish integration test.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[ghost]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[ghost]

Tagging subscribers to this area: @safern
See info in area-owners.md if you want to be subscribed.

[am11]

ping - if there are no other comments, can this be merged?

[safern]

LGTM

[safern]

I think it might be to late to get it backported to 5.0.0 but I'll ask @ericstj and @danmosemsft

[danmoseley]

We are close to "would we service for this" bar now. @ericstj ?

[ericstj]

I don’t have a good read on bar for single file scenario. This change is not without risk. @agocke @jeffschwMSFT do you support this?

[agocke]

My initial feeling is we should not risk destablizing non-single-file scenario for the single-file scenario unless this is really critical.

So here's my question: this change was really made to support ConfigurationManager. How prevalent is that? Is this going to hit a large portion of apps within 5 minutes? Is there any workaround?

[am11]

This change is not without risk.

Yes, although risk is not high. We have a good test coverage of client configuration. We just don't have tests for signlefile form factor, it was not the intentional breakage.

ConfigurationManager. How prevalent is that?

I think fair to say; not any less prevalent than [m]any other fixes being backported to 5.0.

it seems these single file regressions were not intended by this change: #40974. It happened due to lack of single file test infra and made known when the user reported in #42161.

[ericstj]

I think fair to say; not any less prevalent than [m]any other fixes being backported to 5.0.

Not so. ConfigurationManager is just a .NETFramework compat assembly. We don’t reccomend it’s use for new code. It is not part of the shared framework. Question for anyone consuming this: do you actually have an end to end that works well in single file? @mshobohm can you let us know?

[ericstj]

@JeremyKuhne @merriemcgaw do you think this would be important for Windows Forms apps using Single File? I know WinForms has some Settings integration that other components do not.

[mshobohm]

For me the bug fix in 6.0 was to late, I went life with 5.0 rc1,publishsinglefile and a simple workaround:

Convert App.config to myAppname.Settings.json

using System.Text.json;
public static class xxx ...
static Dictionary<String, String> settings=null;      
        // centralized access to ConfigurationManager
        public static String AppSetting(String setting)
        {
           if (settings ==null)
            {
                var jsontext = File.ReadAllText("<myAppname>Settings.json");
                settings = JsonSerializer.Deserialize<Dictionary<String, String>>(jsontext);               
            }
            if (settings.ContainsKey(setting))
            return settings[setting];
            else
                return setting;
            //return ConfigurationManager.AppSettings[setting];
        }

---

ConfigurationManager (in my opinion) has 2 disadvantages:

1. The rename step of App.config with assembyname.dll.config (lack of transparency)
2. The rename step of ConfigurationManager.OpenExeConfiguration didn't help me with this issue.

Keep it simple keep it stupid (KISS) would be to give the (relative or Full) Path of the config file not the path of the exe-file
The main reason for the usage of PublishSingleFile was the saving of filespace.
I wonder why you cant fix it with 5.0.1 or 5.0.2 while 6.0 will arrive in November 2021 ?

[ericstj]

In terms of simplicity I think your workaround approach is better for simple immutable application configuration. Another option might be Microsoft.Extensions.Configuration.

System.Configuration is a legacy API that carries the overhead of the desktop scenarios that are no longer interesting or fully supported, eg: strongly-typed configuration classes, layered machine/user/app config, XML, etc.

why you cant fix it with 5.0.1 or 5.0.2

We have a high-bar for the product right now as we approach GA, and an even higher bar in servicing. We need to justify the importance of fixes because we have to weigh the benefit of the fix over the risk of breaking other customers. It's especially important when we look at a fix in a legacy component like this, where a lot of people are using specifically because they don't/can't touch the old code that is using it. Taking it in 6.0 gives us a longer bake time to discover potential issues with the fix before it released.

[JeremyKuhne]

do you think this would be important for Windows Forms apps using Single File? I know WinForms has some Settings integration that other components do not.

Yes, but I'm not clear on exactly how important. Users are definitely still using AppSettings for their ported (or hopefully soon-to-be-ported) apps and Design relies on IPersistComponentSettings. The change looks like fairly low risk to the non-single file scenario, fwiw.