-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
DESTDIR support #5063
Comments
Link to newsgroup discussion: https://groups.google.com/forum/#!topic/julia-dev/WQ-Duwlo6gg |
@nalimilan looks like that's been submitted as #5114. |
Yes, I merged that one a couple of days back. Would be great to hear if it does indeed work out for you. |
Actually I need something more: |
@nalimilan I gave you commit access to openlibm and openspecfun. It should be easier to create PRs with branches rather than forks, that way. |
OK, please have a look at this branch: https://github.com/nalimilan/julia/compare/libdir I'd like to know what you think of this solution. The idea is that since This is not a final proposal, as there are many areas to check and I wouldn't want to spend time on it if you think that's not the right way. FWIW, I've found out that Rust has been in the very same situation a few months ago: CC: @staticfloat |
Without testing it, this seems like a fine approach. I will test on OSX once you think you have something testable. |
@staticfloat Please give it a try then, I think it should mostly work. There is still something weird which forced me to change I've also tried to remove as much as possible special-casing for Windows/Mac, considering that we would better set the |
Some comments:
Once you've fixed that I'll continue testing. I just don't think we can merge something that requires a |
Thanks for the testing and comments, I'll try to fix these. One issue that is more difficult is your second point. Indeed it is silly to have to rebuild things only because some install paths change. GNU make's documentation warns against this:
In principle I very much agree with this. But in practice, this does not play well with the fact that Julia embeds rpaths, and that That said, we should be able to much reduce the problem: in theory, only the Julia executables would need to be rebuilt when |
What do people think about this scenario? This would be quite a pervasive change. (I realize it would have been much easier to go on assuming that |
Note that another solution would be to set rpath to point only to the final destination, and to rely on |
It's even better than that; we use (or at least, we should be using) relative RPATHs wherever we can. That means that only changes to In practice, only distributors/packagers are going to be messing with
Unfortunately, this becomes a little hostile to development, and the userbase that doesn't currently use packages to get their latest Julia often has their julia source just plugged into their |
I think the new version should address your concerns. By default Regarding the changes in |
Our |
Cool. Tell me when you have had the chance of testing this. |
@staticfloat Your https://github.com/JuliaLang/julia/commits/sf/libdir branch looks fine AFAICT (I've not tried to understand everything), and it builds fine here, but it triggers a crash in the RPM debugging symbols extraction tool: https://bugzilla.redhat.com/show_bug.cgi?id=1049839 I hope people there will help sorting this out. Also, the TravisCI checks fail quite early (which doesn't seem related). |
If I had to guess, I'd say the RPM debugging symbols extraction tool failure is my fault. I do something that's a little shady in this process, and it's exactly this testing that is important in making sure it doesn't break anything, (which apparently it does) The path to the system image file is hardcoded into the binary. (See this C file and this Makefile) This has to happen because we have no idea where that file is actually located. We can't store it in some other configuration file because the only place we could store that configuration file would be right next to the binary in I thought it would be neat to embed this information into Julia via RPATHs, e.g. we already encode the information of where the This seems to work pretty well, but it unfortunately it seems to screw something up in your process. Perhaps there is a simpler/better solution. I will sleep on it. :) |
It would be interesting to check whether something may be wrong in the way of set the string in |
Ah, and looking a the output of Also, on Linux you may simply rely on the RPATH, which is more standard, and only use the hack on Windows. |
@nalimilan I think I may have found the problem. I was accidentally re-using the string offset across julia executables. Now I correctly calculate the string offset for each executable individually, which should help a lot. This also explains why I want to try and keep the build as similar across platforms as possible. If need be I will special-case, but I'd like to avoid that as much as I can. |
Great, now the crash is gone! But I get an error when starting
Running
|
Where is the binary being built? On your local machine? This looks like a libc mismatch error to me. If we don't do the string patching (e.g. if you comment out these lines in |
Actually, only the last commit introduces the bug. Looks like the call to
So AFAICT this is correct, but maybe there's a problem in the Makefile. I'll leave you find the solution. ;-) |
Can you give me the command you're running in order to create this bug? Everything works fine on my ubuntu machines (as far as I can tell!) so I'd like to reproduce your bug locally, if possible. |
I've only tried this when building my RPM, but here are the commands:
|
I've tried, and I cannot reproduce this. :( Can you send me an RPM that, when installed, crashes? Or are you not to the point of creating an RPM yet? |
I have put the RPM here: http://nalimilan.perso.neuf.fr/transfert/julia-base-0.2.0-2.fc20.x86_64.rpm FWIW, in CFLAGS/CXXFLAGS/FFLAGS is equal to this when I build it:
I have checked that the problem also happens with the |
Well, something is definitely going wrong. What |
gcc (GCC) 4.8.2 20131212 (Red Hat 4.8.2-7) |
Actually, only |
Also, using |
I've downloaded a virtualbox image of fedora 20, do you have scripts or something that you use to generate your Julia install? Can I use them to generate the RPM in the same way that you do? |
Hm, that's a little painful. You need to install RPM packages for double-conversion, openlibm and openspecfun from here: http://nalimilan.perso.neuf.fr/transfert/julia-libdir/ Then run At this point you're almost done: For a few more details: |
Ah, and since you'll need development tools, the short way of installing them is |
You're right, it's not related. It's because of this line; it's the default search path, we don't need to be changing that around. |
Hmmm, quick question. I notice that you're depending on |
Regarding Rmath, I know I should use the Julia-specific version, but for now, using system packages makes building the package faster for testing. I guess it's not related to our current problem either. |
You're right, it's not related. I can reproduce locally, I will try to track down what is going on. Thanks for all your patience. |
Cool! |
You know someone actually understands software engineering when they say On Wed, Jan 15, 2014 at 2:15 AM, Milan Bouchet-Valat <
|
Alright. I've managed to track down why this is happening. Funnily enough, it is NOT because of my The problem is that when we use
This doesn't happen when we don't use
at the top of |
Yikes. I'm impressed that you tracked this down. |
Well thank you, Stefan. Working on Julia has taught me more about dynamic linking, executable file formats, and plenty of other strange topics that I wouldn't otherwise have had reason to learn about. Speaking of which, I think I've found the problem with |
The direction this is heading is clearly that we're going to have our own linker. |
Orz. I've opened an issue on the patchelf github repo. Hopefully this will be a simple fix, and we can just download the latest |
Wow. This one was deep in the stack... Looks like nobody before you has ever tried to write a program which would at the same time use Regarding practical issues, even if |
Sorry, yes that's exactly what I meant.
|
Hey, it works! ;-) So as far as I'm concerned, this branch is OK. The bug should only affect people modifying |
Alright. I will rebase, do a final check pass and open a PR. On Fri, Jan 17, 2014 at 2:24 PM, Milan Bouchet-Valat <
|
I've seen a5c253a. Thanks for doing the cleaning! Don't you think it would be useful to give a few more details and a link to this issue? |
Hahaha, I'm still working on it, hence the |
OK. |
Status update: almost there! I've gotten the windows cross-compile finishing nicely, but All these changes are really nice, I think this overhaul has been a long time coming. Thank you for doing this initial work, @nalimilan! A nice byproduct of this effort is that some of our windows special cases are now handled automatically simply by changing |
our install target is now much more compliant with expectations |
As I said in my mail to the list, this is one of the points that would make packaging easier. Currently we need a patch in particular (see the Debian one in [1]) so that we can specify SYSCONFDIR=/etc (the value when the package will be installed) and still get the files to be installed to PREFIX/../etc.
Since apparently Julia does is completely relocatable, i.e. PREFIX is not hardcoded anywhere, we could live without DESTDIR. But it would still be nice since it would make things simpler:
make install
should install files to $DESTDIR/$PREFIX and $DESTDIR/$SYSCONFDIR [2]. This would be cleaner than using the PREFIX/../etc. solution.Finally, libuv uses DESTDIR, so currently I need to pass both DESTDIR and PREFIX to
make install
.1: http://ftp.de.debian.org/debian/pool/main/j/julia/julia_0.2.0+dfsg-5.debian.tar.gz, at debian/patches/sysconfdir-install.patch
2: http://www.gnu.org/prep/standards/html_node/DESTDIR.html
The text was updated successfully, but these errors were encountered: