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

root 6.22.06 #65825

Closed
wants to merge 2 commits into from
Closed

root 6.22.06 #65825

wants to merge 2 commits into from

Conversation

chenrui333
Copy link
Member

Created with brew bump-formula-pr.

@BrewTestBot BrewTestBot added the python Python use is a significant feature of the PR or issue label Nov 27, 2020
@chenrui333
Copy link
Member Author

chenrui333 commented Nov 27, 2020

on 10.15 and 11.0

==> Testing root
/usr/bin/sandbox-exec -f /private/tmp/homebrew20201127-61854-132o6x3.sb ruby -W0 -I $LOAD_PATH -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/root.rb --verbose
==> /usr/local/Cellar/root/6.22.06/bin/root -b -l -q -e gSystem->LoadAllLibraries(); 0

(int) 0
==> root -l -b -n -q test.C
==> /bin/bash test_compile.bash
==> /usr/local/opt/[email protected]/bin/python3 -c import ROOT; ROOT.gSystem.LoadAllLibraries()
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'ROOT'

on 10.14

==> /usr/local/Cellar/root/6.22.06/bin/root -b -l -q -e gSystem->LoadAllLibraries(); 0
2020-11-27 22:27:12.026 root[44525:2309804] isPrefsCreateCacheFromEnabledAndDefaultInputSources - can't find anything from GetInputSourceEnabledPrefs, use defaultASCIIKeyLayoutDict = <CFBasicHash 0x7fb893801080 [0x7fff8b4688f0]>{type = mutable dict, count = 3,
entries =>
	0 : <CFString 0x7fff8b4d37e8 [0x7fff8b4688f0]>{contents = "InputSourceKind"} = <CFString 0x7fff8b518d28 [0x7fff8b4688f0]>{contents = "Keyboard Layout"}
	1 : <CFString 0x7fff8b504e68 [0x7fff8b4688f0]>{contents = "KeyboardLayout ID"} = <CFNumber 0xd41dc6b5e9e839b1 [0x7fff8b4688f0]>{value = +2, type = kCFNumberSInt64Type}
	9 : <CFString 0x7fff8b4ce4e8 [0x7fff8b4688f0]>{contents = "KeyboardLayout Name"} = British
}

(int) 0
==> root -l -b -n -q test.C
2020-11-27 22:27:13.285 root[44529:2309852] isPrefsCreateCacheFromEnabledAndDefaultInputSources - can't find anything from GetInputSourceEnabledPrefs, use defaultASCIIKeyLayoutDict = <CFBasicHash 0x7fd158f0c270 [0x7fff8b4688f0]>{type = mutable dict, count = 3,
entries =>
	0 : <CFString 0x7fff8b4d37e8 [0x7fff8b4688f0]>{contents = "InputSourceKind"} = <CFString 0x7fff8b518d28 [0x7fff8b4688f0]>{contents = "Keyboard Layout"}
	1 : <CFString 0x7fff8b504e68 [0x7fff8b4688f0]>{contents = "KeyboardLayout ID"} = <CFNumber 0x9c639056006fe85 [0x7fff8b4688f0]>{value = +2, type = kCFNumberSInt64Type}
	9 : <CFString 0x7fff8b4ce4e8 [0x7fff8b4688f0]>{contents = "KeyboardLayout Name"} = British
}
==> /bin/bash test_compile.bash
==> /usr/local/opt/[email protected]/bin/python3 -c import ROOT; ROOT.gSystem.LoadAllLibraries()
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'ROOT'

@chenrui333
Copy link
Member Author

test out fine on my 10.14 machine locally

