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

(SDK-168) Replace check:symlinks with platform independent alternative #193

Merged
merged 1 commit into from
Jun 6, 2017

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented Jun 6, 2017

Shelling out to find to check for symlinks won't work well for our Windows friends, so this PR replaces it with a pure Ruby implementation that works on *nix and Windows.

On Linux:

asmodean :1: puppet/puppetlabs-ntp git:(master)!» bundle exec rake check:symlinks
Symlink found: spec/fixtures/modules/my_ntp => /home/tsharpe/code/puppet/puppetlabs-ntp/spec/fixtures/my_ntp
Symlink found: spec/fixtures/modules/ntp => /home/tsharpe/code/puppet/puppetlabs-ntp
rake aborted!
Symlink(s) exist within this directory

On Windows:

C:\Users\tsharpe\code\puppetlabs-ntp>bundle exec rake check:symlinks
Symlink found: spec/fixtures/modules/my_ntp => C:/Users/tsharpe/code/puppetlabs-ntp/spec/fixtures/my_ntp
Symlink found: spec/fixtures/modules/ntp => C:/Users/tsharpe/code/puppetlabs-ntp
rake aborted!
Symlink(s) exist within this directory

@glennsarti
Copy link
Contributor

Should there be tests for this?

Did you try this with both hard links and junctions?
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365006(v=vs.85).aspx

Also here's the symlink detection code in Puppet https://github.com/puppetlabs/puppet/blob/ce5500dd660019b2be6d132b14f70baff4c553fa/lib/puppet/util/windows/file.rb#L213-L220

@rodjek
Copy link
Contributor Author

rodjek commented Jun 6, 2017

Yep, junctions are detected fine with the standard File.symlink? or Pathname#symlink?. I haven't tested hard links as the original find based implementation didn't check for them (would've needed something like find . -type f -links +1).

If hard links are something we want to test for, we should probably also change the name of the rake task :)

Will look at getting some basic acceptance tests going for puppetlabs_spec_helper tomorrow

@puppetcla
Copy link

CLA signed by all contributors.

@DavidS
Copy link
Contributor

DavidS commented Jun 6, 2017

We specifically check for symlinks, to avoid issues on windows when cloning the repository. tbh, after the recent discoveries around junctions, I believe this to be a git bug, that it uses symlinks, and not junctions, but that's a battle I'm willing to fight.

@DavidS
Copy link
Contributor

DavidS commented Jun 6, 2017

https://github.com/puppetlabs/puppetlabs_spec_helper/pull/193/files#diff-c7beddb7111031927788e851096dc9deR545 excludes all .git directories instead of just the top-level, but I think this is fine.

@DavidS DavidS merged commit 8b70df1 into puppetlabs:master Jun 6, 2017
@rodjek rodjek deleted the sdk-168 branch June 7, 2017 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants