search

tom-englert / Wax

An interactive editor for WiX setup projects.
MIT License
219 stars 25 forks source link

Fix duplicated references #27

Closed Leogiciel closed 7 years ago

Leogiciel commented 7 years ago

Fix for #26 : Add a GroupBy on FileName to fix duplicated second-tier references

Fix for #22 : This is how I handled ASP.Net architecture, and now Wax behaves exactly how needed

tom-englert commented 7 years ago

GroupBy followed by Select(p => p.First()) is the same as Distinct(), which is already used by the calling methods, so this should be superfluous here.

Leogiciel commented 7 years ago

They are distinct on the file path, not on the filename.

GroupBy(p=>p.FileName).Select(gp=>gp.First()) permit to do a "DistinctBy" approach.

tom-englert commented 7 years ago

This PR reverts all the code added during the last weeks, so I don't think it's a good idea to merge it.

It would be more helpful if you could use the tip of the main branch and create issues only for things not working there.

tom-englert commented 7 years ago

Yes, the distinct on the file path is intended. What is not working?

Also it's not a good idea to start adding workarounds and leave alone wrong code. If you think the distinct is wrong, fix it, not add another one at a different place.

tom-englert commented 7 years ago

Distinct on the file name is definitely wrong, e.g. all localized resource dlls will have the same file name, but are stored in a different folder each.

Leogiciel commented 7 years ago

It's the only place where we can implement a Distinct on FileName, because some references can come from GetBuildFiles and from GetLocalFileReferences. The only way to avoid that is to make the distinct on FileName after the Concat.

Here is an example :

capture

Leogiciel commented 7 years ago

Ok you're right for the resources... So we have to make a distinct on file destination, not on file source !

Leogiciel commented 7 years ago

NB1 : The PR only reverts changeset b49c60a because we worked on the same bug at the same time, and I implemented my fix in an other way than yours.

Your fix is only partial, because second-tier references are not placed into "bin" output, and can be duplicated.

NB2 : each bug I opened is active on your main branch, maybe my API project contains specificities (eg : references) which disable you to reproduce them.

tom-englert commented 7 years ago

Then you did not try the latest version, second-tier references are already fixed. I also have changed ProjectOutput to use the target name, so this should work now as expected.

Leogiciel commented 7 years ago

I did dude...

I'll reproduce them immediately on your branch and update the issues with screens.

tom-englert commented 7 years ago

Yes, a "how to reproduce" would be very helpful!

Leogiciel commented 7 years ago

I just saw your last commit, sorry. It just fixed duplicated references, and some of the duplicated which were going to TargetDir and not "bin". Well done.

The missing ComponentGroupRef is still active, I close the other twos and the PR.

Thank you !