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

(FACT-1276) Make external facts dir absolute instead of canonicalizing #1286

Merged
merged 1 commit into from
Mar 11, 2016

Conversation

branan
Copy link

@branan branan commented Mar 10, 2016

It's not necessary to get the full canonical path for external facts
dir - we only need it to be absolute. Attempting to canonicalize paths
is more expensive, and can fail on Windows when boost::filesystem is
built to support Windows 2003, as we do.

It's not necessary to get the full canonical path for external facts
dir - we only need it to be absolute. Attempting to canonicalize paths
is more expensive, and can fail on Windows when boost::filesystem is
built to support Windows 2003, as we do.
@kylog
Copy link

kylog commented Mar 11, 2016

when boost::filesystem is built to support Windows 2003, as we do

I thought we had dropped the 2003-supporting builds? I may have confused intention and reality (or branches). /cc @MikaelSmith

@branan
Copy link
Author

branan commented Mar 11, 2016

In that case, another fix for this is to build boost::filesystem with -D_WIN32_WINNT=0x0600. It's not so much that we're explicitly building support for NT 5, so much as that we haven't opted in to breaking compatibility

@MikaelSmith
Copy link

Ah, possible but a little more involved. Is there value to canonicalizing the path?

@branan
Copy link
Author

branan commented Mar 11, 2016

Regardless of the windows situation, though, canonicalization can be a very expensive operation (lots of stat calls) vs just creating an absolute path and letting the kernel do it's job to follow symlinks

@branan
Copy link
Author

branan commented Mar 11, 2016

@MikaelSmith I don't think there's any value to canonicalizing in this case

@MikaelSmith
Copy link

I'll add a ticket for using _WIN32_WINNT=0x0600 for Windows builds; that's unrelated to this change then.

MikaelSmith pushed a commit that referenced this pull request Mar 11, 2016
(FACT-1276) Make external facts dir absolute instead of canonicalizing
@MikaelSmith MikaelSmith merged commit ac72d17 into puppetlabs:stable Mar 11, 2016
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.

3 participants