[Bug 436036] Review Request: jna - Pure Java access to native libraries

bugzilla at redhat.com bugzilla at redhat.com
Fri Mar 21 21:20:54 UTC 2008


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: jna - Pure Java access to native libraries


https://bugzilla.redhat.com/show_bug.cgi?id=436036





------- Additional Comments From walters at redhat.com  2008-03-21 17:20 EST -------
Not even mentioning to upstream that we're patching their code is always wrong.
If they reject a patch, then we can carry it, but it's far better to get
everything merged.
http://fedoraproject.org/wiki/PackageMaintainers/WhyUpstream

I just had a look at your patch.  First, I don't understand this:

-CFLAGS=$(PCFLAGS) $(COPT) $(CDEBUG) $(CDEFINES) $(CINCLUDES) \
-       -DVERSION='"$(VERSION)"' -DCHECKSUM='"$(CHECKSUM)"'
+CFLAGS=FEDORA

Are you trying to define something?  We need to use -DFEDORA then, right?

On an overall level, I think you could make most of this patch conditional, like
this:

$(BUILD)/%.o : %.c dispatch.h $(FFI_LIB)
 	@mkdir -p $(BUILD)
ifeq ($(USE_SYSTEM_FFI),)
ifneq ($(SDKROOT),)
	$(CC) -arch $(ARCH) $(CFLAGS) -c $< -o $@.$(ARCH)
	for arch in $(ALT_ARCHS); do \
	  $(CC) -arch $$arch -I$(BUILD)/libffi.$$arch/include $(CFLAGS) -c $< o
$@.$$arch; \
	done
	lipo -create -output $@ $@.*
else
	$(CC) $(CFLAGS) -c $< $(COUT)
endif
else
	$(CC) $(CFLAGS) -c $< $(COUT) `pkg-config --cflags libffi`
endif

This would allow upstream to merge it without affecting builds on operating
systems with no system ffi, or a too-old version (and the latter almost
certainly includes older Fedora releases or RHEL, remember).

Thanks a lot for your work on this, I'm looking forward to having JRuby in the OS!


-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.




More information about the Fedora-package-review mailing list