I’ve found that the file removal in VersioncontrolGitRepositoryManagerWorkerDefault::reInit() doesn’t actually remove files. I expect the shell expansion of the …/{one,two} is not happening with how the command is executed.

This ends up leaving promoted sandboxes with the sandbox config and hooks.

CommentFileSizeAuthor
#2 2985905.patch951 bytesdrumm

Comments

drumm created an issue. See original summary.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Active » Needs review
StatusFileSize
new 2985905.patch951 bytes

This patch simplifies this to be a less-clever rm in a loop. It may not be efficient, but it doesn’t need to be. And it is more-readable code.

  • marvil07 committed 5f2ddc6 on 7.x-1.x authored by drumm 
    Issue #2985905 by drumm: Do not rely on shell expansion in reInit()
marvil07’s picture

marvil07 commented
Status: Needs review » Fixed

@drumm, thanks for the report and the patch here.

You are right, the shell expansion is not working.
escapeshellarg() was sourrounding the generated string in quotes, including the parenthesis, and therefore invalidating the shell expansion.

This can also be fixed by adding a bit more logic, but I agree with simplifying the code expanding it a little.
As mentioned in the last comment, it is easier to read after the changes, and the performance gain from possible change should not be considerable, so I added the change here to 7.x-1.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.