Endi,
Comments in-line.
On 10/27/12 07:33, Endi Sukma Dewata wrote:
On
10/26/2012 7:06 PM, Matthew Harmsen wrote:
* The JNI 'symkey.jar' is a file and
contains no embedded version in
its name
Please take a look at the following code in
base/symkey/src/CMakeLists.txt:
install(
FILES
${CMAKE_BINARY_DIR}/dist/symkey.jar
DESTINATION
${LIB_INSTALL_DIR}/symkey -- (A)
)
'${LIB_INSTALL_DIR}' is defined in
'pki/cmake/Modules/DefineInstallationPaths.cmake' as
'${EXEC_INSTALL_PREFIX}/lib${LIB_SUFFIX}' which basically places
'symkey.jar' into 'usr/lib/symkey' (32-bit) or 'usr/lib64/symkey'
(64-bit) unless this is being somehow overridden by a newer version
of cmake?
install(
FILES
${CMAKE_BINARY_DIR}/dist/symkey.jar
DESTINATION
${JAVA_LIB_INSTALL_DIR} -- (B)
)
'${JAVA_LIB_INSTALL_DIR}' is defined in
'pki/cmake/Modules/DefineInstallationPaths.cmake' as
'${CMAKE_INSTALL_PREFIX}/lib/java' which basically places
'symkey.jar' into 'usr/lib/java' (32-bit & 64-bit) unless this
is being somehow overridden by a newer version of cmake?
and
in pki-core.spec:
cd %{buildroot}%{_libdir}/symkey
%if 0%{?fedora} >= 16
%{__rm} %{buildroot}%{_jnidir}/symkey.jar -- (C)
%{__mv} symkey.jar %{buildroot}%{_jnidir}/symkey.jar -- (D)
%else
%{__rm} symkey.jar -- (E)
%{__ln_s} symkey-%{version}.jar symkey.jar -- (F)
%endif
For Fedora 16 and greater, this removes 'symkey.jar' on 32-bit and
removes nothing on 64-bit systems, and moves symkey.jar under the
appropriate 'usr/lib/java' (32-bit) and 'usr/lib64/java' (64-bit).
This code block may not be necessary if we make the change I suggest
below.
As RHEL 5, RHEL 6, and Fedora 17 or less are not supported, and we
are not yet certain of the RHEL 7 changes for JNI, I agree that the
%else code block could be removed.
%if
0%{?rhel} || 0%{?fedora} < 16
cd %{buildroot}%{_jnidir}
%{__rm} symkey.jar -- (G)
%{__ln_s} %{_libdir}/symkey/symkey.jar symkey.jar -- (H)
%endif
Since this code exists on the 'master', and RHEL 5, RHEL 6, and
Fedora 15 or less are not supported, and we are not yet certain of
the RHEL 7 changes for JNI, I agree that this code block could be
removed.
Some
observations:
1. In A and B the code installs symkey.jar in /usr/lib64/symkey
and /usr/lib64/java, respectively. In C the symkey.jar is removed
from /usr/lib64/java. In D the symkey.jar is moved from
/usr/lib64/symkey to /usr/lib64/java. It seems that the code in A,
C, and D is not needed since we have B.
2. The code in E & F doesn't seem to be needed anymore since
we don't support anything before F16.
3. The code in G & H removes the symkey.jar in /usr/lib64/java
and replaces it with a link to symkey.jar in /usr/lib64/symkey.
This is not necessary since we have B.
If these are correct we probably can remove everything above
except B.
Per my comments above, I think that we could remove everything above
except B by making the following changes (I need to do test builds
on 32-bit and 64-bit systems to prove this theory) from:
install(
FILES
${CMAKE_BINARY_DIR}/dist/symkey.jar
DESTINATION
${JAVA_LIB_INSTALL_DIR} -- (B)
)
to:
install(
FILES
${CMAKE_BINARY_DIR}/dist/symkey.jar
DESTINATION
${LIB_INSTALL_DIR}/java -- (B)
)
Do you agree with this approach?
Other
than that the patch looks good, ACK.