Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Expand ~ to home directory using File::HomeDir instead of File::Glob #6

Closed
wants to merge 1 commit into from

2 participants

@dougwilson

Using File::Glob::bsd_glob to expand the ~ to the home directory seems easy, but as File::Glob itself admits, it doesn't work on very many systems. Notably it does not work on Windows by default (it works if HOME is set in the environment, but HOME it not set by default in Windows).

This changes ~ expansion to use File::HomeDir which is much more portable. It also changes _normalize_id_file to normalize the directories to that of the system as well.

I have mainly made this change for the Windows perl users. Unless one of those users sets the HOME environment variable (not too likely) then the tests won't pass. The tests will also have issues on any system where File::Glob::bsd_glob does not expand ~. Recently I submitted a patch to perl to expand ~ on Windows, but of course that won't help all the old perls which don't do that. Since this module already uses File::HomeDir and File::Spec it seemed logical to use those modules for the ~ expansion.

@dougwilson dougwilson Expand ~ to home directory using File::HomeDir instead of File::Glob
Using File::Glob::bsd_glob to expand the ~ to the home directory
seems easy, but as File::Glob itself admits, it doesn't work on
very many systems. Notably it does not work on Windows by default
(it works if HOME is set in the environment, but HOME it not set
by default in Windows).

This changes ~ expansion to use File::HomeDir which is much more
portable. It also changes _normalize_id_file to normalize the
directories to that of the system as well.
14b7add
@dougwilson

Also as a note, here is what the typical Windows user will see:

t\03_config_file.t ............. 1/62
#   Failed test 'normalize id_file: ~/other.json'
#   at t\03_config_file.t line 76.
#          got: 'C:\Users\Douglas\AppData\Local\Temp\CPAN-Reporter-testhome-evqWYP1A\.cpanreporter\~\other.json'
#     expected: '~/other.json'
# Looks like you failed 1 test of 62.
t\03_config_file.t ............. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/62 subtests
        (less 2 skipped subtests: 59 okay)

As you can see, it was expecting ~/other.json because bsd_glob didn't expand it (since HOME is not set). Worse yet, it normalized the path thinking ~ was a directory since without being able to expand ~ the path is not absolute.

@dagolden
Owner
@dougwilson

Using File::HomeDir on Win32 only seems reasonable to me as well. Also if the ~ expansion is not a feature that you don't mind not always working, I'd suggest at least skipping that test on Win32 or such. The main issue is that bsd_glob on Win32 can only expand ~ from the HOME environment variable which is not present on Windows unless the user manually added it to their environment.

@dagolden
Owner

I've adopted an alternate strategy instead. Please see the master branch and see if it works for you. Thank you!

@dougwilson

This fix works for me, thank you!

@dougwilson dougwilson closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 10, 2011
  1. @dougwilson

    Expand ~ to home directory using File::HomeDir instead of File::Glob

    dougwilson authored
    Using File::Glob::bsd_glob to expand the ~ to the home directory
    seems easy, but as File::Glob itself admits, it doesn't work on
    very many systems. Notably it does not work on Windows by default
    (it works if HOME is set in the environment, but HOME it not set
    by default in Windows).
    
    This changes ~ expansion to use File::HomeDir which is much more
    portable. It also changes _normalize_id_file to normalize the
    directories to that of the system as well.
This page is out of date. Refresh to see the latest.
Showing with 19 additions and 6 deletions.
  1. +17 −3 lib/CPAN/Reporter/Config.pm
  2. +2 −3 t/03_config_file.t
View
20 lib/CPAN/Reporter/Config.pm
@@ -3,7 +3,6 @@ package CPAN::Reporter::Config;
# VERSION
use Config::Tiny 2.08 ();
-use File::Glob ();
use File::HomeDir 0.58 ();
use File::Path qw/mkpath/;
use File::Spec 3.19 ();
@@ -451,10 +450,25 @@ sub _is_valid_grade {
sub _normalize_id_file {
my ($id_file) = @_;
+ my ($volume, $path, $file) = File::Spec->splitpath($id_file);
+ my @dirs = File::Spec->splitdir($path);
- if ( $id_file =~ /~/ ) {
- $id_file = File::Glob::bsd_glob( $id_file );
+ if ( @dirs > 0 && q{~} eq substr $dirs[0], 0, 1 ) {
+ # Expand home directory
+ if ( 1 == length $dirs[0] ) {
+ $dirs[0] = File::HomeDir->my_home;
+ }
+ else {
+ $dirs[0] = File::HomeDir->users_home(substr $dirs[0], 1);
+ }
}
+
+ $id_file = File::Spec->catpath(
+ $volume,
+ File::Spec->catdir(@dirs),
+ $file,
+ );
+
unless ( File::Spec->file_name_is_absolute( $id_file ) ) {
$id_file = File::Spec->catfile(
CPAN::Reporter::Config::_get_config_dir(), $id_file
View
5 t/03_config_file.t
@@ -9,7 +9,6 @@ use Test::More;
use Config::Tiny;
use IO::CaptureOutput qw/capture/;
use File::Basename qw/basename/;
-use File::Glob qw/bsd_glob/;
use File::Spec;
use File::Temp qw/tempdir/;
use File::Path qw/mkpath/;
@@ -67,9 +66,9 @@ is( CPAN::Reporter::Config::_get_config_file(), $config_file,
my @id_file_cases = (
[ $metabase_file => $metabase_file ],
[ 'metabase_id.json' => $metabase_file ],
- [ '/other/path.json' => '/other/path.json' ],
+ [ '/other/path.json' => File::Spec->catfile( '/other/', 'path.json' )],
[ 'other.json' => File::Spec->catfile( $config_dir, 'other.json' )],
- [ '~/other.json' => File::Spec::Unix->catfile( bsd_glob('~'), 'other.json' )],
+ [ '~/other.json' => File::Spec->catfile( File::HomeDir->my_home, 'other.json' )],
);
for my $c ( @id_file_cases ) {
Something went wrong with that request. Please try again.