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

env: add a flag to output as environment variable bindings #6316

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Nov 28, 2024

fix #5791

The name is still to be determined (it's a sed-able name ftm)

@rjbou rjbou added this to the 2.4.0~alpha1 milestone Nov 28, 2024
Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

I'm ok with the idea but i'm really not sure about the name of the new option.

--bindings(s) doesn't really describe anything to me personally. I haven't thought about it much but so far i prefer the original suggestion: --bare.

One other issue is that the expected usage is to add to the $GITHUB_ENV file in Github Actions. However the syntax for it is slightly more complex than we can first assume. In the rare case where a newline character is part of the value of an environment variable, the "binding" syntax is not correct anymore.
I think we should check that no newline character exists before printing and fail otherwise.

src/client/opamConfigCommand.ml Outdated Show resolved Hide resolved
@rjbou
Copy link
Collaborator Author

rjbou commented Dec 2, 2024

From dev meeting: Consensus over the name --raw. We won't handle newline in this PR, but prefer output a constructed json for CI handling.

@rjbou
Copy link
Collaborator Author

rjbou commented Dec 3, 2024

Update with --raw

@rjbou
Copy link
Collaborator Author

rjbou commented Dec 16, 2024

From (last week) dev meeting: we are going to a full json output for CI, containing the information of the full computed path, and also what to append and/or prepend. Like that CI scripts can use that information to change their environment variables as needed.
For ex:

{ 
  "var" : "PATH"
  "full": "foo:bar:/usr/bin:/etc:baz";
  "prepend": ["foo"; "bar"];
  "append": ["baz"]
} 

@rjbou rjbou mentioned this pull request Dec 16, 2024
@kit-ty-kate kit-ty-kate added PR: WIP Not for merge at this stage and removed PR: WAITING FOR REVIEW labels Dec 26, 2024
@kit-ty-kate kit-ty-kate removed their request for review December 26, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: WIP Not for merge at this stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support new flag: opam env --bare for use with GitHub actions
2 participants