Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: don't link to optional_mutex if it doesn't exist #513

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

gperciva
Copy link
Member

No description provided.

@gperciva
Copy link
Member Author

This has no effect on any project which has liball_optional_mutex_normal (such as libcperciva, spiped, and kivaloo). However, if you updated the build system for imds-filterd (for some crazy reason), you would be unable to compile it, because the build would try to link to liball_optional_mutex_normal.a.

This PR is probably pointless for any practical projects, because once we have noeintr_close, we'll need optional_mutex for everything. But it's rather untidy to have a build system which just plain breaks if we don't have certain files.

Makefile.inc Outdated
.if !defined(NOLIBALL)
.if exists(${FINALIZED_SUBDIR_DEPTH}/liball/optional_mutex_normal)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no firm feelings about what we should have here. We could spell out the full path to liball_optional_mutex_normal.a (or even that and liball_optional_mutex_pthread.a!).

I initially had the full path, but it looked messy, and I figured the directory would be sufficient to be confident that the project either had optional_mutex or not.

# subdirectory depth, but it's not fully set at this stage in the build
# process. Instead, we jump back to the shell and print the (finalized)
# value of SUBDIR_DEPTH.
FINALIZED_SUBDIR_DEPTH != ${MAKE} -v SUBDIR_DEPTH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this doesn't result in an infinite loop. Running make -V SUBDIR_DEPTH would process the Makefile and come across this line again, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short answer: because make -v SUBDIR_DEPTH gets it from Makefile (where it's defined in full), not Makefile.BSD.

Medium answer: I'll update the comment.

Better answer: hmm, that's weird, make -f Makefile.BSD -v SUBDIR_DEPTH and -V FINALIZED_SUBDIR_DEPTH both work, instead of going into an infinite loop. I need to investigate more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addendum to that "better answer": even if we run make -f Makefile.BSD -v SUBDIR_DEPTH on the command-line, the top-level Makefile.inc runs make -v SUBDIR_DEPTH (which has the fully defined value) so there's no loop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so what happens if we run make Makefiles and there isn't a Makefile already? We get a broken one and then it gets fixed the next time we run make Makefiles?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not broken. (Although admittedly, that's a happy accident rather than by design.)

metabuild.sh line 25:

SUBDIR_DEPTH=$(${MAKEBSD} -v SUBDIR_DEPTH)

That works fine regardless of whether there's a Makefile or not.

metabuild.sh line 172 puts SUBDIR_DEPTH into the Makefile. Until that point,

.info ${FINALIZED_SUBDIR_DEPTH}

in Makefile.inc shows that it's empty. But after line 172 of metabuild.sh, it's fine.

Line 179 deals with liball via copyvar_LIBALL_optional_mutex.

I've added a comment to metabuild.sh to note this dependency.

# Link everything to liball.a, unless they specifically ask not to use it.
# If appropriate, metabuild.sh will do:
# s/optional_mutex_normal/optional_mutex_pthread/g
LIBALL = ${SUBDIR_DEPTH}/liball/liball.a ${SUBDIR_DEPTH}/liball/optional_mutex_normal/liball_optional_mutex_normal.a
.else
# There is no optional_mutex; link everything to only liball.a.
LIBALL = ${SUBDIR_DEPTH}/liball/liball.a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write this instead as as LIBALL=.../liball.a followed by an optional LIBALL+=.../liball_optional_mutex_normal.a ?

@gperciva gperciva force-pushed the fix-build-no-optional-mutex branch from 3fea5e7 to 213639b Compare February 21, 2024 19:20
@gperciva
Copy link
Member Author

Updated. The comment for FINALIZED_SUBDIR_DEPTH states why it avoids an infinite loop, and I added a LIBALL += for the optional_mutex stuff.

@gperciva gperciva force-pushed the fix-build-no-optional-mutex branch from 213639b to 521eb68 Compare February 22, 2024 00:51
@cperciva cperciva merged commit 5be14ac into master Apr 7, 2024
2 checks passed
@cperciva
Copy link
Member

cperciva commented Apr 7, 2024

I wish there was a way to do this without needing the FINALIZED_SUBDIR_DEPTH hack, but I don't see any better alternative.

@gperciva gperciva deleted the fix-build-no-optional-mutex branch April 7, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants