Provide programmatic version information in all of our executables and dlls #6205

Merged
merged 6 commits into from Sep 1, 2016

Conversation

Projects
None yet
3 participants
@michaelDCurran
Contributor

michaelDCurran commented Jul 25, 2016

Fixes #6204

michaelDCurran added some commits Jul 24, 2016

Provide proper version information in all executables and dlls: year,…
… major, minor, build. Build is conveyed to scons via a version_build argument. AppVeyor uses its own build number for this.
Don't set fileDescription on executables and dlls. It is not really n…
…eeded, plus it is misleading unless it is set with a string specific to each file.
nvdaHelper/client/sconscript
@@ -30,9 +30,13 @@ controllerRPCHeader,controllerRPCClientSource=env.MSRPCStubs(
clientLibName="nvdaControllerClient%s"%('64' if env['TARGET_ARCH']=='x86_64' else '32')
+rcFile=env.Command('nvda.rc',env['projectRCFile'],Copy('$TARGET','$SOURCE'))
+RESFile=env.RES(rcFile)

This comment has been minimized.

@jcsteh

jcsteh Aug 9, 2016

Contributor

Is there a reason we can't use the same .res file for all of these dlls/exes, rather than having a separate one for each.

@jcsteh

jcsteh Aug 9, 2016

Contributor

Is there a reason we can't use the same .res file for all of these dlls/exes, rather than having a separate one for each.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 9, 2016

Contributor

nvda.rc.subst has been saved as UCS-2 (16 bit), rather than ASCII. Is there any reason for this? It makes it show up as a binary file in GitHub, among other things.

Contributor

jcsteh commented Aug 9, 2016

nvda.rc.subst has been saved as UCS-2 (16 bit), rather than ASCII. Is there any reason for this? It makes it show up as a binary file in GitHub, among other things.

nvdaHelper/remote/sconscript
@@ -72,9 +72,11 @@ nvdaInProcUtilsRPCHeader,nvdaInProcUtilsRPCServerSource=env.MSRPCStubs(
ia2utilsObj=env.Object("./ia2utils","../common/ia2utils.cpp")
+

This comment has been minimized.

@jcsteh

jcsteh Aug 9, 2016

Contributor

nit: Extraneous added white space.

@jcsteh

jcsteh Aug 9, 2016

Contributor

nit: Extraneous added white space.

@@ -61,6 +61,7 @@ keyCommandsLangBlacklist=set([])
vars = Variables()
vars.Add("version", "The version of this build", versionInfo.version)
+vars.Add("version_build", "A unique number for this build.", "0")

This comment has been minimized.

@jcsteh

jcsteh Aug 9, 2016

Contributor

Is there a reason you went with version_build rather than the camel case versionBuild? It just seems out of place because all the other variables are camel case. I'm guessing you're trying to make it clear that build is a component of version...? Same for the names in the versionInfo module.

@jcsteh

jcsteh Aug 9, 2016

Contributor

Is there a reason you went with version_build rather than the camel case versionBuild? It just seems out of place because all the other variables are camel case. I'm guessing you're trying to make it clear that build is a component of version...? Same for the names in the versionInfo module.

This comment has been minimized.

@michaelDCurran

michaelDCurran Aug 9, 2016

Contributor

Correct. versionBuild seems like bad English. buildVersion would be better. But since they are all specific to version I figured version_* was better.

@michaelDCurran

michaelDCurran Aug 9, 2016

Contributor

Correct. versionBuild seems like bad English. buildVersion would be better. But since they are all specific to version I figured version_* was better.

source/versionInfo.py
+version_year=2016
+version_major=3
+version_minor=0
+version_build=0

This comment has been minimized.

@jcsteh

jcsteh Aug 9, 2016

Contributor

nit: I wonder if these should be moved down to where version is defined, just to keep them all in the same place.

@jcsteh

jcsteh Aug 9, 2016

Contributor

nit: I wonder if these should be moved down to where version is defined, just to keep them all in the same place.

source/versionInfo.py
@@ -31,11 +36,11 @@ def _updateVersionFromVCS():
# Otherwise, py2exe will break.
name="NVDA"
longName=_("NonVisual Desktop Access")
-version="2016.3dev"
+version="%s.%s.%sdev"%(version_year,version_major,version_minor)

This comment has been minimized.

@jcsteh

jcsteh Aug 9, 2016

Contributor

This will result in, for example, 2016.3.0dev instead of just 2016.3dev. Perhaps this doesn't matter for now, as we override the version for releases anyway.

@jcsteh

jcsteh Aug 9, 2016

Contributor

This will result in, for example, 2016.3.0dev instead of just 2016.3dev. Perhaps this doesn't matter for now, as we override the version for releases anyway.

This comment has been minimized.

@michaelDCurran

michaelDCurran Aug 9, 2016

Contributor

Agreed. I don't think it matters.

@michaelDCurran

michaelDCurran Aug 9, 2016

Contributor

Agreed. I don't think it matters.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 17, 2016

Contributor

All good. Incubate away.

Contributor

jcsteh commented Aug 17, 2016

All good. Incubate away.

@jcsteh jcsteh merged commit 438d022 into master Sep 1, 2016

@nvaccessAuto nvaccessAuto removed the incubating label Sep 1, 2016

@nvaccessAuto nvaccessAuto added this to the 2016.4 milestone Sep 1, 2016

@jcsteh jcsteh deleted the versionedBinaries branch Sep 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment