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

(PCP-769) Fix permissions hacks #641

Merged
merged 1 commit into from
Aug 30, 2017
Merged

Conversation

MikaelSmith
Copy link
Contributor

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.

MikaelSmith added a commit to MikaelSmith/puppet-agent that referenced this pull request Aug 29, 2017
Copy link
Contributor

@ekinanp ekinanp left a 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.

@puppetcla
Copy link

CLA signed by all contributors.

@MikaelSmith
Copy link
Contributor Author

MikaelSmith commented Aug 30, 2017

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.
@MikaelSmith MikaelSmith changed the title (maint) Fix permissions hacks (PCP-769) Fix permissions hacks Aug 30, 2017
@MikaelSmith
Copy link
Contributor Author

Tests updated.

@mruzicka
Copy link
Contributor

mruzicka commented Aug 30, 2017

So the problem was the functions making use of the static variable were called before the variable was initialized?
I'm assuming they were called from an initialization of another static/global object.

Copy link
Contributor

@mruzicka mruzicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mruzicka mruzicka merged commit d35e028 into puppetlabs:master Aug 30, 2017
@MikaelSmith
Copy link
Contributor Author

Not quite. The global variable was copying from other global variable before that one was initialized.

@mruzicka
Copy link
Contributor

Ah, I see!

@MikaelSmith MikaelSmith deleted the fixes branch August 30, 2017 16:01
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.

4 participants