WixSharp.Files generation : same folder on different features

[manytwo]

Is it possible to improve Files resolution to create sub WiwSharp.Dir only if not exist?

See this simple use case:

Folder structure:

├─ Main
|	└─ run.exe
├─ Feature1
|	└─ Docs
|	       └─ file.txt
├─ Feature2
|	└─ Docs
|	       └─ file.txt

WixSharp Code:

var main = new Feature("Main");
var feature1 = new Feature("Feature1");
var feature2 = new Feature("Feature2");
...
new Dir(main, @"%ProgramFiles%\My company\My product",
  new Files(main, @"Main\*"			  
   ...
   new Dir(common,"Files",
	...
	new Files(feature1, @"Files\Feature1\*"),
	new Files(feature2, @"Files\Feature2\*")
   )                            
 );

This doesn't work, because "Docs" folder and "file.txt" are created twice.

[manytwo]

I'm not very familiar with fork/pull request, and I prefer to have your opinion.

I did a modification to make it work, but I don't know if it impact something or if keep existing folder should not be done for some reason.

First, we need to get parent Wix.Dir from Project.ResolveWildCards to Files.GetAllItems

Files.cs:

 public WixEntity[] GetAllItems(string baseDirectory, Dir parentWixDir)

Project.cs:

public Project ResolveWildCards(bool ignoreEmptyDirectories = false)
 {
int iterator = 0;
var dirList = new List<Dir>();
var fileList = new List<File>();

 dirList.AddRange(Dirs);

while (iterator < dirList.Count)
 {
    foreach (Files dirItems in dirList[iterator].FileCollections)
         foreach (WixEntity item in dirItems.GetAllItems(SourceBaseDir, dirList[iterator]))
...

I can so check on Files.GetAllItems if parent dir already contains subdir based on name:

var subDir = parentDir.Dirs.FirstOrDefault(dir => string.Compare(dir.Name,dirName,ignoreCase: true) == 0 );
       if (subDir == null)
       {
            subDir = new Dir(this.Feature, dirName, new DirFiles(IO.Path.Combine(subDirPath, this.IncludeMask)) ...                                       

this modification was done on AgregateSubDirs delegate and on sub dir resolution

using System.Linq;
...
public WixEntity[] GetAllItems(string baseDirectory, Dir parentWixDir = null)
{
if (IO.Path.IsPathRooted(Directory))
    baseDirectory = Directory;
if (baseDirectory.IsEmpty())
    baseDirectory = Environment.CurrentDirectory;

baseDirectory = IO.Path.GetFullPath(baseDirectory);

string rootDirPath;
if (IO.Path.IsPathRooted(Directory))
    rootDirPath = Directory;
else
    rootDirPath = Utils.PathCombine(baseDirectory, Directory);

Action<Dir, string> AgregateSubDirs = null;
AgregateSubDirs = delegate (Dir parentDir, string dirPath)
                    {
                        foreach (var subDirPath in IO.Directory.GetDirectories(dirPath))
                        {
                            var dirName = IO.Path.GetFileName(subDirPath);

                            var subDir = parentDir.Dirs.FirstOrDefault(dir => string.Compare(dir.Name,dirName,ignoreCase: true) == 0 );
                            if (subDir == null)
                            {
                                subDir = new Dir(this.Feature, dirName, new DirFiles(IO.Path.Combine(subDirPath, this.IncludeMask))
                                {
                                    Feature = this.Feature,
                                    AttributesDefinition = this.AttributesDefinition,
                                    Attributes = this.Attributes,
                                    Filter = this.Filter
                                });
                            }
                            AgregateSubDirs(subDir, subDirPath);

                            parentDir.Dirs = parentDir.Dirs.Add(subDir);
                        }
                    };


var items = new List<WixEntity>
{
    new DirFiles(IO.Path.Combine(rootDirPath, this.IncludeMask))
    {
        Feature = this.Feature,
        AttributesDefinition = this.AttributesDefinition,
        Attributes = this.Attributes.Clone(),
        Filter = this.Filter
    }
};



if (!IO.Directory.Exists(rootDirPath))
    throw new IO.DirectoryNotFoundException(rootDirPath);

foreach (var subDirPath in System.IO.Directory.GetDirectories(rootDirPath))
{
    var dirName = IO.Path.GetFileName(subDirPath);
    var subDir = parentWixDir.Dirs.FirstOrDefault(dir => string.Compare(dir.Name, dirName, ignoreCase: true) == 0);
    if (subDir == null)
    {
        subDir = new Dir(this.Feature, dirName, new DirFiles(IO.Path.Combine(subDirPath, this.IncludeMask))
        {
            Feature = this.Feature,
            AttributesDefinition = this.AttributesDefinition,
            Attributes = this.Attributes.Clone(),
            Filter = this.Filter
        });
    }

    AgregateSubDirs(subDir, subDirPath);

    items.Add(subDir);
}

return items.ToArray();
}

And finaly In resolve wilcards the WixSharp.Dir is added to full list only if doesn't exist:

else if (item is Dir && !dirList[iterator].Dirs.Contains(item))
        dirList[iterator].Dirs = dirList[iterator].Dirs.Add(item as Dir);

Whole code:

        public Project ResolveWildCards(bool ignoreEmptyDirectories = false)
        {
            int iterator = 0;
            var dirList = new List<Dir>();
            var fileList = new List<File>();

            dirList.AddRange(Dirs);

            while (iterator < dirList.Count)
            {
                foreach (Files dirItems in dirList[iterator].FileCollections)
                    foreach (WixEntity item in dirItems.GetAllItems(SourceBaseDir, dirList[iterator]))
                        if (item is DirFiles)
                            dirList[iterator].DirFileCollections = dirList[iterator].DirFileCollections.Add(item as DirFiles);
                        else if (item is Dir && !dirList[iterator].Dirs.Contains(item))
                            dirList[iterator].Dirs = dirList[iterator].Dirs.Add(item as Dir);

                foreach (Dir dir in dirList[iterator].Dirs)
                    dirList.Add(dir);

                foreach (DirFiles coll in dirList[iterator].DirFileCollections)
                    dirList[iterator].Files = dirList[iterator].Files.Combine<File>(coll.GetFiles(SourceBaseDir));

                //clear resolved collections
                dirList[iterator].FileCollections = new Files[0];
                dirList[iterator].DirFileCollections = new DirFiles[0];

                iterator++;
            }

            if (ignoreEmptyDirectories)
            {
                var emptyDirs = AllDirs.Where(d => !d.Files.Any() && !d.Dirs.Any());

                emptyDirs.ForEach(emptyDir => AllDirs.ForEach(d =>
                                              {
                                                  if (d.Dirs.Contains(emptyDir))
                                                      d.Dirs = d.Dirs.Where(x => x != emptyDir).ToArray();
                                              }));
            }

            return this;
        }

I have only manually tested it, and it is working.
I can try to do a pull request and unit tests if you want, I must take a look at yours, but as I said, I m not familiar with GitHub pull request.

[oleg-shilo]

Hi there. Thank you for your contribution.
I think the original problem is caused by the duplicated file IDs after resolving wild cards. This in fact should not happen as the the ID auto-allocation algorithm should normally ensure uniqueness of the all IDs. I think it is failing in case of Files (while it works for File).

I will have a look at this and after the fix you you may not even need to merge your changes. And if your fix is still required then I will help you to create a pull request (or just merge your code directly into master branch).

In any case I suggest that you wait for me (may be day or two) to let you know my findings.

[manytwo]

Hi, no problem :)
You right the error displayed is a duplicated id error.
But if you look at .wxs, you can see "Docs" dir is created twice, even if we can generate different ids it must be created once no? It is same folder, should it be referenced on each feature (I m pretty new on wix)?

Reuse existing folder also cover case where WixSharp.Files resolves same folder with same feature :
in my previous sample replace

new Files(feature1, @"Files\Feature1\*"),
new Files(feature2, @"Files\Feature2\*")

with

new Files(main, @"Files\Feature1\*"),
new Files(main, @"Files\Feature2\*")

(and also rename one of "file.txt" or supress ICE30 validation)

It is a "developper-friendly" use case, we can merge folders and create wix required folder structure before, with Jenkins for exemple.

[oleg-shilo]

Sorry it took me so long to come back to this one.

I had a look at your fix and it seems simple enough. Though it only addresses one problem. Removal of the duplicated Directory elements . It's good as it removes the ICE warning but it also leaves the Features associations broken. Thus a file from that single dir is only associated with a single feature, while your definition associated it with two features.

It's not caused by your fix. It is a current limitation of the Wix# wild card support. Thus the direct inclusion of the same files doesn't cause the Features problem problem but ICE30 only.

Unfortunately I have to to think abut a proper solution for the File-to-Feature relationship, which is in fact N-to-N. Rolling out your fix as it stand will hide the warning but trigger another hidden problem.

Will let you know when I progress with this one.

---

A few useful things to share. :)
When you want to suppress ICE30 warning you can just pass the light options for that

project.LightOptions += "-sice:ICE30";

When comparing the strings you use SameAs extension:

var subDir = parentDir.Dirs.FirstOrDefault(dir => dir.Name.SameAs(dirName, ignoreCase: true));

[manytwo]

Ok, thanks for infos. Sorry I didn't investigate all cases, I was focused on resolving my scenario and I didn't think about others cases.

I m working without ICE30 validation from the begining (my project have features which override base files), but even without ICE30 validation, I can't resolve my sample. The id Files.Docs is duplicated.

So if I understand, WixSharp use one feature by file/folder, and if same file is resolved by an other feature, it is considered as an other component at wxs generation. But wix can technicaly use same component in different features, which is not the design of wixsharp.

The WixSharp approach should work, but if I look at wxs generated by my sample, files ids (and components) are well generated (Component.file.txt and Component.file.txt.1), but dirs ids are not (Files.Docs exists twice, should create Files.Docs.1 ?). Same in <RemoveFolder ...>

Maybe generate different dirs ids can be a workaround?

If you really want to handle n-n relationship, I presume you must rewrite WixSharp.Dir and WixSharp.File with a Feature[] attribute, it is a hudge rework! If I can help for something, please say me what to do.

Do you think my fix is "useable" if I don't use same files in different features? I really need to resolve my example scenario :)

[oleg-shilo]

Maybe generate different dirs ids can be a workaround?

This is what I am also wondering. Wix# ID auto-allocation works really well but it doesn't cover Dirs. I just didn't think it was needed. And after today tests with your code I am leaning towards your approach with consolidating the unique dirs.

The best way to test is to generate the build batch file with project.BuildMsiCmd. This way you can modify wix manually and run batch again. When I fixed duplicated id's but left duplicated folders it still had some problems (including ICE30).

Thus I feel that your solution is good, yes it is "usable" :), even if it has a side effect and I would prefer to fix the side effect directly. Unless of course it becomes over complicated.

I presume you must rewrite WixSharp.Dir and WixSharp.File with a Feature[] attribute, it is a huge rework!

Yes it might be. This is what I meant "over complicated". And I really want to avoid this. The solution can be in aggregating the features prior the wxs generation:

class File: WixEntity
{  
    internal Feature[] AllFeatures;
    ...

// and then instead of 
new Dir("Tools",
       new File(AllToolsFeature, "logger.exe"),
       new File(LightToolsFeature, "logger.exe"),

// use this new API
new Dir("Tools",
       new File("logger.exe")
       {
           AllFeatures = new [] { AllToolsFeature , LightToolsFeature }
        ...

If the new API only involves new members but not new signatures than it is still OK. I really don't want to fiddle with the existing constructors.

Will need to sleep on it...

[oleg-shilo]

Done.

Actually it was smoother than I anticipated. Thanks to some beneficial design decisions made earlier the solution required minimal changes and the old API is still intact. The fix is effectively your proposed change combined with the extension of the Feature-to-WixObject relationship.

Thus one of your original code snippets will look like this:

var main = new Feature("Main");
var feature1 = new Feature("Feature1");
var feature2 = new Feature("Feature2");
...
new Dir(main, @"%ProgramFiles%\My company\My product",
   new Files(main, @"Main\*"			  
   ...
   new Dir("Files",
	...
	new Files(@"Files\Shared\*"),
	{
           Features = new [] { feature1, feature2 }
        }
   )                            
 );

Until the new release is available the new version assembly can be downloaded from here: https://github.com/oleg-shilo/wixsharp/blob/master/Source/src/WixSharp.Samples/WixSharp.dll

Will wait for a few days for feedback and then will push the new release.

[manytwo]

Well done! Sorry I was not able to use my computer for many days due to health problem.
I ll test it soon but I presume if you released it you have enougth feedbacks.
Glad to hear that doing changes was easier than excpected!

[oleg-shilo]

Don't worry. Use your time to recover.
I haven't received any other feedback on the fix, which may be a good sign :) Though you were the only one who discovered the problem.

For me releasing the fix was important as it addresses that bigger N-to-N issue. And if you still find that it is not fully addressing your issue we will proceed from there.

Get well mate.

[manytwo]

Thx man :) Sorry for delay, I m back to work and I lll test this this week.

[oleg-shilo]

No worries, and if there are any problems with the fix we will reopen the issue.

[manytwo]

I have tested it and my scenario doesn't work anymore.
I have done some modifications to support multi features.
I attached a test project if you want to check code and results.
Test-WixSharp_Files.zip

All components are referenced in features.
If installing feature 1 & feature 2, all files are well installed.
Feature 2 files with same name and location than feature 1 are overwritting feature1 files.
Empty directory is well installed.
It is hard to do unit tests on this because it need to be installed on system, I did not scripted it sorry.

I m reading again code posted on github but I m afraid that my last edit on 04/04/2017 post (patch code) was not taken... I probably failed something really sorry for that.
DirFiles resolution must be done even if subdir already exist.

I added features to dirs to support empty directories.

var subDir = parentDir.Dirs.FirstOrDefault(dir => dir.Name.SameAs(dirName, ignoreCase: true))?? new Dir(dirName);

foreach (var feature in this.ActualFeatures)
{
        if (!subDir.ActualFeatures.Contains(feature)) { subDir.Features = subDir.Features.Add(feature); }
}
subDir.AddDirFileCollection(new DirFiles(IO.Path.Combine(subDirPath, this.IncludeMask))
                                        {
                                            Feature = this.Feature,
                                            Features = this.Features,
                                            AttributesDefinition = this.AttributesDefinition,
                                            Attributes = this.Attributes,
                                            Filter = this.Filter
                                        });

[oleg-shilo]

Txs. I had to bring your changes manually not by pullrequest as I had to modify your changes to coexist with mine. Thus quite possibly I indeed missed some changes. I don't think you did anything wrong :)
Will have a look at it again.

[oleg-shilo]

OK, I had a look and indeed your code addresses the rest of the unresolved issues.

I only did a slight refactoring of your code:

static class CommonTasks
{
    static public Dir AddFeatures(this Dir dir, params Feature[] items)
    {
        ...

Dir subDir = parentDir.Dirs.FirstOrDefault(dir => dir.Name.SameAs(dirName, ignoreCase: true)) ?? new Dir(dirName);

subDir.AddFeatures(this.ActualFeatures);
subDir.AddDirFileCollection(...

Actually AddFeatures is something that I had (but forgot) to do when I was developing all these extensions.

The code is committed. And if you don't want to wait for the release then just grab the binaries from the Samples directory (https://github.com/oleg-shilo/wixsharp/tree/master/Source/src/WixSharp.Samples).

Thank you for such a high quality contribution.

[manytwo]

Thanks, WixSharp save a lot of work to me, so I can try to help a little, specially if I need something. It is well documented, well architetured, with few effort we can understand some parts of the code.

Continue your good job :)

[oleg-shilo]

👍 I am sincerely glad Wix# gained your appreciation.
Should we close this issue now or you need some more time to verify the fix?

[manytwo]

Ok I have tested it with some uses case: updates, modify / repair, with features switch (feature1 installed, upgrade only feature 2 etc)

All looks ok!
I just want to focus on some scenario, based on this described on this topic. But it is not an improvment required or a bug, just the normal behavior due to ICE30 validation suppression.
Can be usefull for some other users who ll read this.

├─ Main
|	└─ run.exe
├─ Feature1
|	└─ Docs
|	       └─ file.txt
|	       └─ f1_file2.txt
├─ Feature2
|	└─ Docs
|	       └─ file.txt
|	       └─ f2_file2.txt

new Files(feature1, @"Files\Feature1\*"),
new Files(feature2, @"Files\Feature2\*")

If a target file is same in 2 features (file.txt):

-Install/Update : the last file referenced will be the one selected.
if Install [feature1 & 2], result is file.txt [feature2 version], f1_file2.txt, f2_file2.txt

-Modify : if you change [feature 1 & 2 was installed] -> modify [feature 1 only] ,
the modification WILL NOT install file [feature1 version]. The result will be only f1_file2.txt.
The file file.txt is probably removed after feature 1 installation. You must change & repair to get all
your files.