Skip to content

Conversation

doblon8
Copy link
Contributor

@doblon8 doblon8 commented Oct 16, 2025

When I tried building Sparrow in a Spanish Windows 10 system, the build failed in the task that sets write permissions for the user:

  > Task :addUserWritePermission FAILED
  Se procesaron correctamente 0 archivos; error al procesar 1 archivos
  Process 'command 'icacls'' finished with exit value 1332 (state: FAILED)

The failure occurred because the task used the group name Users, but in non-English systems it could be different (for example., Usuarios in Spanish), so Windows cannot find the group and the icacls command returns error 1332.

This change replaces the group name with the SID *S-1-5-32-545, a constant that always identifies the local Users group regardless of Windows version or language.

@craigraw
Copy link
Collaborator

Looks like a good change, but I think we can just replace Users with *S-1-5-32-545 directly so we don't declare the variable for other operating systems.

@doblon8
Copy link
Contributor Author

doblon8 commented Oct 17, 2025

I didn’t think about that issue, and just used the usersGroup varibale instead of inlining the *S-1-5-32-545 value for readability.

Before making the change you suggested, what do you think about defining the usersGroup variable inside the if (os.windows) block instead?

tasks.register('addUserWritePermission', Exec) {
    if (os.windows) {
        def usersGroup = '*S-1-5-32-545' // Windows "Users" group SID (language-independent)
        commandLine 'icacls', "$buildDir\\image\\legal", '/grant', "${usersGroup}:(OI)(CI)F", '/T'
    } else {
        commandLine 'chmod', '-R', 'u+w', "$buildDir/image/legal"
    }
}

That way the variable is only defined for Windows, while still keeping the readability benefit.

But if you still prefer the inline form, I’m happy to update the PR with that change.

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.

2 participants