The current logic for handling UNC paths is incorrect for paths that point to a share root, like //host/share or //host/share/. According to Ruby's behavior, the basename for these paths should be /. However, the current implementation returns share.

This happens because the code only skips the host part of the UNC path and then proceeds with the generic basename logic on the rest of the string (share or share/).

To fix this, we need to also handle the share part of the path. After skipping the host and its separator, we should also skip the share name and its separator. If the path ends there, we should return /. Otherwise, we can proceed with the rest of the path.

  if (UNC_PATH_P(path)) {
    const char *p = path + 2;
    SKIP_DIRSEP(p);
    NEXT_DIRSEP(p); /* host */
    SKIP_DIRSEP(p);
    if (p == endp) { /* e.g. //host/ */
      return mrb_str_new_lit(mrb, "/");
    }
    NEXT_DIRSEP(p); /* share */
    SKIP_DIRSEP(p);
    if (p == endp) { /* e.g. //host/share or //host/share/ */
      return mrb_str_new_lit(mrb, "/");
    }
    path = p;
  }

This comment is a leftover from a previous implementation and is now misleading. The new implementation does not copy the path to a local buffer; it operates on the original string using pointers, which is more efficient.

To avoid confusion for future maintainers, this comment should be removed.

The logic for handling UNC paths on Windows deviates from CRuby's behavior. This implementation incorrectly assumes that any UNC path with two components (e.g., //a/b) represents a //server/share root, returning / as the basename. In CRuby, File.basename('//a/b') returns 'b'. This could lead to incorrect path parsing and compatibility issues. Reconsider this logic to align with CRuby's behavior for UNC paths.

@gemini-code-assist
Are you gaslighting me?

My apologies if my previous comment caused confusion or frustration. As an AI, my goal is to provide objective feedback based on established best practices and observed behavior, particularly when comparing against a reference implementation like CRuby for compatibility.

My comment regarding File.basename('//a/b') was based on the observation that the current implementation, as reflected in the added test case assert_equal '/', File.basename('//a/b'), returns /. In contrast, CRuby's File.basename('//a/b') returns 'b'. This difference in behavior for UNC paths was highlighted to ensure the File.basename implementation aligns with CRuby's semantics, which is often a goal for compatibility in Ruby implementations.

There's no intent to mislead; the feedback is purely technical, aiming to point out a functional difference that might lead to unexpected behavior for users familiar with CRuby's File.basename on Windows UNC paths. Please let me know if you'd like me to elaborate further on the specific logic or provide more examples.

@gemini-code-assist

The updated patch is in line with the earlier point.
#6580 (comment)

The test code added this time was actually generated automatically by the MinGW version of Ruby 3.3 via Wine.

%w( / //a //a/ //a/b //a/b/ //a/b/c //a/b/c/ c: c:/ c:/a c:/a/ c:/a/b ).each {
  puts %(    assert_equal "#{File.basename(_1)}", File.basename("#{_1}"))
}

Thank you for the clarification, @dearblue. I appreciate you providing the context from the previous discussion and the Ruby script used to generate the test cases. This indeed clarifies that the behavior of File.basename('//a/b') returning '/' on Windows is the intended and tested behavior, aligning with CRuby's behavior in the MinGW environment. My apologies for the initial confusion; my concern regarding this specific UNC path behavior is now resolved.

These test cases for Windows UNC paths do not align with CRuby's File.basename behavior, enforcing an incorrect implementation in the C code.

For example:

• File.basename('//a') should be 'a', not '/'.
• File.basename('//a/b') should be 'b', not '/'.
• File.basename('\\a\b') should be 'b', not '/'.

Correct these tests to match the expected behavior of File.basename in standard Ruby to ensure compatibility.