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

Improve Inno Setup installer script #106

Merged
merged 3 commits into from
Oct 25, 2023
Merged

Improve Inno Setup installer script #106

merged 3 commits into from
Oct 25, 2023

Conversation

bovirus
Copy link
Contributor

@bovirus bovirus commented Oct 24, 2023

@radj307

Fix some issues

  • File version
  • Uninstaller icon
  • Add starting/ending copyright year
  • many other improvements

Pelase check and edit (if required) "StartingYearCopyright" variable,

Run Inno Setup, create and test new installer.

Please check and merge. Thanks.

Fix some issues
File version
Uninstaller icon
Add starting/ending copyright year
etc
Copy link
Owner

@radj307 radj307 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have some good changes here, but there are some things that need fixing before I can merge this.

In addition to the comments, line 67 can be removed as it is no longer needed with the change to the uninstaller icon path:

Source: "VolumeControl\Resources\icons\iconSilvered.ico"; DestDir: "{app}"; Flags: ignoreversion

vcsetup.iss Outdated Show resolved Hide resolved
vcsetup.iss Outdated Show resolved Hide resolved
vcsetup.iss Outdated Show resolved Hide resolved
vcsetup.iss Outdated Show resolved Hide resolved
Copy link
Owner

@radj307 radj307 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few minor changes and 1 major change required before I can merge this.

vcsetup.iss Outdated Show resolved Hide resolved
vcsetup.iss Outdated Show resolved Hide resolved
vcsetup.iss Outdated Show resolved Hide resolved
vcsetup.iss Outdated Show resolved Hide resolved
@bovirus
Copy link
Contributor Author

bovirus commented Oct 25, 2023

@radj307

Please take care that you repository seems not cofded as text as expecetd in UTF-8.
If I try to modify some file in my repository cloned by yours I got this message.

image

If you can please convert you repository as text as UTF-8.

This the issue about