==> Testing root
/usr/bin/sandbox-exec -f /private/tmp/homebrew20201127-49306-1p1v224.sb ruby -W0 -I $LOAD_PATH -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/root.rb --debug --verbose
/usr/local/Homebrew/Library/Homebrew/test.rb (Formulary::FromPathLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/root.rb
==> /usr/local/Cellar/root/6.22.06/bin/root -b -l -q -e gSystem->LoadAllLibraries(); 0

(int) 0
==> root -l -b -n -q test.C
==> /bin/bash test_compile.bash
/usr/local/Homebrew/Library/Homebrew/test.rb (Formulary::FormulaLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/[email protected]
==> /usr/local/opt/[email protected]/bin/python3 -c import ROOT; ROOT.gSystem.LoadAllLibraries()
NOTE:  Most system logs have moved to a new logging system.  See log(1) for more information.

@chenrui333 chenrui333 added the CI-requeued PR has been re-added to the queue label Nov 27, 2020
@gromgit gromgit mentioned this pull request Nov 28, 2020
4 tasks
@henryiii
Copy link
Contributor

Is this a problem on ROOT's side or homebrew's side? Sounds a little (maybe also from #65840) that this might be on homebrew's side?

@SMillerDev
Copy link
Member

==> /usr/local/opt/[email protected]/bin/python3 -c import ROOT; ROOT.gSystem.LoadAllLibraries()

This fails because it can't find ROOT. I'm not sure if that changed between the last versions, but I'd guess it's a code change that the install didn't adapt to yet.

@henryiii
Copy link
Contributor

henryiii commented Dec 6, 2020

@chenrui333 Could you give me access to your branch in your fork? I'll have some time to debug this tomorrow.

@fxcoudert fxcoudert removed the CI-requeued PR has been re-added to the queue label Dec 7, 2020
@henryiii
Copy link
Contributor

henryiii commented Dec 8, 2020

Should I open up a new PR for debugging? Actually, if I PR to my own repo, maybe the actions will run and I can debug that way, and then I can suggest here whatever I find. I'll try that first.

@henryiii
Copy link
Contributor

henryiii commented Dec 8, 2020

I can reproduce this locally on 10.15! Don't need CI now, at least for the 10.15 problem. :)

@henryiii
Copy link
Contributor

henryiii commented Dec 8, 2020

Found the problem with finding ROOT, at least fixing 10.15 and 11.0. Before, everything was in /usr/local/Cellar/root/6.22.04/lib/root; now everything is in /usr/local/Cellar/root/6.22.06/lib, there is a folder called ROOT in that with __init__.py in it, which is why the directory exists, but doesn't really do anything when added using the .pth file. I'm checking with the ROOT team to see if this was intentional - it's a big reordering for a patch release, especially after they assured me that nothing significant was supposed to change with the install. :)

@henryiii
Copy link
Contributor

henryiii commented Dec 8, 2020

It should not be dumping everything in /lib, it should be in /lib/root due to the way Homebrew links. I've verified by building 6.22.04 (with the GitHub tarball, since the release was pulled) that it now does this too, so it's a change in Homebrew, not ROOT. I've also rebuilt the CMake formula with 3.18 and retried, with the same result, so it's not due to a CMake update. Has possibly a variable been removed or added that might affect a gnu style install?

@henryiii
Copy link
Contributor

henryiii commented Dec 9, 2020

Found the bug with the help from members of the ROOT team, @oshadura and @amadio ! The problem is this line in homebrew, which was added here. The libdir is now specified (but not all the other GNU paths?), so this breaks ROOT.

There are two options.

  1. We can specify this back to what we want it to be, /lib/root. This will recover the old behavior, where ROOT lives in a nested folder, /lib/root/stuff.
  2. Or, we can remove gnuinstall=ON (OFF is the default), and then ROOT will install itself into un-nested /lib and such. The problem is it is huge, and has a lot of commonly named files, like libMath.so and RConfig.h (colliding with at least R), so this doesn't live as nicely with other packages when it is OFF. But it is more "standard". At least conda-forge and Gentoo have this OFF.

Thoughts?

@henryiii
Copy link
Contributor

@chenrui333 I still can't edit your branch, so my recommendation is:

Add:

-DCMAKE_INSTALL_LIBDIR=lib/root

to the cmake line. Put a comment like this above the place these get set:

# Homebrew now sets CMAKE_INSTALL_LIBDIR to /lib, which is incorrect
# for ROOT with gnuinstall, so we set it back here.

I bet they set it to avoid it being set to lib64. Technically, ROOT maybe should install to CMAKE_INSTALL_LIBDIR/root in gnuinstall mode, as these variables are supposed to be set to the lib, not lib + something, I think. Anyway, this should unblock this PR.

@chenrui333
Copy link
Member Author

I will give this a shot.

@chenrui333
Copy link
Member Author

@henryiii also, you should be all set with permissions to my branch. :)

@henryiii
Copy link
Contributor

henryiii commented Dec 10, 2020

Thank you, though hopefully this will Just Work™, and I won't need to debug further. :)

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Thanks @chenrui333 and @henryiii ! Without contributions like yours it'd be impossible to keep homebrew going with the high standards that users have come to expect from the project. You can feel good knowing that you've made the world a tiny bit better for homebrew users around the world! 👍 🎉

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@chenrui333 chenrui333 deleted the bump-root-6.22.06 branch December 18, 2022 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants