Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Apr 6, 2019

Since fbrt0.o and libfb.a are statically linked to the bootstrap fbc,
rtlib should be listed as a dependency of BOOTSTRAP_FBC. This patch
fixes the race condition described in issue #131.

Since fbrt0.o and libfb.a are statically linked to the bootstrap fbc,
rtlib should be listed as a dependency of BOOTSTRAP_FBC. This patch
fixes the race condition described in issue #131.
@countingpine
Copy link
Collaborator

Yeah, I think this makes sense.
Probably better for @jayrm to give the official verdict, so I'll give him a chance to weigh in, but it looks good to me.

@ghost
Copy link
Author

ghost commented Apr 7, 2019

I came across this issue while compiling a bootstrap FreeBASIC compiler to compile the FreeBASIC compiler in 1.06.0.tar.gz:

cc => bootstrap_fbc => fbc

I noticed that for this use case, compiling gfxlib2 is unnecessary because the fbc source code does not depend on it. Would it make sense to remove the gfxlib2 dependency from the bootstrap make recipe in this patch?

Would it instead be better to open a second pull request to implement a new make recipe for this scenario: bootstrap-minimal which compiles a bootstrap compiler with the minimal features (e.g. rtlib dependency) necessary to compile another fbc?

@jayrm
Copy link
Member

jayrm commented Apr 8, 2019

@vilhelmgray, yes, makes sense to me also. Thanks for finding this. I will merge in. Since this pull request specifically fixes issue #131, making a separate pull request for bootstrap-minimal sounds like a good approach to tracking the changes.

The only time fbgfx library is needed is if any gfx function used, or user explicitly includes fbgfx.bi. The fbc compiler does neither, so a bootstrap-minimal would be a nice additional option for the builder.

@jayrm jayrm merged commit 9d996f4 into freebasic:master Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants