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

Fix Mold-search in older systems #100827

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Conversation

OS-of-S
Copy link
Contributor

@OS-of-S OS-of-S commented Dec 26, 2024

I fixed a bug in a compiling script so that using Mold for faster development corresponds to official documentation.

This bug show itself if the user's computer has an old version of GCC installed. As you can see in the commentary inside code, it affects systems with GCC version < 12.1. Especially old systems of Ubuntu, where user just can't install newer version of GCC. In this case script starts to search for specific mold link inside "/usr/libexec/mold", "/usr/local/libexec/mold", "/usr/lib/mold" and "/usr/local/lib/mold" instead of place, where user installed mold binaries manually (the way as it said in documentation), and prints an error.

I didn't change the existing code, but just added a search for Mold link inside user's PATH variable (where Mold should be as mentioned in documentation). So the old looking-for-mold-in-specific-folders will still work, despite the fact that this point is missed in the documentation.

Here is a screeshot of terminal before and after fix.
image

How this bug occures (my hypothesis):

  1. Godot developers uses newer versions of software and missed that issue, or...
  2. Godot developers uses self-compiled version of Mold, becouse cmake install installs Mold right into one of the folders, mentioned in the script. User don't even need to add it into PATH. And this even more possible becouse for 32-bit Linux Mold is not provided in compiled form. In any case, this shortcoming went unnoticed.

It should be fixed in some way, otherwise, users risk encountering a problem and having troubles with finding ways to solve it.

UPD: I checked mold-1.1 and mold-2.35.1 files as the first mold version wich have compiled binaries and the last mold release respectively. Both have the same file structure, and I don't think any other version between should be checked. The added code will find Mold link in any of those.

@OS-of-S OS-of-S requested a review from a team as a code owner December 26, 2024 17:43
@OS-of-S
Copy link
Contributor Author

OS-of-S commented Dec 26, 2024

I also created a pull-request for documentation changes. It that case code changes weren't needed. But as a result, the documentation may become a bit bloated. So I'd prefer to merge an engine sources and solve the problem unnoticeably for the user.

In any case, if clarification of the documentation is also required, it is already awaiting for approval.

Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

LGTM

@AThousandShips AThousandShips changed the title Fixed Mold-search in older systems Fix Mold-search in older systems Dec 27, 2024
@OS-of-S OS-of-S requested a review from Repiteo December 27, 2024 15:43
@Repiteo
Copy link
Contributor

Repiteo commented Dec 28, 2024

Oop, you'll need to squash the commits before this can be merged. Check out the guidelines from the PR Workflow for a quick rundown!

@OS-of-S
Copy link
Contributor Author

OS-of-S commented Dec 28, 2024

Oop, you'll need to squash the commits before this can be merged. Check out the guidelines from the PR Workflow for a quick rundown!

Done.

@Repiteo Repiteo merged commit 3994e56 into godotengine:master Dec 30, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 30, 2024

Thanks! Congratulations on your first merged contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants