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

[Red Hat Bugzilla]

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 [e-mail]  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!