Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 276640 - MAC OS: The C code needed to build AC on Mac OSX
Summary: MAC OS: The C code needed to build AC on Mac OSX
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: TPTP (show other bugs)
Version: unspecified   Edit
Hardware: Macintosh Mac OS X - Carbon (unsup.)
: P2 major (vote)
Target Milestone: ---   Edit
Assignee: Paul Klicnik CLA
QA Contact: Kathy Chan CLA
URL:
Whiteboard: close472
Keywords:
Depends on:
Blocks: 68111
  Show dependency tree
 
Reported: 2009-05-17 20:02 EDT by Spundun Bhatt CLA
Modified: 2016-05-05 10:50 EDT (History)
1 user (show)

See Also:


Attachments
c code patch guarded by __APPLE__ macro (10.44 KB, patch)
2009-05-17 20:13 EDT, Spundun Bhatt CLA
no flags Details | Diff
adds new c files for apple (22.60 KB, patch)
2009-05-17 20:19 EDT, Spundun Bhatt CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Spundun Bhatt CLA 2009-05-17 20:02:19 EDT
Other than the two patches I submitted earlier, this is the C Code that is needed to build AC on Mac OSX intel core2 duo, 10.5.

All the code here is guarded by macro #ifdef __APPLE__ , including new files. None of the code posted in this patch should alter any line compiled by non apple builds. So it is as safe to include in cvs as I could make it.

It's not very big either. I hope this makes it into the repo soon.
Comment 1 Spundun Bhatt CLA 2009-05-17 20:13:07 EDT
Created attachment 136138 [details]
c code patch guarded by __APPLE__ macro

