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.

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

Only include web.$(Configuration).config where xxxx exists on disk. Fixes #67 #68

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

CZEMacLeod
Copy link
Owner

Ensures that web.*.config files automatically included in None for each configuration, actually exist on disk to prevent missing files showing in the solution tree and issues with TVFS.

This should resolve #67

@CZEMacLeod
Copy link
Owner Author

I'll be pulling in a few PRs together, including #67 at a minimum to make it easier to do releases.

@leusbj
Copy link
Contributor

leusbj commented Aug 2, 2023

@CZEMacLeod I know that I had originally proposed that the "Debug/Release/BindingRedirects" as None objects... but the "DependentUpon" property will also prevent these items from being included in the published/packaged site even if they are Content. As a note of interest, I noticed that Visual Studio MVC/WebAPI/SPA app templates use Content for the Web.Debug.config and Web.Release.config.

Is it worth just simplifying the globbing of .Config files to something like this that contains a wildcard... so it will only ever pull in files that do exist on disk... and then update ones that we believe should not be placed in the puiblished/packages web application

<ItemGroup Condition="'$(EnableWebFormsDefaultItems)'=='true'">
   <_WebConfigConfiguration Include="$(Configurations)" />
   <_WebConfigConfiguration Include="BindingRedirects" />

   <Content Include="Web*.config" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" />
   <Content Update="@(_WebConfigConfiguration->'Web.%(Identity).config')" >
      <DependentUpon Condition="EXISTS('Web.config')">Web.config</DependentUpon>
   </Content>    
</ItemGroup>

@CZEMacLeod
Copy link
Owner Author

CZEMacLeod commented Aug 2, 2023

@leusbj Interesting idea.
I did consider adding "BindingRedirects" to the configurations list, but I felt that it should be handled separately (even though the logic is basically the same).
I'm not sure if a WAP will ever not have a web.config file, but for the sake of completeness, I guess the DependentUpon Condition is a good idea.

I think I will use your idea of just using the DefaultItemExcludes and DefaultExcludesInProjectFolderproperties instead of introducing a new one withExcludeWebConfigConfiguration`.

Unfortunately, while I like the idea of using DefaultItemExcludes, we actually add web.*.config to this property and therefore cannot use it in the itemgroup.

I did feel that there may be other uses for the _WebConfiguration item group around applying transforms; I have some stuff in another project I use to replace slowcheetah, which applies multiple transforms based on the selected configuration and the local computer name.
e.g. if the configuration was Debug.SQL2019, and the PC was MyPC, it would look for (and apply in order), web.Debug.config, web.Debug.SQL2019.config, web.MyPC.config, and web.Debug.SQL2019.MyPC.config. allowing you to have customized settings for specific developer machines, and not repeat settings in each configuration that is 'derived' from another.
On a build server you can pass in a custom pc name such as BuildServer and apply say web.Release.config and web.Release.BuildServer.config.

All of that is out of scope for this PR though, but I wanted to explain my thought process.
Is there a performance, or other reason you feel that keeping these files as None is a problem?

@leusbj
Copy link
Contributor

leusbj commented Aug 2, 2023

@CZEMacLeod No performance reason. Just wasn't sure if treating them differently was actually important anymore... and if it wasn't there might be a straight forward approach to glob them all

@CZEMacLeod
Copy link
Owner Author

@leusbj The thing is, you want the web.*.config files which aren't related to the configuration to be content and included in the packaged outputs so must not have the DependentUpon set. This means that when you deploy them, they can be used by web deploy. So, you do need to handle the configuration and binding files differently.

@leusbj
Copy link
Contributor

leusbj commented Aug 2, 2023

@CZEMacLeod Sorry for confusion. My phrase "treating them differently", was geared towards they don't need to be in different ItemGroups. That they could all be Content and as long as the DependentUpon was set (or not set) correctly for the desired outcome, then everything would work.

  • Set and the package/publish process will be find them and apply the appropriate transformation at package-time, and EXCLUDE these items from being included in the package
  • UnSet and the package/publish process will NOT consider it as a transformation that can be applied at package-time, and INCLUDE it in the package for later Azure Release processes to use

In my mind the primary reason I had initially proposed putting the package/publish transformations as None was a "convenience" factor. That if for some reason, a project had a build configuration that exactly matched a release name... the developer could use the GUI Item Properties pane to switch the ItemGroup from None to Content. Then our logic, which automatically applies the DependentUpon property ONLY affects None Items, would ignore that item... and so that item would start to appear in the package for release pipelines to use later.

What you have looks to work (plus you have insight into the slow-cheetah ideas that I definitely didn't consider).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration-specific Web.configs added to project even if they don't exist
2 participants