wixtoolset / issues

WiX Toolset Issues Tracker
http://wixtoolset.org/
129 stars 36 forks source link

v3 to v4 conversion of a Merge Module isn't as pretty as a Package #7078

Open chrpai opened 1 year ago

chrpai commented 1 year ago

v4 preview 1 vs 17.4.1 hw 0.9.0.5

If you create a v3 product the WiX / Product / Package elements are nicely indendeted. If you give the Package element attributes normally found in v4 SummaryInformationStream and covert the project to v4 the WiX / Package / SummaryInformation elements are nicely indented.

The same isn't true with Merge Modules projects. It starts off nicely indented but after conversion the SummaryInformation element is on the same line with the Module element It typically runs off the screen due to the length and requires a manual formatting step.

Once RC1 comes out I'm going to have hundreds of projects to convert and I was hoping this might be a finishing touch that could be addressed.

example
robmen commented 1 year ago

@cpuwzd if you'd like to work on this issue. Leave a comment, then we can assign it to you.

chrpai commented 1 year ago

I'm willing to ride shotgun on it also. The testing scenarios are pretty straigt forward. I imagine this is in wixcop and that you'd run it through manually instead of through HeatWave?

cpuwzd commented 1 year ago

Rob, I am still interested in taking this. I've set up a test case, but I haven't started debugging it yet. Is this the best place for me to contact Chris?

barnson commented 1 year ago

@cpuwzd @chrpai If this is going to be a large-ish code change, look to get this in asap before -rc.3 in February.

cpuwzd commented 1 year ago

@barnson @chrpai @robmen I have analyzed this issue in great detail:

These issues should be discussed and the timing of such a change, if desired, should probably be postponed. This project might be larger than I would care to undertake.

barnson commented 1 year ago

I prefer the Go philosophy: Formatting isn't precious. https://go.dev/blog/gofmt

chrpai commented 1 year ago

Funny from a guy who rejected my PR because tabs vs spaces.

chrpai commented 1 year ago

Honestly I don't care. Let's not worry about it anymore.

rseanhall commented 1 year ago

If I understand the original request correctly, it was talking about this v3 code:

<Product ...>
  <Package ... />
</Product>

that wix convert turns into this v4 code:

<Package ...>
  <SummaryInformation ... />
</Package>

The bug was that this v3 code:

<Module ...>
  <Package ... />
</Module>

is converted into:

<Module ...><SummaryInformation ... />

instead of:

<Module ...>
  <SummaryInformation ... />

This doesn't sound like something that would be very difficult because it seems to be solved already with the Product -> Package conversion.

chrpai commented 1 year ago

@rseanhall Correct summation. I figured it would be straight forward but I don't know the code base.

I only raised it as a code hygiene issue. To make the diff easier to read in source control when I do the migrations. It's not the end of the world so I closed it. If someone wants to reopen it thats fine by me.

rseanhall commented 1 year ago

The Module's SummaryInformation element is created here: https://github.com/wixtoolset/wix4/blob/fb8903d9a185a020d54d2bb001f4331caf7c3825/src/wix/WixToolset.Converters/WixConverter.cs#L1299

The Package's SummaryInformation element is created here: https://github.com/wixtoolset/wix4/blob/fb8903d9a185a020d54d2bb001f4331caf7c3825/src/wix/WixToolset.Converters/WixConverter.cs#L1393

The tests for the Product->Package conversion is here, though it appears we don't have a test for the SummaryInformation element: https://github.com/wixtoolset/wix4/blob/fb8903d9a185a020d54d2bb001f4331caf7c3825/src/wix/test/WixToolsetTest.Converters/ProductPackageFixture.cs#L14

cpuwzd commented 1 year ago

@robmen @barnson @rseanhall @chrpai

I have a repo to be reviewed. I created a test that encapsulates Chris Painter's first example. I couldn't test the second example because Chris provide only the output for the second example. I marked all points of interest in the code with the comment "// xxxxx". The test passes.

The repo is at "https://github.com/cpuwzd/wix4/tree/Issue7078".

Please modify my test so that it illustrates the failure.

Thanks, Ron

chrpai commented 1 year ago

@cpuwzd I think what your calling the second example is what the Heatwave v4 project templates created out of the box. It represented the ideal formatting. Correct me if I misunderstand and I'll provide anything you need.

rseanhall commented 1 year ago

@chrpai I believe @cpuwzd is saying that the latest code already converts the Module's Package sub-element into a SummaryInformation element on a different line. Unless you can figure out how to make the test fail, there's nothing to do.

cpuwzd commented 1 year ago

@chrpai @rseanhall

Perhaps unfortunately, I'm a lot more familiar with the innards of Wix 4 than I am with actually using the product. I've installed V3 a couple of times and have done one simple installer with it. I've never installed V4 and I've never read any significant V4 documentation to tell me how to actually use it. I've done all of my V4 work using developer builds. Since all of the separate executables appear to have been abandoned or perhaps, in some cases, renamed, I generally don't know what executable to run to accomplish any single goal.

On that note, I have no idea what the Heatwave v4 project templates are, how to install them, or how to use them. I'm in this purely for personal enjoyment. I'm 77, retired for about 16 years. I'm a colon cancer survivor and I have Parkinson's. It doesn't take much to keep me happy.

If anyone wants to suggest a course of study that will make me more useful on the Wix project, I'm all ears. It's probably best not to play this out in public.

Ron Martin cpuwzd (at) comcast (dot) net