qetza / replacetokens-task

Azure Pipelines task to replace tokens in files with variables.
MIT License
11 stars 2 forks source link

Paths are no longer normalised (V6) #16

Closed MattJeanes closed 5 months ago

MattJeanes commented 5 months ago

In the newest version 6 of the task, it seems that paths with multiple directory separators no longer resolve correctly that did on previous versions of the task.

Example: D:\a\r1\a/_infrastructure.git/kubernetes/releases/ingress-nginx.yaml

We can work around of course, but this is a bug compared to previous versions of the task :)

qetza commented 5 months ago

Hi @MattJeanes, Is it just a log/output issue or your files are not read? If it's on the logs, could you share the redacted logs to identify which one are not normalized?

MattJeanes commented 5 months ago

Okay I've got a minimum repro yaml pipeline for you:


trigger: none

variables:
  system.debug: 'true'

strategy:
  matrix:
    linux:
      pool: ubuntu-latest
    windows:
      pool: windows-latest

pool:
  vmImage: $(pool)

steps:
- checkout: none

- pwsh: |
    "Testing #{testvar}#" | Set-Content "test.yaml"
    Write-Host "##vso[task.setvariable variable=testvar;]foobar"
  displayName: Set test.yaml

- task: replacetokens@6
  inputs:
    sources: '$(System.DefaultWorkingDirectory)/test.yaml'
    addBOM: true
    escape: 'off'
    logLevel: 'debug'
    missingVarLog: 'error'
    ifNoFilesFound: 'error'

Interestingly it looks like the directory separator might actually be a red herring / separate issue because if you flip it in the YAML above from /test.yaml to \test.yaml it still fails on Windows and then also fails on Linux too.

I can't actually get it to work on Windows at all, regardless of directory separators.

qetza commented 5 months ago

Thanks @MattJeanes, I'll take a look at the issue and try to find a fix in the sources input you must use only forward slashes whatever the platform, this is a constrains from the fast-glob library i'm using in v6 as the backslash is used as an escape character.

But I understand the issue that if you use variables with paths under windows they will have backslashes so maybe for windows only I'll do the replacement internally: replace all backslashes by forward slash so that its valid input for fast-glob.

MattJeanes commented 5 months ago

Works for me! I did notice that if you use the base folder parameter it also works under Windows, but due to the dynamic nature of some paths we put in we can't use that everywhere so that would be fantastic.

Thank you 🙂

qetza commented 5 months ago

Hi @MattJeanes, Version 6.0.3 is released and fixes your issue 😄

MattJeanes commented 5 months ago

Amazing, just tried it now and seems to work!

It does seem to break if I explicitly enter a \ on Linux but that's fine we can and do normalise to Linux normally anyway so we'll just use forward slashes everywhere which is what we were doing anyway 🙂

Happy to close this unless you wanted to fix that little thing too, not sure what the behaviour of that was on older versions

qetza commented 5 months ago

On the previous version I was using a glob pattern library which didn’t support the backslash as an escape character I was normalizing the paths on all platforms.

With version 6 now the glob pattern syntax has more features needing the backslash as an escape character.

So I will say that in Linux if someone puts a backslash it’s to use as an escape character; I’m okay with this breaking change as normally you shouldn’t have backslash in paths on Linux. On windows, as you showed with your example, I cannot force users to put forward slashes so doing the normalization was mandatory.

MattJeanes commented 5 months ago

Makes sense, thanks for the explanation and thanks again for the quick fix! 🙂