Respect (AMENT|COLCON|CATKIN)_IGNORE directories

[rotu]

This reflects the behavior of colcon, which tests whether the path exists, not whether it is a regular file.

[dirk-thomas]

Looking at different REPs mentioning CATKIN_IGNORE (128, 133, 141) all of them mentioned it to be an empty marker file. So I am not sure if a directory with that name should be considered equal. It could just be that colcons implementation unknowingly diverges from this.

Can you explain why you think these ignore marker should also possibly subdirectories and can't just create a file instead?

[rotu]

My particular use case is admittedly a bit of a hack - because vcstool doesn't like clobbering repos on Windows, I instead install a marker directory inside the repo to ignore: https://github.com/ros2/rmw_cyclonedds/blob/3dc89b885cbdfba8372184cb0e6279b28e6bad7e/.github/resources/suppress_other_rmw.repos

1. I think it's less surprising for both to work. If a COLCON_IGNORE directory exists but is a directory, it's clear the user's intent is to ignore the parent directory. Also, if COLCON_IGNORE is a directory, then touching that directory file will succeed silently.

2. It provides some added utility for git users, since git ignores empty directories. So you can create COLCON_IGNORE as a file if it should be checked in and as a directory if it shouldn't.

[rotu]

Also, I would argue that a directory is a type of file. There was just no use for the REPs to get pedantic about it.

[dirk-thomas]

REP 128 uses the description "empty marker file" which imo clearly identifies a file and not a directory. So I think before we can consider this patch the REP(s) need to be updated to clarify that both types should be considered.

[rotu]

1. By that reading a marker file which is not empty should not be respected. That's insane.
2. A directory is a file.

There is no need to update the REP, but if you feel there is for some reason, go ahead. Nevermind, did it myself ros-infrastructure/rep#256

[gavanderhoorn]

@rotu: is it your intention to antagonise people?

I'm Dutch and quite used to directness (sometimes even probably called rudeness), but there doesn't seem to be a need to be abrasive in your responses to people.

Why is it insane to understand the sentence is an empty marker file to mean just that: an empty file? A directory is commonly understood to not be a file, that's why it's called a directory, and why there are different ways to manage them and there are different usages for them. It may be that file systems in their implementation consider them similarly, but the very fact that they have different names seem to imply they are not the same for all intents and purposes.

If the REP states that the file should be empty, then yes, that would seem to mean that non-empty files are not considered valid CATKIN_IGNORE files. I'm pretty sure non-empty files are not considered invalid, but I wouldn't call it insane even if they were.

[rotu]

@rotu: is it your intention to antagonise people?

I'm sorry, I got riled up. @dirk-thomas, I felt like you were being willfully stubborn by insisting I open yet another PR for this change. I should have given you the benefit of the doubt, and I apologize that I didn't.

Why is it insane to understand the sentence is an empty marker file to mean just that: an empty file?

As the file is described as "an empty marker file" I interpreted it to mean "the presence of an AMENT_IGNORE file", not its content or properties, is what matters. I do think it's unreasonable to disregard an AMENT_IGNORE file if it's not empty. For instance, if it has a single newline, which text editors may insert.

the very fact that they have different names seem to imply they are not the same for all intents and purposes.

A directory is a file. As per man ls:

NAME
       ls - list directory contents

SYNOPSIS
       ls [OPTION]... [FILE]...

DESCRIPTION
       List information about the FILEs (the current directory by default).

I made that explicit in ros-infrastructure/rep#256.

[dirk-thomas]

A directory is a file. As per man ls:

I assume you are familiar that the terms file and directory are clearly distinguished in a lot of other cases, e.g. in the Python API is_dir / is_file.

The fact that you read this differently than many others is just an indicator that it needs to be explicit / clarified in the REP - as done in ros-infrastructure/rep#256.

[rotu]

I assume you are familiar that the terms file and directory are clearly distinguished in a lot of other cases, e.g. in the Python API is_dir / is_file.

Even the linked documentation concedes that a directory is a file. I suppose "is_regular_file" was just too wordy:

Return True if the path points to a regular file (or a symbolic link pointing to a regular file), False if it points to another kind of file.

[dirk-thomas]

I simplified the logic in the patch: e335bbe, adee347.

With the REP updated I will go ahead and merge (and release) this.