Skip to content

Use findlib configs instead of env variables#732

Closed
rauanmayemir wants to merge 7 commits intomasterfrom
rauanmayemir/feature/findlib-config
Closed

Use findlib configs instead of env variables#732
rauanmayemir wants to merge 7 commits intomasterfrom
rauanmayemir/feature/findlib-config

Conversation

@rauanmayemir
Copy link
Copy Markdown
Collaborator

This PR adds an ability to generate findlib configs as part of the build plan and then put those files in place during build phase.

I suspect that at this point this is no longer an optimal solution as we have sandboxes, and it might be better to generate findlib configs during sandbox configuration phase.

@andreypopp Thoughts?

@andreypopp
Copy link
Copy Markdown
Member

Would be interesting to try this approach — for each package we can produce a prefix (a path with bin/, etc/) where we would place etc/findlib.conf and link (hardlink) executables from dependencies into bin/*. That prefix can by cached by build id too.

As a side effect — we'll get env size reduction because we won't need to accumulate all packages's bin into $PATH like we do now.

@rauanmayemir
Copy link
Copy Markdown
Collaborator Author

Let me try that after I make findlib configs work in all envs.

@rauanmayemir rauanmayemir force-pushed the rauanmayemir/feature/findlib-config branch 2 times, most recently from 8581204 to 5a55f88 Compare December 19, 2018 22:51
@rauanmayemir rauanmayemir force-pushed the rauanmayemir/feature/findlib-config branch from 5a55f88 to efba7f8 Compare January 5, 2019 23:55
@rauanmayemir rauanmayemir changed the title [WIP] Use findlib configs instead of env variables Use findlib configs instead of env variables Jan 5, 2019
@rauanmayemir
Copy link
Copy Markdown
Collaborator Author

This is ready for review. I'd like to wrap this up as quickly as possible and move to other related issues.

@rauanmayemir rauanmayemir force-pushed the rauanmayemir/feature/findlib-config branch from efba7f8 to fcbd3fd Compare January 6, 2019 10:29
Copy link
Copy Markdown
Member

@andreypopp andreypopp left a comment

Choose a reason for hiding this comment

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

Looks great! Discussed offline with @rauanmayemir: In the current implementation it is required for package build to be executed before esy CMD starts seeing findlib.conf — otherwise it's working great!

@rauanmayemir rauanmayemir force-pushed the rauanmayemir/feature/findlib-config branch 2 times, most recently from 75b7992 to 1b40a68 Compare January 14, 2019 14:16
This will check the existance of findlib config in command environment when the project hasn't yet been build.

The plan is to add a 'hack' to create `findlib.conf` before running any invocations even though usually we only create it on `esy-b-p` phase
This mixed diff adds a new `prefixPath` notion and a new `p` directory in a package store.

It also refactors build plan to accept streamlined list of files and emit them into the package prefix.
@rauanmayemir rauanmayemir force-pushed the rauanmayemir/feature/findlib-config branch from 1b40a68 to b810c4a Compare January 18, 2019 12:32
@rauanmayemir
Copy link
Copy Markdown
Collaborator Author

After refactoring, we now create a package-level prefix and store build config in it.

We also make sure that the prefix exists even for esy CMD invocations. this helps us with running arbitrary commands that involve findlib config, e.g esy ocamlfind list (when the root is not built).

Current use of prefix is limited to keeping findlib.conf in it, but we have plans to extend it to things like ld.conf and executable links. This will help with pollution of env.

@andreypopp
Copy link
Copy Markdown
Member

Going to close this for now, we are going to try creating a package-specific prefix in the next version of the builder though. This concept seems sound.

@andreypopp andreypopp closed this Jan 12, 2020
@andreypopp andreypopp deleted the rauanmayemir/feature/findlib-config branch January 13, 2020 09:22
@EduardoRFS EduardoRFS mentioned this pull request Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants