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

Generate Win32 version string based on build no, git tag or fallback to changelog #1730

Merged
merged 27 commits into from
Apr 5, 2018

Conversation

DRSDavidSoft
Copy link
Contributor

This PR addresses two aspects of the C++ launcher compilations, and fixes #1729.

handling versions

With this PR, upon build time the version string is automatically generated by reading the contents of CHANGELOG.md and adjusting a resource file. atm, the Copyright/Company info is hardcoded directly into the template file because I was unsure whether I should've read them from README.me.

added manifest

The manifest file handles dpi-awareness (prevents the messagebox becoming blurry) and controls the UAC definitions. (More work needs to be done on the subject of dpi-awareness and switching the dialogs to TaskDialog as discussed in #1726.)

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla & @MartiUK – ready to merge.
@Jackbennett Can you kindly have a look at my powershell scripts?
@daxgames – thoughts on also adding Copyright year from README.me automatically?

@Stanzilla
Copy link
Member

Not sure about the reading from the changelog part but otherwise super cool!

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla thanks! I thought it was a stable and consistent way to do it. Any other constants to use?

@Jackbennett
Copy link
Contributor

I'm just wondering if the git tag is available to use that might better. Regex parsing the readme just seems to be asking for trouble.

Can't you take advantage of the inbuilt [version] data type instead of smushing strings together? They won't like words in the version string. Can't github tag the release as a pre-release rather than putting it in the version string?

scripts/utils.ps1 Outdated Show resolved Hide resolved
@Stanzilla
Copy link
Member

So in most of my other projects, nightly version names are generated on the last tag name + a sequential number + a git hash + a release type. See here for example: https://www.wowace.com/projects/weakauras-2/files

That would work quite well here as well, I think

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla this is perfect! Notably becauss it also includes a build number. @Jackbennett any ideas how to incorporate this instead of regexing the Changelog?

@DRSDavidSoft
Copy link
Contributor Author

@Jackbennett @Stanzilla It seems that using powershell scripts I can use AppVoyer's environment variables and/or the build worker APIs to get variables like the project name, type of compilation (can be used for copyright year), build number from AppVoyer, tag_name (contains version strings) and even whose commit is being built.

However, we should also consider the fact that the build.ps1 should be able to be built also even without the use of CI tools such as AppVoyer, and thus there needs to be alternate methods of retrieving data on build time.

On the other hand, CHANGELOG.md being built automatically with github-changelog-generator is also almost guranteed to have a regular format.

Should I alter the method to retrieve data from AppVoyer when available and attempt to extract them from CHANGELOG.md when AppVoyer is not available?

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla Then you'd have to update them on each build.
How about hardcoding them in a .json file (like the one in sources/vedor.json) that you regularly update?
And then only using the build number from the AppVeyor.

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla even a better idea! We can use the already existing "git tag" defined in here:

$version = Invoke-Expression "git describe --abbrev=0 --tags"

It would work if it's a AppVeyor environment, or the repo has been cloned.
Otherwise we could use a hardcoded value!

@Stanzilla
Copy link
Member

Sounds good

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla please add a commit to the sources/vedor.json, or any .json file and define the following:

[
    "name": "Cmder",
    "version": "v1.3.6-pre1",
]

I will rewrite my commits to read from that file if AppVeyor or Git is not available.

@Stanzilla
Copy link
Member

meh, do we actually need that? If someone wants to compile without Git present, I don't think that will happen?

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla It's technically possible to download a .zip file archive of the source and launch the build script from it. Currently, this builds a correct archive.

If the repo's not cloned using git clone, even if git is present, the git describe --abbrev=0 --tags won't work.

So, my guess is to either block the user from compiling a downloaded .zip file and throwing an error instead, and warn the user not to download a .zip file,

or we can add a hardcoded .json with the package version.

@Stanzilla
Copy link
Member

Okay, then how about hardcoding the name in your script since the name will actually never change and fall back "version" to "dev" if git is not present or not on AV

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla the name is already hardcoded:

#define CMDER_PRODUCT_NAME_STR      "Cmder"
#define CMDER_FILE_DESCRIPTION_STR  "Cmder: Lovely Console Emulator."
#define CMDER_INTERNAL_NAME_STR     "Cmder"
#define CMDER_ORIGINAL_FILENAME_STR "Cmder.exe"
#define CMDER_COMPANY_NAME_STR      "Samuel Vasko"
#define CMDER_COPYRIGHT_YEAR_STR    "2016"

I can fallback the version string to dev if git is not present, but what about the Win32's file version and product version? Should they be set to 0,0,0,0?
Preview here: #1729 (comment)

@Stanzilla
Copy link
Member

Yes

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla instead of 0.0.0.0, can I fallback to Changelog.md, considering it's a consistent file, and rare that a .zip file will be used?

@Stanzilla
Copy link
Member

Hrm, I guess. What do @cmderdev/trusted-contributors think about the whole thing btw?

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla I implemented and tested the new logic. If all is good, you can merge it now.

@Stanzilla
Copy link
Member

VERBOSE: successfully built Cmder v1.3.6-pre1.635! that doesn't look right :D

@DRSDavidSoft
Copy link
Contributor Author

@Stanzilla what should change in that?

@Stanzilla
Copy link
Member

Oh I thought that was a mistake, but it's the appveyor build number. Hrm it is at a pretty useless value, too. Just git describe --tags then, without adding anything at the end. that will have the sequential number from describe and a pointer to the hash it was compiled from

@DRSDavidSoft
Copy link
Contributor Author

Without --abbrev=0, that gives us: v1.3.6-pre1-40-g94e7, but Win32 doesn't like words like g94e7.
I think the build number was useful as it's a unique and sequential number.

Any other method to substitute AppVeyor's build number?

@Stanzilla
Copy link
Member

what's win32's problem with g94e7?

@DRSDavidSoft
Copy link
Contributor Author

It simply only allows integers at the compiled version info:

cmder_v_error

@Stanzilla
Copy link
Member

but v1.3.6-pre1.635 worked?

@DRSDavidSoft
Copy link
Contributor Author

DRSDavidSoft commented Mar 30, 2018

@Stanzilla It translates to this: 1.3.6.1.635 which works, I'm not sure I can replace g94e7 to 947, for example. (g94e7 is a git hash.)

@Stanzilla
Copy link
Member

meh, just leave it like that then, I guess

@DRSDavidSoft
Copy link
Contributor Author

DRSDavidSoft commented Mar 30, 2018

Alright then. BTW @Jackbennett Do you approve? I also used the git tag as you suggested.
Looking forward to merge when the commits are approved by all!

@DRSDavidSoft DRSDavidSoft changed the title Generate Win32 version string based on changelog Generate Win32 version string based on build no, git tag or fallback to changelog Mar 30, 2018
@Stanzilla Stanzilla merged commit 959073a into cmderdev:master Apr 5, 2018
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.

Cmder.exe should have a version number
3 participants