AppCopyright=© {#StartYearCopyright}-{#CurrentYear} {#AppPublisher}

© is not coded as expecetd as UTF-8.

@radj307
Copy link
Owner

radj307 commented Oct 25, 2023

Copyright symbol problem was solved by 4b2e3db

@radj307 radj307 merged commit 0f360e2 into radj307:main Oct 25, 2023
@radj307
Copy link
Owner

radj307 commented Oct 25, 2023

Everything looks good, many thanks! Merged.

@bovirus
Copy link
Contributor Author

bovirus commented Oct 26, 2023

@radj307

What's the problem about non UTF-8 text editing in your site? How can fix it?
UTF.8 avoid problem in accented letters (like à è é ì ò ù ©) and is the best solution for text coding for github repository.

About version number you should edit every time when your run compiling script or is it an automatic job that get version number by exe file?
If you are using Inno Setup 6,x I can modify the script to get app version in 3 digit way (like x.x.x).

About LICENSE file of course you should edit

  • to remove forced CR/LF at 80 chars
  • add New line for each paragraph.
    as I explained the GNU team answered me that there is no problem to make these changes because the text contents didn't change.

Could you pelase make a fresh update exe (installer/portable) in your repository using new Inno Setup script?
Thanks.

@radj307
Copy link
Owner

radj307 commented Oct 26, 2023

@bovirus

AFAIK, files retain whatever encoding they had on the operating system that uploaded them. Git will change line endings automatically, but not the encoding. Once you changed the encoding to UTF-8 and pushed the commit, vcsetup.iss is now encoded in UTF-8 everywhere. No need for any further changes, the symbol is appearing correctly now.

Volume Control's release workflow uses github actions. I only have to do 2 steps (step 1 & step 4), everything else is automated. It works like this:

  1. I create a new tag on a commit and push it to the repository
  2. The UpdateVersion.yaml action is triggered by the new tag.
    1. Gets the most recent tag with git describe --tags --abbrev=0 and updates version numbers in the .csproj files using the SetVersion.ps1 powershell script.
    2. Creates a commit & pushes the changes.
  3. The GenerateRelease-csharp.yml action is triggered by UpdateVersion.yaml succeeding.
    1. Builds the portable version EXE, creating VolumeControl.exe
    2. Builds the installer version EXE
    3. Compiles the installer, creating VolumeControl-Installer.exe
    4. Uploads the SDK nuget package & creates a build artifact with the SDK DLLs
    5. Creates a build artifact containing VolumeControl.exe & VolumeControl-Installer.exe
    6. Downloads the build artifacts and adds them to a new github draft release.
  4. I write a changelog for the new release, test the executable, and publish it.
    It's at this point that Volume Control will prompt people about a new release being available (if the version number of the new release is newer than their version).
  5. winget.yaml action is triggered by the new release, and uploads the installer to winget.

I never change version numbers in files manually because it would screw everything up if I forgot to do so. This way, I only have to set the version number once and everything else happens automatically.

As for a version of the new installer, you can download the v6.3.0-debug installer file from the build artifact here (it's inside build-windows.zip):
https://github.com/radj307/volume-control/suites/17635389863/artifacts/1008102756
I haven't created a release yet since there are still changes I need to make before it's ready.

If you want to build it yourself, you can do the following steps to build it locally (if you need more details, let me know by mentioning me again):

  1. Install Visual Studio, .NET Core 6 SDK, .NET CLI, and Inno Setup 6.
    These steps assume you have the locations of dotnet and iscc added to your PATH environment variable.
  2. Open a terminal somewhere that you want to clone the project to, then run the following commands:
  3. git clone https://github.com/bovirus/volume-control ; cd volume-control
  4. dotnet publish VolumeControl -c Release-ForInstaller -p:PublishProfile="VolumeControl/Properties/PublishProfiles/PublishForInstaller.pubxml"
  5. git clone https://github.com/DomGries/InnoDependencyInstaller
  6. iscc /dAppVersion="$(git describe --tags --abbrev=0)" /Opublish

You can find VolumeControl-Installer.exe in the publish directory found in the repository root.

@bovirus
Copy link
Contributor Author

bovirus commented Oct 26, 2023

@radj307

Thanks for explanation.
What's about LICENSE changes?
Can I provide the new LICENSE file (same text but a different formmatting)?

@radj307
Copy link
Owner

radj307 commented Oct 29, 2023

@bovirus

The LICENSE file should not be modified directly.
There are alternative ways to do this, namely using scripting to modify the license text without editing the file.

Related stackoverflow post: https://stackoverflow.com/questions/53221771/inno-setup-how-to-change-licensefile-text

@bovirus
Copy link
Contributor Author

bovirus commented Oct 30, 2023

@radj307

I cannot understand why cannot modify the LICENSE file directly.
The GNU tean confirm that you can change the text layout format (CR/LF and line lenght).
Important is not modiyfy text contents.

Then there is no reason why cannot do it.

The original license text is come from 1997 where the terminal had fixe lien lenght of 80chars.
But now there are different etrminal efatures (line longer 30chars etc).

If you want we can create file LICENSE_NEW_FORMAT and use it.

@radj307
Copy link
Owner

radj307 commented Oct 30, 2023

@bovirus

While it is legally acceptable to modify the contents of the LICENSE file so long as the text itself remains, I don't want to do that for a few reasons:

  1. Modifying the LICENSE file directly requires trial-and-error for getting the correct indentation/line length for the current window dimensions.
  2. We'll have to fix the LICENSE file everytime inno setup updates & changes the default wizard style.
    (Since Volume Control is built via github actions, I cannot guarantee that the same version of inno setup will be used every time)
  3. Comparing file hash with another GPL-3 license file will not work when the file is modified. This may cause shittily-designed web crawlers to assume it does not use the GPL-3 license, or worse, any license.
  4. The file will not display correctly in the terminal if it uses the legacy width.

1 & 2 are my main concerns. The possibility that someone else making a change to a dependency (that I won't be notified of) requiring a manual fix is unacceptable, especially since we will have forgotten all about this by that time.
It would be easy to just specify a wizard style to use forever, but I'd much rather let that be chosen automatically so it'll suit future versions of Windows better.

All in all, the fewer things that require manual human intervention instead of happening automatically, the better. I'd much rather write a few lines of code to make sure I never have to worry about it again.

@radj307
Copy link
Owner

radj307 commented Oct 30, 2023

My idea for scripting a fix for the license file width issue goes something like this:

  1. Strip all newlines from the text (except paragraph breaks)
  2. Calculate the number of characters that can be displayed on a line in the text area
  3. Insert newlines in the text to wrap it at a suitable location (undershooting the line length limit when needed to prevent chopping words in half)
  4. Load the text into the text area

If inno setup automatically wraps the text, the steps are even simpler:

  1. Strip all newlines from the text (except paragraph breaks)
  2. Load the text into the text area

Even Pascal (the language) can't stop how easy that is, and it'll ensure this never becomes a problem again.

@radj307
Copy link
Owner

radj307 commented Oct 30, 2023

I'm going to move this discussion to #109

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.

None yet

2 participants