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

Recognize install snippets that follow environment variable declarations #770

Closed
rnjudge opened this issue Jul 17, 2020 · 4 comments · Fixed by #809
Closed

Recognize install snippets that follow environment variable declarations #770

rnjudge opened this issue Jul 17, 2020 · 4 comments · Fixed by #809

Comments

@rnjudge
Copy link
Contributor

rnjudge commented Jul 17, 2020

Describe the Feature
It is common in Dockerfiles that build go apps to set some environment variables before running the actual snippet that might install go modules. For example:

RUN CGO_ENABLED=0 GOOS=linux go build -v -a -installsuffix cgo -o swag cmd/swag/main.go

For the above line, Tern currently does not recognize "go build" as a snippet to install modules, despite that snippet being listed in snippets.py. This issue proposes changes to the way Tern parses

Use Cases
See golang example above.

Implementation Changes
Changes to filter_install_commands() or other associated functions, like split_command(), to better filter through potential environment variables that might come before a snippet install command.

@ForgetMe17
Copy link
Contributor

This is kind of bug for now, because our parser will consider
RUN CGO_ENABLED=0 GOOS=linux go build -v -a -installsuffix cgo -o swag cmd/swag/main.go
as a variable assignment. Here are the result.

{'content': 'CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o app .',
 'variable': {'name': 'CGO_ENABLED',
              'value': '0 GOOS=linux go build -a -installsuffix cgo -o app .'}}

@ForgetMe17
Copy link
Contributor

I can take on this issue! @rnjudge
We can modify parse_shell_variables_and_command() in tern\utils\general.py. We can loop on a search for the variable_pattern until we can not find the macthed case.
For command CGO_ENABLED=0 GOOS=linux go build -v -a -installsuffix cgo -o swag cmd/swag/main.go:

  1. Extract CGO_ENABLED=0 by ^([A-Za-z_][A-Za-z0-9_]*)=(.*) and the remaining part should be GOOS=linux go build -v -a -installsuffix cgo -o swag cmd/swag/main.go.
  2. Extract GOOS=linux by ^([A-Za-z_][A-Za-z0-9_]*)=(.*) and the remaining part should be go build -v -a -installsuffix cgo -o swag cmd/swag/main.go.
  3. The remaining part does not contains a variable_pattern so it should be command.

Note that this method only works on the condition that the variable assignment are followed by command which should be a common case. The reason is that we are looking for a variable assignment from the start of command.

@nishakm
Copy link
Contributor

nishakm commented Sep 3, 2020

@ForgetMe17 do you still want to work on this?

@ForgetMe17
Copy link
Contributor

@nishakm I will try on it!

ForgetMe17 added a commit to ForgetMe17/tern that referenced this issue Sep 14, 2020
This commit modifies parse_shell_variables_and_command() in
tern\utils\general.py to enable shell script parser to capture variable
assignments before a command.

Fixes tern-tools#770.

Signed-off-by: WangJL <hazard15020@gmail.com>
ForgetMe17 added a commit to ForgetMe17/tern that referenced this issue Sep 22, 2020
This commit modifies parse_shell_variables_and_command() in
tern\utils\general.py to enable shell script parser to capture variable
assignments before a command.

Fixes tern-tools#770.

Signed-off-by: WangJL <hazard15020@gmail.com>
nishakm pushed a commit that referenced this issue Nov 9, 2020
This commit modifies parse_shell_variables_and_command() in
tern\utils\general.py to enable shell script parser to capture variable
assignments before a command.

Fixes #770.

Signed-off-by: WangJL <hazard15020@gmail.com>
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 a pull request may close this issue.

3 participants