[roland/mman-linux] Move bits/mman-linux.h out of sysdeps/unix/sysv/linux/.
diff mbox

Message ID 20140314200309.83A247449E@topped-with-meat.com
State New
Headers show

Commit Message

Roland McGrath March 14, 2014, 8:03 p.m. UTC
I'm working on a non-Linux port (NaCl) that uses the Linux bits for various
things, including the bits/mman.h bits.  It reduces duplication if I can
just use bits/mman-linux.h, but as things stand that file is only available
in Linux configurations.

It's a bit nonobvious that sysdeps/unix/sysv/linux/Makefile (and soon
sysdeps/nacl/Makefile) has to add bits/mman-linux.h to sysdep_headers
though the header itself is outside of sysdeps/unix/sysv/linux/.  But I
think that wrinkle is preferable to winding up with manual duplication.
If people think this is a bad idea, I can always do something more hokey
in the NaCl port to copy the source file during the build or something.

Opinions?


Thanks,
Roland


2014-03-14  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/unix/sysv/linux/bits/mman-linux.h: Moved to ...
	* bits/mman-linux.h: ... here.

Comments

Carlos O'Donell March 14, 2014, 9:31 p.m. UTC | #1
On 03/14/2014 04:03 PM, Roland McGrath wrote:
> I'm working on a non-Linux port (NaCl) that uses the Linux bits for various
> things, including the bits/mman.h bits.  It reduces duplication if I can
> just use bits/mman-linux.h, but as things stand that file is only available
> in Linux configurations.
> 
> It's a bit nonobvious that sysdeps/unix/sysv/linux/Makefile (and soon
> sysdeps/nacl/Makefile) has to add bits/mman-linux.h to sysdep_headers
> though the header itself is outside of sysdeps/unix/sysv/linux/.  But I
> think that wrinkle is preferable to winding up with manual duplication.
> If people think this is a bad idea, I can always do something more hokey
> in the NaCl port to copy the source file during the build or something.
> 
> Opinions?

I'm happy with such a change. A perfectly structured set of headers is the
enemy of the good. I think reducing duplication of these constants is a good
thing.

Every other way I can think of organizing the header, include_next, or
something else, just looks uglier than moving the header. So your solution
seems like the most reasonable.

On request. Add enough comments that we know why this file is there so
someone reading either Makefile's or the header itself knows why it's
not where it should be.

Cheers,
Carlos.
Roland McGrath March 18, 2014, 9:52 p.m. UTC | #2
> I'm happy with such a change. A perfectly structured set of headers is the
> enemy of the good. I think reducing duplication of these constants is a good
> thing.

I merged it.

> Every other way I can think of organizing the header, include_next, or
> something else, just looks uglier than moving the header. So your solution
> seems like the most reasonable.

I also had a hack where sysdeps/.../Makefile just copied the source file to
the build directory.  That works well enough and avoids nastiness inside
the file itself.  But it's less clean than moving the file.

> On request. Add enough comments that we know why this file is there so
> someone reading either Makefile's or the header itself knows why it's
> not where it should be.

I forgot about this request before I pushed the change.
I'll add some comments now.


Thanks,
Roland
Carlos O'Donell March 18, 2014, 11:27 p.m. UTC | #3
On 03/18/2014 05:52 PM, Roland McGrath wrote:
>> Every other way I can think of organizing the header, include_next, or
>> something else, just looks uglier than moving the header. So your solution
>> seems like the most reasonable.
> 
> I also had a hack where sysdeps/.../Makefile just copied the source file to
> the build directory.  That works well enough and avoids nastiness inside
> the file itself.  But it's less clean than moving the file.

It always irritates me when other build systems do this as it makes it
harder to understand the entirety of the source without having a build
directory. At present we do a pretty good job of keeping the source
structured such that you can read it and understand what it should do.
 
>> On request. Add enough comments that we know why this file is there so
>> someone reading either Makefile's or the header itself knows why it's
>> not where it should be.
> 
> I forgot about this request before I pushed the change.
> I'll add some comments now.

Thanks.

Cheers,
Carlos.

Patch
diff mbox

diff --git a/sysdeps/unix/sysv/linux/bits/mman-linux.h b/bits/mman-linux.h
similarity index 100%
rename from sysdeps/unix/sysv/linux/bits/mman-linux.h
rename to bits/mman-linux.h