jramosd / javachromiumembedded

Automatically exported from code.google.com/p/javachromiumembedded
0 stars 0 forks source link

Binary distribution should include version numbers #17

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Windows 7 64-bit

What steps will reproduce the problem?
1. Run the make_distrib.bat script
2. Examine the resulting binary distribution

What is the expected output? What do you see instead?
The resulting distribution should include CEF and JCEF version number 
information (for example, in the directory name and README.txt file). Instead, 
no version number information is included.

Original issue reported on code.google.com by magreenb...@gmail.com on 31 Oct 2013 at 9:16

GoogleCodeExporter commented 9 years ago

Original comment by magreenb...@gmail.com on 31 Oct 2013 at 9:17

GoogleCodeExporter commented 9 years ago
Hi Marshall,
here is a patch file which adds versioning to JCEF. I've used some python tools 
from your CEF project and modified them somewhat.

What I've done
------------
[Java]
(*) Added nested Class CefVersion to CefApp
(*) Added method CefVersion getVersion() to CefApp
(*) Added native method N_GetVersion() to handle the JCEF version within the 
native code

[Native]
(*) Modified CefApp.cpp/.h to handle the java version method
(*) Added jcef_version.cpp with the function implementation of 
"jcef_version_info(int entry)"
(*) Modified jcef_dll.rc to use the JCEF version defines instead of CEF version 
defines

[Tools]
(*) Added make_version_header.[bat|sh] and make_version_header.py
(*) Added call to make_version_header.py into gyp_jcef
(*) Modified make_distrib.[bat|sh] and added make_distrib.py
(*) make_distrib.py creates the README.txt from Readme-templates in the 
directory "distrib"
(*) Added Readme templates README.footer.txt, README.header.txt, and the 
platform dependent ones README.standard.txt, README.redistrib.txt and removed 
all README.txt files
(*) Added several Python files from CEF (e.g. date_util.py, exec_util.py, 
file_util.py, gclient_util.py, git_util.py, svn_util.py)

If you execute "gclient runhooks" the jcef_version.h is generated on the fly 
and if you execute make_distrib.[bat|sh] the README.txt is generated from the 
template files and extended by the version numbers of JCEF, CEF and Chromium.

Regards,
Kai

Original comment by k...@censhare.de on 20 Jun 2014 at 1:59

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the patch. Some comments:

1. I've make a few changes to make_distrib.bat (attached) to get it working on 
Windows 8.1:

A. The line "python.bat tools\make_distrib.py" needs to be prefixed with "call" 
or the script terminates and the .txt files are not copied.

B. The path to EXCLUDE_FILES.txt for xcopy /exclude needs to be hard-coded 
instead of using variable substitution because otherwise the command fails with 
a `Can't read file: ".\tools\distrib\EXCLUDE_FILES.txt"` error. This feels like 
a bug in xcopy since it can resolve the variable for the error message but not 
for actually reading the file.

2. I've filed https://code.google.com/p/chromiumembedded/issues/detail?id=1330 
for the incorrect Chromium URL in CEF's README.txt file.

3. I've been using a 4-part version number for JCEF: 
CEF_MAJOR.CEF_MINOR.CEF_REVISION.JCEF_REVISION (e.g. 3.1750.1738.80). If we 
continue with this format then we could remove the new "VERSION" file and the 
JCEF_VERSION_MAJOR #define from jcef_version.h and just rely on parsing CEF's 
README.txt file in make_distrib.py instead.

4. Is the jcef_version_info function necessary? It seems like the #defines in 
jcef_version.h should be sufficient since there are no callers from outside of 
the jcef library who would need the exported function.

5. The name of the make_distrib.* scripts isn't really accurate since it just 
creates the README.txt file. Consider calling it make_readme.* instead.

Original comment by magreenb...@gmail.com on 2 Jul 2014 at 8:06

Attachments:

GoogleCodeExporter commented 9 years ago
@comment #3:
Hi Marshall,
Thank you for your review.

Attached you'll find an updated patch file:
- added your changes according make_distrib.bat
- extended the version number to the 4-part pattern and removed 
JCEF_VERSION_MAJOR #define from jcef_version.h
- removed jcef_version.cpp and the jcef_version_info function
- modified the windows resource file to fit to the 4-part version number
- renamed make_distrib.py to make_readme.py and added make_readme.[bat|sh]

Regards, Kai

Original comment by k...@censhare.de on 3 Jul 2014 at 6:38

Attachments:

GoogleCodeExporter commented 9 years ago
@#4: Thanks, added in revision 95 with minor changes:
- Include version number info in jcef_helper.exe.
- Fix include header ordering in CefApp.cpp.

Original comment by magreenb...@gmail.com on 10 Jul 2014 at 4:24