Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pruner should check for excluded files without considering to output_dir #1313

Closed
agross opened this issue Feb 10, 2018 · 14 comments
Closed

Pruner should check for excluded files without considering to output_dir #1313

agross opened this issue Feb 10, 2018 · 14 comments

Comments

@agross
Copy link
Contributor

@agross agross commented Feb 10, 2018

When a pattern excluded from pruning is contained in the output_dir config variable all files below output_dir get excluded.

Steps to reproduce

  1. Set output_dir: output in nanoc.yaml (the default)
  2. Set prune.exclude: ['output'] in nanoc yaml
  3. Compile site
  4. touch output/something
  5. Run nanoc prune --yes

Expected behavior

output/something should be pruned.

Actual behavior

output/something remains.

Details

When checking whether a file is excluded from pruning, the output_dir path component(s) should not be considered.

@agross
Copy link
Contributor Author

@agross agross commented Feb 10, 2018

Background: I have a site with output_dir: build/bin but there is an external tool that creates files in build/bin/bin and I do not want nanoc to prune build/bin/bin/**/*, hence I added bin to prune.exclude.

Loading

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Feb 10, 2018

I believe the behavior you're witnessing is normal. As per Nanoc manual:

The exclude option determines which files and directories you want to exclude from pruning. If you version your output directory, you should probably exclude VCS directories such as .git or .hg.

Now I would acknowledge there's an ambiguity in Nanoc's wording.
Should prune.exclude: ['output'] exclude output or output/output?

The current behavior makes it output/ is excluded from pruning. And by definition, output/something being inside output/ (which is excluded from pruning) doesn't get deleted.

In any case, the exclusion logic is implemented in terms of `String#include? so it's definitely loose and can't achieve what you're asking.

Finally, given your additional example of having output_dir: build/bin and a tool creating files in build/bin/bin it seems to me you want to exclude build/bin/bin and not build/bin right?

Loading

@agross
Copy link
Contributor Author

@agross agross commented Feb 10, 2018

Finally, given your additional example of having output_dir: build/bin and a tool creating files in build/bin/bin it seems to me you want to exclude build/bin/bin and not build/bin right?

Right, but given that the exclude patterns are checked against each path component I cannot achieve that either.

Adding bin as a pattern essentially disables pruning for all files.

Loading

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Feb 10, 2018

I mean prune.exclude: ['build/bin/bin']

Loading

@agross
Copy link
Contributor Author

@agross agross commented Feb 10, 2018

Now I would acknowledge there's an ambiguity in the Nanoc's wording. Should prune.exclude: ['output'] exclude output or output/output?

From my point of view it should only exclude output/output.

Consider this:

  1. output_directory: foo and prune.exclude: ['bar']
  2. You use pruning and like it
  3. Later, you decide you want your output in bar, hence you set output_directory: bar
  4. Pruning gets disabled without anybody telling you

Loading

@agross
Copy link
Contributor Author

@agross agross commented Feb 10, 2018

I mean prune.exclude: ['build/bin/bin']

Not possible, as each path component is checked against a pattern. What you suggested never is a component. For build/bin/bin/foo.dll these components will be checked against exclude patterns: build, bin, bin, foo.dll.

I remember a discussion we had earlier that I would prefer pruning to use globs. It allows for fine-grained control, is rather standard, but I do still believe these globs should be checked against paths below output_dir, whatever that value is at the moment.

Loading

@agross
Copy link
Contributor Author

@agross agross commented Feb 10, 2018

Loading

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Feb 17, 2018

Having globs would certainly still be useful, though it wouldn’t fix this particular bug.

Fix is coming up.

Loading

@agross
Copy link
Contributor Author

@agross agross commented Feb 17, 2018

Thank you!

Loading

@agross
Copy link
Contributor Author

@agross agross commented Feb 27, 2018

I'm sorry but it still doesn't work. I made a little repro: https://github.com/agross/nanoc-pruning

Run run which creates a file untracked by nanoc in output_dir which is not pruned during compilation.

Loading

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Mar 1, 2018

Reopening!

Loading

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Mar 1, 2018

@agross Can you verify that #1324 works for you?

Loading

@agross
Copy link
Contributor Author

@agross agross commented Mar 1, 2018

Looking good!

Loading

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Mar 2, 2018

Nice! That PR needs better test coverage, so I’ll spend a bit more time to finish it up and then release 4.9.2, hopefully this weekend.

Loading

ddfreyne added a commit that referenced this issue Mar 3, 2018
…-pruner

Really ignore output_dir when checking for excludes in pruner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants