Ticket #534 (closed todo: fixed)

Opened 5 years ago

Last modified 4 years ago

Get rid of PARROT_NET_DEVEL

Reported by: Infinoid Owned by: whiteknight
Priority: minor Milestone:
Component: core Version:
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

The sockets/networking support in parrot is conditional based on a define, PARROT_NET_DEVEL. This field is internal to the IO subsystem (so I don't think deprecation rules apply), it lives in src/io/io_private.h. It was added in r4381, and modified to a true value in r7974. It doesn't appear to have been touched since.

Both of the svn revisions I mentioned above happened in 2005. Do we really need this to be conditional?

I propose we either rename it to PARROT_NETWORKING_SUPPORT (bacek++ for suggesting this) and generate it from Configure.pl, or just get rid of it.

Change History

in reply to: ↑ description ; follow-up: ↓ 2   Changed 5 years ago by jkeenan

Replying to Infinoid:

I propose we either rename it to PARROT_NETWORKING_SUPPORT (bacek++ for suggesting this) and generate it from Configure.pl, or just get rid of it.

1. What would be the effect of simply eliminating PARROT_NET_DEVEL from src/io/io_private.h? It appears to always be true, so the resulting code would simply be:

#ifdef UNIX
#  include <sys/socket.h>
#endif

#ifdef WIN32
#  include <winsock.h>
#endif

Correct?

2. If it were handled during configuration:

(a) Would it be the result of a probe?

(b) Would it be user-configurable?

(c) In which step would we set it?

(d) What would be the impact if a user set it to 0?

Thank you very much.
kid51

in reply to: ↑ 1   Changed 5 years ago by Infinoid

Replying to jkeenan:

1. What would be the effect of simply eliminating PARROT_NET_DEVEL from src/io/io_private.h? It appears to always be true, so the resulting code would simply be: {{{ #ifdef UNIX # include <sys/socket.h> #endif #ifdef WIN32 # include <winsock.h> #endif }}} Correct?

Correct. Though, there's a nearby XXX comment in that header suggesting we probe for the appropriate socket header files. If we implement that, I guess the result will probably look different from the above.

(a) Would it be the result of a probe?

Whether or not the platform supports sockets, and what to include (and/or link against) to get them. If we do add a "support networks" flag, I guess it should be named something more appropriate than the current "PARROT_NET_DEVEL", which looks more like a debugging flag than a feature selection.

(b) Would it be user-configurable?

If Configure.pl probes for it, it seems appropriate that it should accept command line arguments related to it as well.

(c) In which step would we set it?

I don't have a clue.

(d) What would be the impact if a user set it to 0?

I would guess the sockets support should be disabled. More specifically, the Socket and Sockaddr PMCs would not be built in (or stubbed out somehow), and the lower level functions in socket_api.c should toss EXCEPTION_UNIMPLEMENTED when called. But I'm just guessing, I haven't put much thought into the best way to handle the details.

I think the runtime configuration stuff can be deferred until the point if/when we try to build on a platform that doesn't have any networking support at all. (Does such a thing still exist? How do we retain pbc portability in this case?) In the meantime, just enabling networking in all cases would be my vote.

Mark

  Changed 5 years ago by whiteknight

  • status changed from new to assigned
  • owner set to whiteknight

I'm happy to change this to use PARROT_NETWORKING_SUPPORT, that seems sane at the least. If set to 0, I agree that all the socket functions should throw EXCEPTION_UNIMPLEMENTED.

If this all sounds sane to everybody else, or if this message gets completely warnocked, I can add this in the io_cleanups branch ASAP.

follow-up: ↓ 6   Changed 5 years ago by whiteknight

See r39778, r39787, and r39788 for changes I've made in relation to this ticket.

  • the macro is now called PARROT_NETWORKING_SUPPORT
  • all the API functions consistently throw EXCEPTION_UNIMPLEMENTED when we don't have networking support.
  • We can query Parrot to determine whether we have this support by calling $I0 = interpinfo INTERPINFO_NETWORKING_SUPPORT (which I need to add tests for still).

These changes should satisfy this ticket. Once the branch merges to trunk (which should be soon) if I don't hear any complaints I will close this.

  Changed 5 years ago by jkeenan

  • component changed from none to core

in reply to: ↑ 4   Changed 4 years ago by jkeenan

Replying to whiteknight:

See r39778, r39787, and r39788 for changes I've made in relation to this ticket.

These changes should satisfy this ticket. Once the branch merges to trunk (which should be soon) if I don't hear any complaints I will close this.

Infinoid, Whiteknight:

Can we close this ticket?

Thanks.

kid51

  Changed 4 years ago by whiteknight

No, the branch in question never merged and was deleted because it was too old and unmaintained. I will take a look at this in more depth soon.

  Changed 4 years ago by coke

  • status changed from assigned to closed
  • resolution set to fixed

While whiteknight++ did some work to clean this up in a branch a year ago, it never got merged back.

Taking the path of least resistance and removing the #define (r47583). If someone wants to build parrot on a machine with no networking support at all, they can re-add an option like this then.

Note: See TracTickets for help on using tickets.