Support configuring the Windows binaries details #261
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Seems like an interesting idea in here, but a couple of things with this that didn't feel right. The Resource class is already including compilation of pre existing resource files, so it seems ResourceCompiler is a duplicate and much reduced function of that. Resources are deliberately compiled after IDL/MC and before C++/C/Fortran. This happens to match the order of Visual studio. I think the interesting idea here is generating part of the resource file to do with the version rather than say including a generic resource block that takes defines. The existing WindowsPlatform implementation is limited in various ways, such as hard coding english stringfileinfo and only supporting 1 language, usage of this generation may be limited? Example our existing method to include version - we have 9 languages and have a predefined version.rc included into other .rc files project or parent pom.xml has |
|
I am aware that there is a previous definition of Resource compiler, but I
am not sure how it is actually working, I only knows that it does not
generate the Version Info.
Greg- is this functionality already supported in a different way? I will be
glad to see how.
Eyal
…On Tue, Jan 31, 2017 at 9:11 AM, Greg Domjan ***@***.***> wrote:
Seems like an interesting idea in here, but a couple of things with this
that didn't feel right.
The Resource class is already including compilation of pre existing
resource files, so it seems ResourceCompiler is a duplicate and much
reduced function of that.
Resources are deliberately compiled after IDL/MC and before C++/C/Fortran.
This happens to match the order of Visual studio.
I think the interesting idea here is generating part of the resource file
to do with the version rather than say including a generic resource block
that takes defines.
That being said there is a lot of configuration here, it might be nice to
have simplified 'by convention' handling taking project/parent version and
other values by default, it would require a different setting for deciding
on 'generate version'.
The existing WindowsPlatform implementation is limited in various ways,
such as hard coding english stringfileinfo and only supporting 1 language,
usage of this generation may be limited?
Example our existing method to include version - we have 9 languages and
have a predefined version.rc included into other .rc files
VS_VERSION_INFO VERSIONINFO
FILEVERSION VER_MAVENARTIFACT_CSV
PRODUCTVERSION VER_MAVENPRODUCT_CSV
FILEFLAGSMASK VS_FFI_FILEFLAGSMASK
FILEFLAGS VER_FILEFLAGS
FILEOS VOS_NT_WINDOWS32
FILETYPE VFT_APP
FILESUBTYPE VFT2_UNKNOWN
BEGIN
BLOCK "StringFileInfo"
BEGIN
BLOCK "000004b0" // neutral
BEGIN
VALUE "CompanyName", VER_MAVENARTIFACT_COMPANYNAMESTR
VALUE "FileVersion", VER_MAVENARTIFACT_DOTSTR
VALUE "FileDescription", VER_MAVENARTIFACT_DESCRIBESTR
VALUE "InternalName", VER_MAVENARTIFACT_NAMESTR
VALUE "ProductVersion", VER_MAVENPRODUCT_DOTSTR
VALUE "ProductName", VER_MAVENPRODUCT_DESCRIBESTR
VALUE "LegalCopyright", VER_LEGALCOPYRIGHT_STR
VALUE "LegalTrademarks", VER_LEGALTRADEMARKS_STR
END
BLOCK "040004b0" // process default
BEGIN
VALUE "CompanyName", VER_MAVENARTIFACT_COMPANYNAMESTR
VALUE "FileVersion", VER_MAVENARTIFACT_DOTSTR
VALUE "FileDescription", VER_MAVENARTIFACT_DESCRIBESTR
VALUE "InternalName", VER_MAVENARTIFACT_NAMESTR
VALUE "ProductVersion", VER_MAVENPRODUCT_DOTSTR
VALUE "ProductName", VER_MAVENPRODUCT_DESCRIBESTR
VALUE "LegalCopyright", VER_LEGALCOPYRIGHT_STR
VALUE "LegalTrademarks", VER_LEGALTRADEMARKS_STR
END
BLOCK "040404b0" // zh-TW
...
project or parent pom.xml has
<resource>
<defines>
<define>VER_LEGALTRADEMARKS_STR=\"${product.trademarks}\"</define>
<define>VER_LEGALCOPYRIGHT_STR=\"${product.copyright}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_DE_DE=\"${product.copyright.de_de}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_EN_US=\"${product.copyright.en_us}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_ES_ES=\"${product.copyright.es_es}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_FR_FR=\"${product.copyright.fr_fr}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_JA_JP=\"${product.copyright.ja_jp}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_PL_PL=\"${product.copyright.pl_pl}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_PT_BR=\"${product.copyright.pt_br}\"</define>
<define>VER_LEGALCOPYRIGHT_STR_ZH_TW=\"${product.copyright.zh_tw}\"</define>
<define>PRODUCT_DESCRIBESTR=\"${product.name}\"</define>
<define>PRODUCT_DESCRIBE=${product.name}</define>
<define>VER_MAVENARTIFACT_CSV=${solution.full.csvversion}</define>
<define>VER_MAVENPRODUCT_CSV=${product.full.csvversion}</define>
<define>VER_MAVENARTIFACT_COMPANYNAMESTR=\"${product.organization}\"</define>
<define>VER_MAVENARTIFACT_DOTSTR=\"${solution.full.version}\"</define>
<define>VER_MAVENARTIFACT_DESCRIBESTR=\"${project.description}\"</define>
<define>VER_MAVENARTIFACT_NAMESTR=\"${project.name}\"</define>
<define>VER_MAVENPRODUCT_DOTSTR=\"${product.version}\"</define>
<define>VER_MAVENPRODUCT_DESCRIBESTR=\"${product.name}\"</define>
</defines>
</resource>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#261 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKT4IWL-R1lAwzsUyAqQNtu_jwI0q2Tvks5rXt68gaJpZM4LrAqo>
.
|
|
@eyalg1972 the ant code seems to handle what you want but not currently being triggered. You have added both a new compiler def and added the versioninfo to it. I think the main triggering point is the addition of the VersionInfo to task/compiler/linker config at which point the linker will collect all version info and make a final generated file which is included during the linker step. So simply inclusion of |
|
Weird, because before I updated it, the RC compiler was not running during
compilation and no res file was generated.
Eyal
…On Tue, Jan 31, 2017 at 10:46 AM, Greg Domjan ***@***.***> wrote:
@eyalg1972 <https://github.com/eyalg1972> the ant code seems to handle
what you want but not currently being triggered.
You have added both a new compiler def and added the versioninfo to it.
I think the main triggering point is the addition of the VersionInfo to
task/compiler/linker config at which point the linker will collect all
version info and make a final generated file which is included during the
linker step.
So simply inclusion of task.addConfiguredVersioninfo(versionInfo); should
cause generation and inclusion into compile.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#261 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKT4ITMBkNaWy3By_mEYtXdCPq6jiGn7ks5rXvTWgaJpZM4LrAqo>
.
|
|
After checking- I removed the Resource Compiler class and the specific call for it |
|
@GregDomjan Are you happy with this now? |
|
It seems ok now - with supporting optional usage and without duplicate execution of resource compiles. Have some doubts, but think they are not blocking.
|
|
The "windows.h" is required for the define of VERSIONINFO there.
Without it- the version resource is created but not being displayed
properly.
(I tested first without it)
Eyal
…On Tue, Apr 18, 2017 at 10:56 AM, Greg Domjan ***@***.***> wrote:
It seems ok now - with supporting optional usage and without duplicate
execution of resource compiles.
Sorry, insufficient time to check further.
Have some doubts, but think they are not blocking.
- change to task looks to be changing bidding and not simply including
resource
- addition of "windows.h" to resource, there are other options so this
might need expansion
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#261 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKT4IVrj6HRVCVA6k1GxrZWOKRIYWbfMks5rxGylgaJpZM4LrAqo>
.
|
|
Alternate options would also include Also there is the option to include the generated .rc into another .rc which may already have an include that then conflicts. Even so, as nobody else is yet using this, it is only for consideration of future compatibility issue that could be considered. |
|
Thanks guys. Merging now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support to the Windows resource files, currently the rc file and res are being generated but the version details are not presented yet