#398 √ resolved
glenndavy

[PATCH] expand_path now raises error on invalid path and expands ~ENV['USER']

Reported by glenndavy | March 7th, 2008 @ 11:05 PM | in 1.0 preview

This patch simply removed previously commented lines, which enabled test to pass without breaking ci.

The comment was

#FIXME: Making this compliant requires implimenting getpwnam support

then the line of code required for ffi commented out

then the line of code to check for an invalid path, commented out

i checked that the ffi line was in posix.rb, so I assume getpwnam has since been implimented.

Comments and changes to this ticket

  • glenndavy

    glenndavy March 8th, 2008 @ 12:30 AM

    • → Title changed from “[PATCH] expand_path now generates ArgumentError for invalid path” to “[PATCH] expand_path now raises error on invalid path and expands ~ENV['USER']”
  • glenndavy

    glenndavy March 8th, 2008 @ 12:31 AM

    • Have added expanding ~username to users home dir
  • Charles Comstock

    Charles Comstock March 11th, 2008 @ 03:24 PM

    • → State changed from “new” to “open”

    I wrote the comment. Just switching ~/ to ~$USER isn't correct it's supposed to expand to the absolute path. I'm trying to think if there is anything wrong with doing a gsub to ENV['HOME'] though. That was why I thought getpwnam was necessary, but perhaps environment variables do take care of it.

  • glenndavy

    glenndavy March 11th, 2008 @ 08:32 PM

    hey charles

    ahh.. ok i see your point.. thought it seemed to simple a solution to be true, but hey, i figured it passed the script and behaved functionally. now i see that that whole user/group section of posix.rb with getpwnam doesn't seem to have an implementation - thanks for pointing it out

    glenn

  • Charles Comstock

    Charles Comstock March 13th, 2008 @ 03:10 PM

    • → Assigned user changed from “” to “Charles Comstock”
    • → State changed from “open” to “hold”

    I mean if you want to fake it using $HOME then that's fine, $USER is most definitely not enough though. Do you want to try and fix that?

  • Ryan Davis

    Ryan Davis March 13th, 2008 @ 07:22 PM

    1.8 requires $HOME to be defined on windows systems for expand_path on ~ to work... it may use more but at a minimum, that'll work.

  • glenndavy

    glenndavy March 13th, 2008 @ 07:48 PM

    ok, so $HOME is ok.

    The concern with $USER charles is a windows/platform based concern or something else?

    If so I can run up on windows and see what happens, or try %LOGINNAME% or whatever equiv is there when platform is windows?

    when you say 'do I want to try and fix' am happy to, but short of relying on platforms env, not sure where else to go? would you rather wait until getpwnam is implimented as per you're original approach.

    Glenn

  • Ryan Davis

    Ryan Davis March 17th, 2008 @ 03:03 PM

    OK. I just double checked the code. Basic pseudo-code is:

    if "~" or "~/..." then
      $HOME
    else
      getpwnam
    end
    
  • Ryan Davis

    Ryan Davis May 12th, 2008 @ 02:24 PM

    • → Assigned user changed from “Charles Comstock” to “Ryan Davis”

    stealing

  • Ryan Davis

    Ryan Davis May 12th, 2008 @ 02:30 PM

    • → State changed from “hold” to “resolved”

    applied patch with some changes.

    send evan your ssh public key for commit bits.

    If there are other concerns about this (windows and otherwise) please open a new bug.

Please Login or create a free account to add a new comment.

You can update this ticket by sending an email to from your email client. (help)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket