Implement linkage for Linux

[sjackman]

extend/pathname: Add os/linux/elf.rb

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew tests with your changes locally?

[MikeMcQuaid]

The above two lines aren't particularly obvious in what they are doing. Can you add a comment and then we can hopefully figure out how to make them more readable?

[MikeMcQuaid]

I'd say pretty much every number in this file should be a constant explaining what it means otherwise they are a bit "magic".

[MikeMcQuaid]

Is StandardError not the default for raise? I can't remember. Also, make this message no more than 80 characters or use <<~EOS`

[MikeMcQuaid]

Don't know what LDD_RX means

[MikeMcQuaid]

A Metadata.new shouldn't be modifying files.

[MikeMcQuaid]

This regex is repeated so consider making it a constant

[MikeMcQuaid]

This doesn't need to be a regex.

[MikeMcQuaid]

This doesn't need to be a regex.

[MikeMcQuaid]

Can probably just make this (*)

[sjackman]

Thanks for the feedback, Mike. I've addressed your comments.

[MikeMcQuaid]

Looking much better! A few more nits, thanks.

[MikeMcQuaid]

MAGIC_NUMBER_OFFSET

[MikeMcQuaid]

Use MAGIC_NUMBER.size

[MikeMcQuaid]

OS_ABI_OFFSET

[MikeMcQuaid]

Use OSABI_LINUX.to_s.size

[sjackman]

I've added a read_uint8 helper function.

[MikeMcQuaid]

Use TYPE_EXECUTABLE.to_s.size

[sjackman]

These are not equivalent.
TYPE_EXECUTABLE.to_s.size ≠ TYPE_SIZE
TYPE_EXECUTABLE.to_s.size = 1
TYPE_SIZE = 2

[sjackman]

I've added a read_uint16 helper function.

[MikeMcQuaid]

.first rather than [0]

[MikeMcQuaid]

os_abi

[MikeMcQuaid]

.first rather than [0]

[MikeMcQuaid]

[OSABI_SYSTEM_V, OSABI_LINUX].include?(osabi)

[MikeMcQuaid]

.first rather than [0]

[MikeMcQuaid]

Stick a newline after each return; makes this easier to follow/scan (and consider doing this elsewhere in the file).

[sjackman]

Fixed. Thanks again for the feedback, Mike.

[MikeMcQuaid]

Thanks again @sjackman!

[MikeMcQuaid]

@sjackman Merging this because there's been a lot of back and forth but ideally we'd get some tests to cover a bit more of the diff, here. Can do that in follow-up PR.

[sjackman]

It looks as though Codecov doesn't count lines-of-code as covered that are covered by tests that run only on Linux. Is that the case?
https://codecov.io/gh/Homebrew/brew/compare/0ad42ebbcb735422612a7c6d4edc5ad88044dd4b...d79c5ade1abaad0fb1ea88f2b949def8dcfddb81/diff#diff-TGlicmFyeS9Ib21lYnJldy9vcy9saW51eC9lbGYucmI=

[sjackman]

Thanks for merging, Mike!

[MikeMcQuaid]

It looks as though Codecov doesn't count lines-of-code as covered that are covered by tests that run only on Linux. Is that the case?

@sjackman Correct. If we can figure out a way to upload and combine the coverage that would be good. @reitermarkus looked at this before I think; any thoughts Markus?