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

Set up environment before post-fetch actions #456

Merged
merged 10 commits into from
Jul 6, 2020
Merged

Conversation

mosteo
Copy link
Member

@mosteo mosteo commented Jun 23, 2020

Fixes on top of #452 to ensure the environment is ready during post-fetch actions. Also, generate proper paths for softlinked dependencies.

This PR refactors Alr.Build_Env into Alire.Environment so environment generation is not split into two locations.

@mosteo mosteo marked this pull request as ready for review June 23, 2020 18:12
begin
Requires_Full_Index;

Requires_Valid_Session;

Alr.Build_Env.Export (Alr.Root.Current);
if Export_Build_Env then
Copy link
Member

Choose a reason for hiding this comment

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

If would be nice to export the environment just when we need it, when spawning a command.

In python it is doable with popen, but I don't know any similar API for Ada: https://docs.python.org/3/library/subprocess.html#subprocess.Popen

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see. That's too bad we don't have something portable like that in GNAT or GNATCOLL...

@Fabien-Chouteau
Copy link
Member

Looks like a great cleanup, thanks @mosteo :)

src/alire/alire-environment.ads Outdated Show resolved Hide resolved
src/alire/alire-environment.ads Outdated Show resolved Hide resolved
@mosteo mosteo force-pushed the feat/env_vars branch 3 times, most recently from 5066328 to bc31228 Compare June 25, 2020 22:49
@mosteo mosteo force-pushed the feat/env-actions branch 2 times, most recently from 370d5cf to 8490f85 Compare July 2, 2020 14:13
@mosteo mosteo changed the base branch from feat/env_vars to master July 2, 2020 14:14
src/alire/alire-environment.ads Outdated Show resolved Hide resolved
src/alire/alire-environment.ads Outdated Show resolved Hide resolved
src/alire/alire-environment.ads Outdated Show resolved Hide resolved
@mosteo mosteo force-pushed the feat/env-actions branch 3 times, most recently from 7df98fe to 36b54ec Compare July 2, 2020 17:47
@mosteo mosteo changed the base branch from master to feat/env_vars July 2, 2020 17:48
@mosteo
Copy link
Member Author

mosteo commented Jul 3, 2020

@Fabien-Chouteau, could you please take a look at the two last commits I pushed? These introduce changes since you reviewed.

I'm going to check if something like popen is doable portably next, but my idea is to merge this PR and #452 simultaneously ASAP and if so make another PR later for that.

@mosteo
Copy link
Member Author

mosteo commented Jul 3, 2020

It seems an option could be to rely on Boost.Process. According to their docs, this part of Boost is headers-only, so conceivably we could use it directly without changing anything in our build process. I haven't done any testing though.

@Fabien-Chouteau
Copy link
Member

I'm going to check if something like popen is doable portably next

Don't worry about that, I was thinking out loud. What we have now is good I think.

Base automatically changed from feat/env_vars to master July 6, 2020 08:38
The path lookup has three levels:
1. For the root release, the own root folder
2. For a regular release, the dependencies folder "/" release unique folder
3. For a pinned softlink, the absolute path of the softlink

The root contains the solution which in turn contains the softlinks, so the
query for the path can be directly made to the root.
This flies by CE but is detected by FSF (?)
There's some bug manifesting when a lot of code is in the spec in which the
linker complaints about missing references.
This is to match the output of full path normalization by Ada.Directories.
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.

None yet

2 participants