search

openwall / tcb

Alternative password shadowing scheme
https://www.openwall.com/tcb/
Other
8 stars 3 forks source link

Small improvements to Makefiles. #2

Closed besser82 closed 3 years ago

besser82 commented 3 years ago

libnss_tcb: Apply proper soname during linking.

Rationale from the GNU libc manual Section 29.4.1:

Developers of a new service will have to make sure that their module is created using the correct interface number. This means the file itself must have the correct name and on ELF systems the soname (Shared Object Name) must also have this number.

libnss_tcb: Drop unneeded LIBNSL from linked libraries.

The NIS+ support has been removed long time ago, so there is no need to link against LIBNSL anymore.

make: Allow install and mkdir programs to be user configurable.

besser82 commented 3 years ago

The commit that removes $(LIBNSL) should probably contain

Complements: b91e465a92806dc94b4fa8d6290b583862dd508a

Added in commit msg.

besser82 commented 3 years ago

LGTM. Should the -p option to mkdir be part of MKDIR, though? We don't (can't) include -d in INSTALL (because we use INSTALL on files as well). Also, given that we do specify explicit permissions when installing files, shouldn't we add -m 755 to uses of MKDIR? (I'm not specifically requesting these changes - just thinking out loud.)

Sounds reasonable to me. I'll change that in a minute to specifiy the options to MKDIR in the Makefile explicitly.

solardiz commented 3 years ago

Thanks @besser82! In what order would you like your 3 PRs to be merged? You'll need to be rebasing your additions to ChangeLog in two of them accordingly.

besser82 commented 3 years ago

Thanks @besser82! In what order would you like your 3 PRs to be merged? You'll need to be rebasing your additions to ChangeLog in two of them accordingly.

You're welcome, @solardiz! Let's merge from 4 to 1.

I have a fourth PR in the pipeline, but I'll wait to push until we merged the other ones.

besser82 commented 3 years ago

@solardiz This one is ready now.

solardiz commented 3 years ago

@besser82 Oh, now I see I missed a typo in two places in the previously merged PR - it says fedoraporject.org. Now you'll probably want to fix that separately. Not a reason to delay merging this PR, so I'll merge it now.