Skip to content

Commit

Permalink
Tue Sep 9 06:49:28 UTC 2014 Johnny Willemsen <jwillemsen@remedy.nl>
Browse files Browse the repository at this point in the history 
        * bin/generate_doxygen.pl:
          Use safe way to get an unique tmp file

        * include/makeinclude/platform_linux_common.GNU:
          Added support for detecting and using platform large file
          flags

          Thanks to Pau Garcia i Quiles <pgquiles at elpauer dot org>
          for providing both patches
  • Loading branch information
@jwillemsen
jwillemsen committed Sep 9, 2014
1 parent 4ba4be1 commit 381c152
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
12 changes: 12 additions & 0 deletions ACE/ChangeLog
View file
@@ -1,3 +1,15 @@
Tue Sep 9 06:49:28 UTC 2014 Johnny Willemsen <jwillemsen@remedy.nl>

* bin/generate_doxygen.pl:
Use safe way to get an unique tmp file

* include/makeinclude/platform_linux_common.GNU:
Added support for detecting and using platform large file
flags

Thanks to Pau Garcia i Quiles <pgquiles at elpauer dot org>
for providing both patches

Tue Sep 9 06:46:06 UTC 2014 Johnny Willemsen <jwillemsen@remedy.nl>

* ace/Time_Value.cpp:
Expand Down
3 changes: 2 additions & 1 deletion ACE/bin/generate_doxygen.pl
View file
Expand Up @@ -11,6 +11,7 @@

use Cwd;
use File::Spec;
use File::Temp qw/ tempfile tempdir /;
use Env qw(ACE_ROOT TAO_ROOT CIAO_ROOT DDS_ROOT);

# Configuration and default values
Expand Down Expand Up @@ -174,7 +175,7 @@ sub generate_doxy_files {
}

my $input = "$ROOT_DIR/etc/".$i.".doxygen";
my $output = "/tmp/".$i.".".$$.".doxygen";
($fh, $output) = tempfile(TEMPLATE => 'XXXXXXXX', SUFFIX => '.doxygen', TMPDIR => 1, DESTROY => 1);

open(DOXYINPUT, $input)
|| die "Cannot open doxygen input file $input\n";
Expand Down
6 changes: 6 additions & 0 deletions ACE/include/makeinclude/platform_linux_common.GNU
View file
Expand Up @@ -108,6 +108,12 @@ ifeq ($(ssl),1)
PLATFORM_SSL_CPPFLAGS += -I/usr/kerberos/include
endif # ssl

PLATFORM_LARGEFILE_CFLAGS := $(shell getconf LFS_CFLAGS 2> /dev/null || echo Unknown)

This comment has been minimized.

Sign in to view

Copy link
@bear101

bear101 May 12, 2015

'platform_android.GNU' includes 'platform_linux_common.GNU' but when 'getconf' is called it's the OS of the build-host which is being asked if it supports large files. It should be Android which is asked...
When I compile Android on Mac OS X "Unknown" ends up as a build-flag for the compiler.

This comment has been minimized.

Sign in to view

Copy link
@shuston

shuston May 12, 2015

Contributor

Since this change has been out in releases for quite some time, could you please open a Bugzilla item to describe the problem? If you have a patch, please enter a new pull request (preferred) or attach it to your Bugzilla report.

This comment has been minimized.

Sign in to view

Copy link
@jwillemsen

jwillemsen May 12, 2015

Author Member

platform_android.GNU is at this moment only tested/used for cross compilation using a linux host, probably another platform_android_macosx.GNU has to be added to cross compile Android on MacOSX

This comment has been minimized.

Sign in to view

Copy link
@bear101

bear101 May 12, 2015

but will 'getconf' on all Linux dists always return the same as 'getconf' on Android?

This comment has been minimized.

Sign in to view

Copy link
@jwillemsen

jwillemsen May 12, 2015

Author Member

Probably not, maybe PLATFORM_LARGEFILE_CFLAGS can be set to the correct value in platform_android.GNU and platform_linux.GNU only sets them when not set, but as Steve said, please open a bugzilla item and a pull request when you have a patch

This comment has been minimized.

Sign in to view

Copy link
@dlifshitz-maca

dlifshitz-maca Aug 1, 2016

Contributor

I have discovered this issue as well. I propose to remove the platform_linux_common.GNU include from platform_android.GNU (replacing it with the contents we need) since it can be compiled on any of Linux/OSX/Windows (I'm using Windows). A specific platform_android_macosx.GNU only needs to be added if there is a build tool specifically needed on OSX, which in this case it doesn't. We should remove the assumption that ACE for Android will be compiled on Linux and also remove the chance that it will be compiled incorrectly (ie with improper CPPFLAGS).

As for requesting a bugzilla report, I do have an account but I seem to only have the canconfirm and editbugs permissions. I can't find any way to request those permissions. Assistance would be appreciated.

This comment has been minimized.

Sign in to view

Copy link
@dlifshitz-maca

dlifshitz-maca Aug 2, 2016

Contributor

I have created http://bugzilla.dre.vanderbilt.edu/show_bug.cgi?id=4216 for this issue
ifdef PLATFORM_LARGEFILE_CFLAGS
# Large file support
CPPFLAGS += $(PLATFORM_LARGEFILE_CFLAGS)
endif #largefile

SYSARCH := $(shell uname -m)

PIC = -fPIC
Expand Down

5 comments on commit 381c152

@jwillemsen
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlifshitz-maca Just gave you permissions for bugzilla

@jwillemsen
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open a pull request for the bugzilla

@dlifshitz-maca
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have attached a patch to the bugzilla. The readme says this is acceptable for an Occasional Contributor. Is that method deprecated?

@jwillemsen
Copy link
Member Author

@jwillemsen jwillemsen commented on 381c152 Aug 2, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do prefer github pull requests, way easier, just updated the README.md to only mention github pull requests as a way to contribute patches, sorry for the inconvenience

@dlifshitz-maca
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, I just don't have a repo set up yet. Will do.
Thanks for updating the doc to reflect this. I'd suggest removing the "Attach a patch file to the Bugzilla issue" section and fixing the typo "A occasional".

Please sign in to comment.