Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.4
    • Fix Version/s: 1.3.4
    • Component/s: None
    • Labels:
      None

      Description

      It does this even if it immediately returns. this does call (in linux) the mkdir system call (which returns EEXIST)

      It should also turn off verbose for it

        Activity

        Hide
        Assaf Arkin added a comment -

        What's the problem then?

        Show
        Assaf Arkin added a comment - What's the problem then?
        Hide
        Ittay Dror added a comment -

        it's an unnecessary IO, and it outputs 'mkpath -p ...' to the screen which is just noise

        Show
        Ittay Dror added a comment - it's an unnecessary IO, and it outputs 'mkpath -p ...' to the screen which is just noise
        Hide
        Assaf Arkin added a comment -

        We should take care of verbose.

        Otherwise, what's the actual performance impact of calling mkpath? I don't want to introduce code to optimize something that's insignificant.

        Show
        Assaf Arkin added a comment - We should take care of verbose. Otherwise, what's the actual performance impact of calling mkpath? I don't want to introduce code to optimize something that's insignificant.
        Hide
        Ittay Dror added a comment -

        i don't know about the performance impact, but it is probably no more than checking if the directory exists

        but the code immediately after is:
        return false if copy_map.empty?

        so why not move mkpath after this check? (must the path exist even if empty?)

        Show
        Ittay Dror added a comment - i don't know about the performance impact, but it is probably no more than checking if the directory exists but the code immediately after is: return false if copy_map.empty? so why not move mkpath after this check? (must the path exist even if empty?)
        Hide
        Assaf Arkin added a comment -

        A filter always creates the target directory (if one is specified), see spec/core/common_spec.rb line 443. It's easier to call mkpath than duplicate that functionality everywhere.

        Show
        Assaf Arkin added a comment - A filter always creates the target directory (if one is specified), see spec/core/common_spec.rb line 443. It's easier to call mkpath than duplicate that functionality everywhere.
        Hide
        Ittay Dror added a comment -

        creating an empty target directory is an unnecessary performance impact, is it a must? if some other code relies on filter.target, can't the spec define that it must do File.exist(filter.target) prior to using it?

        Show
        Ittay Dror added a comment - creating an empty target directory is an unnecessary performance impact, is it a must? if some other code relies on filter.target, can't the spec define that it must do File.exist(filter.target) prior to using it?
        Hide
        Assaf Arkin added a comment -

        $ git blame -l lib/buildr/core/filter.rb -L 177,177
        0379d903272f5cc3fbd336a106e910f909e6bdd5 (Assaf Arkin 2008-05-19 18:32:40 +0000 177) mkpath target.to_s
        $ git log 0379d903272f5cc3fbd336a106e910f909e6bdd5
        commit 0379d903272f5cc3fbd336a106e910f909e6bdd5
        Author: Assaf Arkin <assaf@apache.org>
        Date: Mon May 19 18:32:40 2008 +0000

        Fixed: BUILDR-75 Filter now runs if there's a target directory, even if
        there are no source files to copy over, since everyone else just checks
        resources.target for existence before depending on it.

        Issue number is wrong, but you can search JIRA and the mailing list for further discussion.

        Once again, if you can't quantify the performance impact, there's no point in continuing this thread.

        Show
        Assaf Arkin added a comment - $ git blame -l lib/buildr/core/filter.rb -L 177,177 0379d903272f5cc3fbd336a106e910f909e6bdd5 (Assaf Arkin 2008-05-19 18:32:40 +0000 177) mkpath target.to_s $ git log 0379d903272f5cc3fbd336a106e910f909e6bdd5 commit 0379d903272f5cc3fbd336a106e910f909e6bdd5 Author: Assaf Arkin <assaf@apache.org> Date: Mon May 19 18:32:40 2008 +0000 Fixed: BUILDR-75 Filter now runs if there's a target directory, even if there are no source files to copy over, since everyone else just checks resources.target for existence before depending on it. Issue number is wrong, but you can search JIRA and the mailing list for further discussion. Once again, if you can't quantify the performance impact, there's no point in continuing this thread.
        Hide
        Ittay Dror added a comment -

        on my system, re-creating 100 paths takes 5 milliseconds, not worth doing anything about.

        so just the verbose thing then

        Show
        Ittay Dror added a comment - on my system, re-creating 100 paths takes 5 milliseconds, not worth doing anything about. so just the verbose thing then
        Hide
        Assaf Arkin added a comment -

        :verbose is now on only when running with --trace.

        Show
        Assaf Arkin added a comment - :verbose is now on only when running with --trace.

          People

          • Assignee:
            Unassigned
            Reporter:
            Ittay Dror
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development