build: Add de-duplication for non-result targets in get_all_link_deps

[RobotLeopard86]

Note

This is my first time submitting a PR to Meson, so I hope I did everything right. I just followed the Contributing page on the website.

I recently started having an issue configuring one of my projects on Windows. Meson would get through configuration and just never finish and spit out a build.ninja. I looked into it, and the problem seemed to be that in get_all_link_deps (build.py), the high numbers of interconnected static libraries from LLVM were causing the stack to get stuck in an infinite loop of adding the same libraries over and over, so it never emptied.

I fixed this by adding a second set of "nonresults": targets that are not results, but we have seen and thus already know to ignore for this calculation. Any potential target to be added to the stack must not be in the nonresults list. It's possible that this check incurs a slight performance penalty due to iterating over the targets and adding them one by one instead of in one huge batch. However, it does allow everything to successfully configure and have a Ninja file actually be generated.

I know that feature freeze is in 2 days, so this is a bit of a rushed, quick patch, but I hope I did everything correctly and this can get in for Meson 1.10.0.

I'm not the best with Python or Meson's internals (I'm a C++ guy), so I hope I did everything right. This shouldn't affect anything that relies on get_all_link_deps since it doesn't change the return value.

[RobotLeopard86]

Update: I rebased the changes from upstream into my fork via the GitHub interface. I made sure the button said "Rebase and merge". If I have done something incorrectly, please let me know.

@jpakkane Can you or someone else that is on the team please take a look at this?

[bonzini]

Nope, unfortunately it didn't do the right thing. To rebase just use git rebase origin/master from the command lines, then force-push the branch with git push -f.

[bonzini]

Also, please use git rebase -i origin/master and change the "fix" patch from pick to f, so that you get a single patch. Thanks in advance!

[bonzini]

This can be a regular set:

nonresults: T.Set[BuildTargetTypes] = set()

which is faster than the ordered one.

[RobotLeopard86]

Will do, thanks! I don't typically rebase, so not an area of Git I'm great with. Will try to resolve when I can.

[RobotLeopard86]

Okay, changes made and rebase (hopefully) corrected! Let me know if anything else needs to be tweaked.

[bonzini]

Thanks, it's an improvement but there are changes to make:

1. the rebase didn't remove the new upstream commit. Not knowing your exact configuration, git pull --rebase https://GitHub.com/mesonbuild/meson will do.

2. it's T.Set not Set

3. this has a problem:

                for t2 in t.link_targets:
                    if t2 not in nonresults:
                        stack.appendleft(t2)

It will add t.link_targets to the stack in opposite order compared to the incoming list. You can achieve what the original code does with the following:

stack.expandleft((t2 for t2 in t.link_targets if t2 not in nonresults))
stack.expandleft((t2 for t2 in t.link_whole_targets if t2 not in nonresults))

[RobotLeopard86]

Okay, hopefully that fixed it. I ran the rebase command you provided, and I think it worked since I don't see "RobotLeopard86 committed" all over the commits anymore. Sorry for having issues with the system; like I mentioned, I don't use rebase typically, so I'm bad with managing that.

Hopefully this can be merged now if everything is good.

[bonzini]

It does look good to me!

[RobotLeopard86]

All right, all checks passed. Is this ok to merge then?