-
Notifications
You must be signed in to change notification settings - Fork 59
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a test in modules/task_test.cc that checks the permissions which still have the ifdefs (last one, tasks-cache/sha directory). Other than that, this looks good to me.
CLA signed by all contributors. |
pxp-agent acceptance tests passed on redhat, ubuntu, windows, solaris. I'll see about Windows versions of the permissions tests. |
Applying permissions on Windows and using fs::permissions on Solaris were both hacked around due to a faulty assumption that they were broken on those platforms. The root cause of both issues was a static initialization ordering issue. Fix it by using a macro to alias the variable name, rather than assigning from a global variable in another object. Unwind Windows and Solaris hacks.
Tests updated. |
So the problem was the functions making use of the static variable were called before the variable was initialized? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Not quite. The global variable was copying from other global variable before that one was initialized. |
Ah, I see! |
Applying permissions on Windows and using fs::permissions on Solaris
were both hacked around due to a faulty assumption that they were broken
on those platforms. The root cause of both issues was a static
initialization ordering issue.
Fix it by using a macro to alias the variable name, rather than
assigning from a global variable in another object. Unwind Windows and
Solaris hacks.