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

Add rpath-like $ORIGIN support to compiler flags embedded in cma/cmxa #6642

Closed
vicuna opened this Issue Nov 3, 2014 · 12 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

commented Nov 3, 2014

Original bug ID: 6642
Reporter: @whitequark
Assigned to: @whitequark
Status: closed (set by @xavierleroy on 2016-12-07T10:47:11Z)
Resolution: fixed
Priority: normal
Severity: feature
Fixed in version: 4.02.2+dev / +rc1
Category: ~DO NOT USE (was: OCaml general)
Tags: patch
Monitored by: @gasche @ygrek @hcarty

Bug description

Let me begin with a motivating example. A typical native application consists of a bunch of executables and a bunch of shared libraries that can depend on each other. If you invoke an executable, the dynamic linker would normally only look into the default system paths like /lib, /usr/lib, etc. However, what if you want to make an application that doesn't care where it is installed?

Enter -rpath. -rpath instructs ld to embed a special string in the ELF executable or shared object that provides additional paths to the dynamic linker, a lá LD_LIBRARY_PATH (but it's not global or as fragile). What's more, it also replaces $ORIGIN in these paths with the path to the ELF file itself, allowing the application to be fully relocatable.

OCaml currently has something that's very similar to -rpath--it allows to embed compiler flags (and dllpaths) in the cma/cmxa. However, those paths must be absolute. I propose to make the frontend expand $CAMLORIGIN with the name of cma/cmxa file that is being linked together, so that a cma file in .../lib/ocaml could pass the -L.../lib flag to the linker.

This would greatly help the LLVM OCaml bindings, which currently have to rely on odd hacks, and I imagine there would be other uses as well.

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 20, 2014

Comment author: @whitequark

@gasche, I've attached a patch, can you please take a look?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 22, 2014

Comment author: @damiendoligez

I see 3 problems with this patch:

  1. duplicate definition of replace_origin
  2. replacement fails as soon as there is any $ sign anywhere else in the string
  3. documentation doesn't mention the $(CAMLORIGIN) and ${CAMLORIGIN} syntaxes

I'm not convinced Buffer.add_substitute is the right tool for this job: not only it forces to add a layer of quoting for just one variable, but its quoting conventions are not surjective.

It looks like the $CAMLORIGIN notation only ever makes sense at the beginning of the string anyway, so why not search and replace it with simple substring manipulations?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 28, 2014

Comment author: @whitequark

I've updated the patch as per @doligez's suggestion. I decided against moving the three-line replacement function somewhere else; should I?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 2, 2015

Comment author: @gasche

I think you should use Misc.search_substring to implement replace_origin (that would both make the code simpler and make it work for usages that are not at the beginning), and this suggests adding a Misc.replace_substring function to factorize usage -- note that it ought to replace all occurences of the substring, not only the first.

I find it problematic that the Changes entry mentions "$(CAMLORIGIN)" while only "$CAMLORIGIN" works according to the implementation.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 6, 2015

Comment author: @whitequark

Addressed.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 7, 2015

Comment author: @damiendoligez

OK to apply this patch, but it seems slightly broken as is (name of the replace_[sub]string function, argument labels or not).

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 7, 2015

Comment author: @whitequark

Sorry for uploading an unverified patch--it builds now.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 13, 2015

Comment author: @whitequark

Could you also apply this to 4.02.2? I'd like $CAMLORIGIN to be used in the LLVM 3.6 release, which will branch tomorrow.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 25, 2015

Comment author: @gasche

The replace function of the current patch is rather terrible for any usage other than $CAMLORIGIN (in particular it loops when replacing "a" with "a"). I think I'll push the patch in both 4.02 and trunk, but with a more sensible implementation, such as:

let replace_substring ~pat ~sub str =
let rec search acc curr =
try
let next = search_substring pat str curr in
let prefix = String.sub str curr (next - curr) in
search (prefix :: acc) (next + String.length pat)
with Not_found ->
let suffix = String.sub str curr (String.length str - curr) in
List.rev (suffix :: acc)
in String.concat sub (search [] 0)

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 6, 2015

Comment author: @whitequark

Do I need to do something to have this accepted?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 7, 2015

Comment author: @gasche

No: it's on my merge list, but I'm not sure I'll get to it this week-end.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 8, 2015

Comment author: @gasche

Merged in trunk and 4.02. I decided to merge in 4.02 because the risk of breaking seems low (non-invasive patch that mostly adds a new feature), and whitequark considers using it for the newcoming ocaml-llvm release.

@vicuna vicuna closed this Dec 7, 2016

@vicuna vicuna added the feature-wish label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.