here is a patch that alters existing file, adding only code guarded by __APPLE__ macro (or adding "|| defined __APPLE__"  condition to already existing macro switch
Comment 2 Spundun Bhatt CLA 2009-05-17 20:19:14 EDT
Created attachment 136139 [details]
adds new c files for apple

This patch adds 3 new c files, src-native-new/src/shared/TPTPUtil/TPTPOSCalls_apple.c src-native-new/src/transport/transportSupport/TransportSupportNamedPipe_apple.c and ssrc-native-new/src/transport/transportSupport/TransportOSCalls_apple.c . I haven't thoroughly examined the code in here but made sure that they are entirely guarded by ifdef __APPLE__ so none of this code is used on other platforms. Also this code builds on my Mac OSX 10.5 with intel core2 duo, so there's that.
Comment 3 Kathy Chan CLA 2009-05-22 10:10:25 EDT
Thank you for contributing these patches.  It's great to have the community contribute to the porting of the AC on Mac OS. We'll take a look at the patch and get back to you.  

Could you please clarify on what kinds of scenarios you were able to do on MAC OS with all these patches?  May be you could open a top level bugzilla on porting the AC to MAC and we could continue our general discussion there?  These bugs can then be a dependency for that top level bugzilla.
Comment 4 Spundun Bhatt CLA 2009-05-23 08:19:41 EDT
Hi Kathy,

I am trying to run AC server on mac osx. I have gotten it to compile but the sampleclient still seems to run into errors somehow. I'm making slow progress.

Most of the patches here are originally submitted on bug #68111 , and I have cleaned up and re-structutred for readability. I have also added fixes where I saw the need or the code diverged from the original version so that patch wouldn't apply anymore, in such cases I studied the context of the patch and re worked it to fit the HEAD version of the code.

I thought of bug #68111 as the top level bug for these issues which is blocked by all these bugs that I've created, I'm glad you like this system, I intended for it to make easier for the maintainers to commit these patches, because they are more readable and in general sorted based on issues. If you have a better idea for organising these bugs and tasks then feel free to restructure these bugs in bugzilla or create a new top level bug.

In my opinion, patches in #276591 and #276607 are very small, very readable and obvious fixes. so is the fix #276615 for CBE build. I this those patches could be committed blindly. The code in this bug here has more logic than the others. But al the code here is guarded by macros so that it won't be participating in any non-apple build. So even if it may need fixing later, to get it to work on apple, it will not break AC on linux or windows or any other build.

My hope is that if we get the already existing patches in cvs (I see that there are already so #ifdef __APPLE__ in the cvs code, so someone has previously committed some code for apple) then the people who come later to contribute effort here will have to spend less energy to get up to speed.  I have to confess, I don't think I've even gotten to the point where someone (I think Paul) was, he got a couple of agents working with AC through commandline, and I think I'm really close to getting there but I'm not there yet.

Hope this answers all your questions.
Comment 5 Kathy Chan CLA 2009-05-25 17:01:08 EDT
Thanks!

Paul, please take a look at merging these MAC OS patches to the current code base in HEAD for post TPTP 4.6.
Comment 6 Kathy Chan CLA 2009-07-13 15:54:11 EDT
Hi Spundun,

Could you please add a comment to this bugzilla to confirm that you authored 100% of the content submitted in this patch and you have the rights to donate the content to Eclipse under the EPL (http://eclipse.org/legal/epl-v10.html)? Note that having answer to the above question is part of the requirement for getting legal approval for checking in your contributions to CVS.
Comment 7 Spundun Bhatt CLA 2009-07-21 19:50:23 EDT
-> I Authorize all the work that I've done, to be used under EPL, by the project that this bugzilla currently belongs to.

-> It should be noted that some of the contribution has come under my time at work with my employer "University of Southern California, Information Sciences Institute". The official url is www.isi.edu . 

-> Please understand that my efforts were mostly spent in re-organizing existing patches and code. This code was already contributed to bug #68111 by other folks. I *did* contribute improvements to the existing patch, in terms of better organization, readability, and overall compatibility with current CVS head.
Comment 8 Spundun Bhatt CLA 2009-07-21 19:53:03 EDT
Also  I have gotten the approval of my employer for this contribution
Comment 9 Kathy Chan CLA 2009-07-28 14:27:05 EDT
Please add the following to the header of all files contributed by Spundun:

Contributed by Spundun Bhatt (spundun@gmail.com), with the support and encouragement of the University of Southern California Information Sciences Institute Distributed Scalable Systems Division.
Comment 10 Kathy Chan CLA 2009-07-29 12:34:37 EDT
Note that the CQ for Spundun's contribution to RFE bug 68111 (and it's child defects bug 276591 bug 276607, bug 276615 and bug 276640) have been approved by the Eclipse IPO under CQ 3397 (https://dev.eclipse.org/ipzilla/show_bug.cgi?id=3397).  The patch can be checked in now with the above update to the header of all files contributed by Spundun.
Comment 11 Jonathan West CLA 2009-08-06 23:27:19 EDT
Patch checked into HEAD with the following comments:
- I'd recommend removing the ptrace code from TPTPProcessController.cpp, and replacing it with the ptrace-less code process death code we are presently using on many platforms. Search for _SOLARIS or _AIX in the file to see the relevant ifdefs.
- If necessary, platform non-specific atomic operations are available for the RASharedMemory component. Look at how the USE_PTHREAD_ATOMICS definition is used. It is presently being used for Linux but should work with any Unix.
Comment 12 Kathy Chan CLA 2009-08-07 09:29:29 EDT
Jonathan, since the patch has been checked in, could this defect be resolved?
Comment 13 Kathy Chan CLA 2009-09-11 11:31:24 EDT
Spundun,

Could you please take a look at Jonathan's comment 11 and see if you could address them?
Comment 14 Spundun Bhatt CLA 2009-10-28 17:35:36 EDT
Hi Kathy, Jonathan.

Sorry for such a late response.
Kathy, Jonathan clearly has a better understanding of the software itself and if he recommends switching to the ptrace-less code used for other platforms then I have no objections. I took a quick look at the code in the TPTPProcessController.cpp. 
- I found a couple of hunks that looked like this	#if defined(__linux__) || defined(_SOLARIS) || defined(_AIX) || defined(MVS) . I think these predicates are meant to keep out the code on win32 platform, but it's also excluding __APPLE__ . Would'nt it be better if they put if !(defined _Win32) instead?
- Looks like the ptrace code is used only on linux, I couldn't figure out exactly what the purpose of the ptrace code is, and why you only need it on linux
About the second point, again jwest probably knows a lot more than me at this point so whatever his suggestion is, is fine with me. I'm not sure exactly how to implement it.

I do have a question. Have you guys considered using autoconf and autoheader type tools to figureout the system configurations? I think that would make the code a lot simpler.
Comment 15 Jonathan West CLA 2009-10-29 11:10:31 EDT
Hi Spundun, I had considered autoconf in the past, however, the target platforms for the agent controller include/may include z/OS and AS/400, where availablity/support of the GNU tool chain is not assured. In addition, the code base itself is an amalgamation of two separate code bases, one of which is required for backwards compatibility, so our options for centralization are limited. 

As for your question, in cases where we intend to say UNIX we usually say !WIN32, and for the cases we miss these will be fixed the next time someone is working in that code area.

As for ptrace use, there are no longer any platforms that use this ptrace code. Something like the doCheckProcess() function may still exist in the code, but it is dead code that is no longer called by anything. We now use checkForProcessDeath() as its replacement.
Comment 16 Kathy Chan CLA 2009-12-21 10:49:37 EST
Spundun, 

Please address Jonathan's comment and resubmit a delta patch.  We could then try to get it into TPTP 4.7.  Thanks!
Comment 17 Jonathan West CLA 2010-05-12 14:15:32 EDT
Retargeting to 4.7.1
Comment 18 Kathy Chan CLA 2010-05-28 11:09:33 EDT
Bug 314892 has been opened to address comment #15.  Resolving this for 4.6.1.
Comment 19 Kathy Chan CLA 2011-02-11 13:46:03 EST
This defect had been resolved as FIXED for more than 1 month.  Please verify with the latest TPTP 4.7.2 driver.  If this defect is still left unverified by February 25, we'll close it on the originator's behalf.

TPTP 4.7.2 driver can be downloaded from:

http://www.eclipse.org/tptp/home/downloads/?ver=4.7.2
Comment 20 Kathy Chan CLA 2011-04-09 16:40:23 EDT
Closing on behalf of the reporter.