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

Semicolon in single quotes causes dryruntarget to fail #671

Closed
wkdzzb opened this issue Nov 21, 2024 · 10 comments · Fixed by #678
Closed

Semicolon in single quotes causes dryruntarget to fail #671

wkdzzb opened this issue Nov 21, 2024 · 10 comments · Fixed by #678
Assignees
Labels
bug Something isn't working Feature: Parsers The area of parsing the dry-run or build log
Milestone

Comments

@wkdzzb
Copy link

wkdzzb commented Nov 21, 2024

makefile-tools version:0.11.13

dryrun.log

...
set -e;  echo '  RELOCS  arch/x86/boot/compressed/vmlinux.relocs';  arch/x86/tools/relocs vmlinux > arch/x86/boot/compressed/vmlinux.relocs;arch/x86/tools/relocs --abs-relocs vmlinux; printf '%s\n' 'cmd_arch/x86/boot/compressed/vmlinux.relocs := arch/x86/tools/relocs vmlinux > arch/x86/boot/compressed/vmlinux.relocs;arch/x86/tools/relocs --abs-relocs vmlinux' > arch/x86/boot/compressed/.vmlinux.relocs.cmd
...

Makefile tools output

Error running the compiler args parser script /root/.vscode-server/extensions/ms-vscode.makefile-tools-0.11.13/assets/parseCompilerArgs.sh for regions (--abs-relocs vmlinux' > arch/x86/boot/compressed/.vmlinux.relocs.cmd): /bin/bash: -c: line 1: unexpected EOF while looking for matching `''
/bin/bash: -c: line 2: syntax error: unexpected end of file

I found the real reason is the function preprocessDryRunOutput.

async function preprocessDryRunOutput(cancel, dryRunOutputStr, statusCallback) {
...
    preprocessTasks.push(function () {
        preprocessedDryRunOutputStr = preprocessedDryRunOutputStr.replace(/ && /g, "\n");
    });
    // Split multiple commands concatenated by ";"
    preprocessTasks.push(function () {
        preprocessedDryRunOutputStr = preprocessedDryRunOutputStr.replace(/;/g, "\n");
    });
...
}

the function preprocessDryRunOutput replaces ';' with '\n',but in some shell cmd, like this

echo 'echo 123;echo 456' > ./tmp

replace ';' with '\n'

echo 'echo 123
echo 456' > ./tmp

it will cause an error

@Amy-Li03
Copy link
Collaborator

Hi @wkdzzb, thanks for reporting issue.
We try to repro your issue on our end with latest Makefile Tools v0.12.10 (pre-release), following screenshot is the test result, could you please confirm whether this is your issue?
Thanks for help us build a better Makefile Tools!
Image

@Amy-Li03 Amy-Li03 added more info needed More info is needed from the community for us to properly triage and investigate and removed triage labels Nov 22, 2024
@wkdzzb
Copy link
Author

wkdzzb commented Dec 2, 2024

I wrote a makefile and appended these, and it repeats

$(target):$(obj)
	$(GCC) $(C_FLAGS) $^ $(head) -o $@
	make: Entering directory '/root/code/kernel-5.10.0-60.18.0.50.oe2203'
	gcc   -o arch/x86/tools/relocs arch/x86/tools/relocs_32.o arch/x86/tools/relocs_64.o arch/x86/tools/relocs_common.o;
	echo 'echo 123; arch/x86/tools/relocs --abs-relocs vmlinux' > ./tmp

@gcampbell-msft
Copy link
Collaborator

@wkdzzb @Amy-Li03 Could you confirm whether this is a new / regression issue in the most recent versions? Based on @wkdzzb's information and code link, it's not, but I want to confirm. Thanks!

@gcampbell-msft
Copy link
Collaborator

gcampbell-msft commented Dec 2, 2024

@wkdzzb Also, is this currently blocking you? Or is this something that you're able to workaround by slightly modifying your makefile?

Additionally, does this still repro if you use double quotes?

@gcampbell-msft
Copy link
Collaborator

Update, my initial thoughts are that our first iteration of a possible fix could simply be to not do the replacement if it's within quotes like that, but this will require more investigation and input from @wkdzzb

@wkdzzb
Copy link
Author

wkdzzb commented Dec 3, 2024

@wkdzzb @Amy-Li03 Could you confirm whether this is a new / regression issue in the most recent versions? Based on @wkdzzb's information and code link, it's not, but I want to confirm. Thanks!

I can confirm that this is not a new or regression issue in the latest versions. I tested all versions of makefile tools from 0.8 to 0.12, and this issue exists in all of them. Additionally, I tried using double quotes, but the issue still occurs.

@wkdzzb
Copy link
Author

wkdzzb commented Dec 3, 2024

This is not blocking me; however, it does cause issues with VSCode's hints and code completion, resulting in many errors being reported. The makefile is also difficult to modify since it is the kernel's makefile.

@wkdzzb
Copy link
Author

wkdzzb commented Dec 3, 2024

I think replacing a semicolon with a newline inside single or double quotes is problematic, as it can break the original shell commands. However, replacing a semicolon with a newline outside of quotes should not be an issue.

@gcampbell-msft
Copy link
Collaborator

@wkdzzb Understood, thanks for the additional testing and context. I tend to agree that replacing a semicolon within quotes can cause issues, the best fix here may be to simply overlook those cases and not replace the semicolon with a newline.

@gcampbell-msft gcampbell-msft added this to the On Deck milestone Dec 4, 2024
@gcampbell-msft gcampbell-msft added bug Something isn't working Feature: Parsers The area of parsing the dry-run or build log and removed more info needed More info is needed from the community for us to properly triage and investigate labels Dec 4, 2024
@gcampbell-msft gcampbell-msft modified the milestones: On Deck, 0.12.0 Dec 4, 2024
@gcampbell-msft gcampbell-msft self-assigned this Dec 9, 2024
@gcampbell-msft
Copy link
Collaborator

@wkdzzb Could you please test out the vsix that is linked here: #678?

You'll need to download the .zip and change the extension to .vsix and then you can install it manually into vscode from the extensions pane. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Feature: Parsers The area of parsing the dry-run or build log
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants