Re: New eclass to support java-virtuals

Donnie Berkholz wrote:
> On 23:20 Thu 20 Sep     , Alistair Bush wrote:
>> -		# Create package.env
>> -		(
>> -			echo "DESCRIPTION=\"${DESCRIPTION}\""
>> -			echo "GENERATION=\"2\""
>> -
>> -			[[ -n "${JAVA_PKG_CLASSPATH}" ]] && echo "CLASSPATH=\"${JAVA_PKG_CLASSPATH}\""
>> -			[[ -n "${JAVA_PKG_LIBRARY}" ]] && echo "LIBRARY_PATH=\"${JAVA_PKG_LIBRARY}\""
>> -			[[ -n "${JAVA_PROVIDE}" ]] && echo "PROVIDES=\"${JAVA_PROVIDE}\""
>> -			[[ -f "${JAVA_PKG_DEPEND_FILE}" ]] \
>> -				&& echo "DEPEND=\"$(cat "${JAVA_PKG_DEPEND_FILE}" | uniq | tr '\n' ':')\""
>> -			[[ -f "${JAVA_PKG_OPTIONAL_DEPEND_FILE}" ]] \
>> -				&& echo "OPTIONAL_DEPEND=\"$(cat "${JAVA_PKG_OPTIONAL_DEPEND_FILE}" | uniq | tr '\n' ':')\""
>> -			echo "VM=\"$(echo ${RDEPEND} ${DEPEND} | sed -e 's/ /\n/g' | sed -n -e '/virtual\/\(jre\|jdk\)/ { p;q }')\"" # TODO cleanup !
>> -		) > "${JAVA_PKG_ENV}"
> 
> Why not use a code block {} instead of a subshell ()?

You would have to ask the original author about that, but point taken.

> 
>> -		# register target/source
>> -		local target="$(java-pkg_get-target)"
>> -		local source="$(java-pkg_get-source)"
>> -		[[ -n ${target} ]] && echo "TARGET=\"${target}\"" >> "${JAVA_PKG_ENV}"
>> -		[[ -n ${source} ]] && echo "SOURCE=\"${source}\"" >> "${JAVA_PKG_ENV}"
>> -
>> -		# register javadoc info
>> -		[[ -n ${JAVADOC_PATH} ]] && echo "JAVADOC_PATH=\"${JAVADOC_PATH}\"" \
>> -			>> ${JAVA_PKG_ENV}
>> -		# register source archives
>> -		[[ -n ${JAVA_SOURCES} ]] && echo "JAVA_SOURCES=\"${JAVA_SOURCES}\"" \
>> -			>> ${JAVA_PKG_ENV}
>> -
>> -
>> -		echo "MERGE_VM=\"${GENTOO_VM}\"" >> "${JAVA_PKG_ENV}"
>> -		[[ -n ${GENTOO_COMPILER} ]] && echo "MERGE_COMPILER=\"${GENTOO_COMPILER}\"" >> "${JAVA_PKG_ENV}"
> 
> I don't understand why all these things are done down here instead of in the 
> same code block as $JAVA_PKG_ENV is created.

That to is also historical, and i'm less inclined to changed it. I've
already broken the tree once with a eclass change, breaking this would
completely break java support.  It could be updated as part of our
general QA, but I believe that it should be done separately, and not
bundled in with the changes i've made.

> 
>> -		# Strip unnecessary leading and trailing colons
>> -		# TODO try to cleanup if possible
>> -		sed -e "s/=\":/=\"/" -e "s/:\"$/\"/" -i "${JAVA_PKG_ENV}" || die "Did you forget to call java_init ?"
>> +	
>> +	if [[ $1 != provider ]]; then
> 
> Could you explain what the next section is supposed to do, as opposed to 
> the above section? Are they expected to be mutually exclusive? The 
> comments suggest so, since both have a 'Create package.env'. But the 
> tests suggest otherwise, since it's not an if...else pair.

normally java-pkg_do_write_ is called to write the package.env out, as
can be seen, and is the default behavior for the function.  What I am
adding is the ability to _do_write of a "[virtual|provider].env" file.
While at present it only contains 3 variables, I see no reason why
eventually it will not include other vars that are shared between the
package.env and "virtual.env" ( e.g DESCRIPTION )

Therefore this change can be summarized as

+ if [[ $1 != provider ]]; then
	#Do the default package.env behaviour
+ else
+	#Create a "virtual.env" file. This will only be invoked by
+	#ebuilds that inherit java-virtuals.eclass
+ fi

I could very well reflactor the java-pkg_do_write_ function back to its
current state and create a new function java-pkg_do_write_virtual to be
called by java-virtuals-2.eclass.

The documentation could (and will) be updated to more correctly reflect
whats happening.


> 
>> +		# Create directory for package.env
>> +		dodir "${JAVA_PKG_SHAREPATH}"
>> +		if [[ -n "${JAVA_PKG_CLASSPATH}" || -n "${JAVA_PKG_LIBRARY}" || -f \
>> +				"${JAVA_PKG_DEPEND_FILE}" || -f \
>> +				"${JAVA_PKG_OPTIONAL_DEPEND_FILE}" ]]; then
>> +			# Create package.env
>> +			(
>> +				echo "DESCRIPTION=\"${DESCRIPTION}\""
>> +				echo "GENERATION=\"2\""
>> +
>> +				[[ -n "${JAVA_PKG_CLASSPATH}" ]] && echo "CLASSPATH=\"${JAVA_PKG_CLASSPATH}\""
>> +				[[ -n "${JAVA_PKG_LIBRARY}" ]] && echo "LIBRARY_PATH=\"${JAVA_PKG_LIBRARY}\""
>> +				[[ -n "${JAVA_PROVIDE}" ]] && echo "PROVIDES=\"${JAVA_PROVIDE}\""
>> +				[[ -f "${JAVA_PKG_DEPEND_FILE}" ]] \
>> +					&& echo "DEPEND=\"$(cat "${JAVA_PKG_DEPEND_FILE}" | uniq | tr '\n' ':')\""
>> +				[[ -f "${JAVA_PKG_OPTIONAL_DEPEND_FILE}" ]] \
>> +					&& echo "OPTIONAL_DEPEND=\"$(cat "${JAVA_PKG_OPTIONAL_DEPEND_FILE}" | uniq | tr '\n' ':')\""
>> +				echo "VM=\"$(echo ${RDEPEND} ${DEPEND} | sed -e 's/ /\n/g' | sed -n -e '/virtual\/\(jre\|jdk\)/ { p;q }')\"" # TODO cleanup !
>> +			) > "${JAVA_PKG_ENV}"
> 
> Thanks,
> Donnie
-- 
[email protected] mailing list