Skip to content

Conversation

@JunTaoLuo
Copy link
Contributor

This addresses dotnet/installer#2655

The regression seems to be 41ce223#diff-8a63ea279503159f0bb870af84feae00L10. I'm assuming it was an oversight when removing the BuildNumberSuffix variable. I'm updating the logic to mimic the previous behaviour, albeit using different variables.

I've verified with this change that the product code and upgrade code are now unique between builds. However, I don't think I'll be able to do end to end verification.

I plan on merging as soon as builds are green, so the change can be consumed downstream.

cc @johnbeisner

This affects product code and upgrade code
@johnbeisner
Copy link
Contributor

johnbeisner commented Jun 29, 2019

@JunTaoLuo
Why would the final builds need a stable GUID, release over release. I would think we would have the same problem. I don't think we should block on this, but we should have a discussion on what the expected behaviors that we want to see from our combined installers.

@JunTaoLuo
Copy link
Contributor Author

Yea, I wasn't sure why but this is the behaviour I see in our previous code. Since I couldn't track down why it was built that way, I figured it's safer to keep it until I can find out more.

@JunTaoLuo
Copy link
Contributor Author

Do you know what's the logic used by other teams? @johnbeisner

@johnbeisner
Copy link
Contributor

For dotnet/core-sdk MSIs:

<GenerateGuidFromName Name="$(SdkMSIInstallerFile)">
      <Output TaskParameter="OutputGuid"

Where the seed string 'SdkMSIInstallerFile' for the MSI GUID is full path + filename: "C:\buildrootcoresdk\artifacts\packages\Release\Shipping\dotnet-sdk-3.0.100-preview7-012608-win-x64.msi"

@JunTaoLuo
Copy link
Contributor Author

Ah, then I think we should follow suite.

@JunTaoLuo JunTaoLuo merged commit ef19361 into master Jun 29, 2019
@JunTaoLuo JunTaoLuo deleted the johluo/installer-upgrade-code branch June 29, 2019 04:47
@joeloff
Copy link
Member

joeloff commented Jun 30, 2019

The logic got removed as part of the arcade work it seems: 41ce223#diff-8a63ea279503159f0bb870af84feae00

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.

4 participants