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

feat: allow bypassing chalk via CHALK_BYPASS environment variable #228

Closed
wants to merge 1 commit into from

Conversation

ee7
Copy link
Contributor

@ee7 ee7 commented Mar 1, 2024

Description

@nettrino and I had a good discussion earlier today about bypassing chalk (issue #224). Here's a sketch of a possible implementation.

This is the "avoid as much chalk as possible" flavor, optimizing for a completely independent, minimal code path.

It allows bypassing the exec command:

$ CHALK_BYPASS=1 chalk exec echo hi
chalk: the CHALK_BYPASS environment variable is set.
Running the following command without chalk:
  echo hi
hi

and bypassing the docker command:

$ CHALK_BYPASS=1 chalk docker --version
chalk: the CHALK_BYPASS environment variable is set.
Running the following command without chalk:
  docker --version
Docker version 25.0.3, build 4debf411d1

and errors when the command is not docker or exec:

$ CHALK_BYPASS=1 chalk version
error: CHALK_BYPASS was set, but the chalk command is not 'docker' or 'exec'. Quitting.

Refs: #224

Testing

Run commands like illustrated above.

To-do

If we wanted to continue in this direction:

  • Tests
  • Docs
  • Consider handling chalk being named docker

A sketch of a possible implementation for bypassing chalk.

It allows bypassing the exec command:

    $ CHALK_BYPASS=1 ./chalk exec echo hi
    chalk: the CHALK_BYPASS environment variable is set.
    Running the following command without chalk:
      echo hi
    hi

and bypassing the docker command:

    $ CHALK_BYPASS=1 ./chalk docker --version
    chalk: the CHALK_BYPASS environment variable is set.
    Running the following command without chalk:
      docker --version
    Docker version 25.0.3, build 4debf411d1

and errors when the command is not docker or exec:

    $ CHALK_BYPASS=1 ./chalk version
    error: CHALK_BYPASS was set, but the chalk command is not 'docker' or 'exec'. Quitting.
@viega
Copy link
Collaborator

viega commented Mar 1, 2024

Per my last comment on the linked issue, I really don't think we should do this before the con4m refactor without a very good reason.

@miki725
Copy link
Collaborator

miki725 commented Mar 5, 2024

+1. this approach I think breaks a bunch of semantics about chalk comment. if we need to bypass chalk, I think we need to do that in such a way which bypasses error-prone areas of chalk such as anything to do with user configs/reporting/etc without breaking user semantics

for example chalk exec needs to grab some potential exec args from either command line args or embedded config. saying that CHALK_BYPASS now breaks some chalk commands depending on context I dont think is acceptable.

a lot of con4m evaluation will be nop in v2 so over there we can correctly skip just the bits in chalk which can possible blow up such as chalk collection/reporting/etc.

@miki725
Copy link
Collaborator

miki725 commented Jun 4, 2024

closing as we wont be taking this approach in the long term and this just pollutes PRs list

@miki725 miki725 closed this Jun 4, 2024
@ee7 ee7 deleted the ee7/bypass branch June 4, 2024 16:19
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.

3 participants