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

GNU find breaks portability #773

Closed
iluminite opened this issue Mar 14, 2013 · 12 comments · Fixed by #948
Closed

GNU find breaks portability #773

iluminite opened this issue Mar 14, 2013 · 12 comments · Fixed by #948
Assignees
Milestone

Comments

@iluminite
Copy link

hello!

I am excited to have found pelican after feeling bit let down by tinkerer. sadly, while trying to follow the docs and setup a site, I am running into problems with the make file:

% make html              
find /output -mindepth 1 -delete
find: -delete: unknown option
*** Error 1 in /home/foo/bar (Makefile:46 'clean')

The problem, specifically:

clean:
    find $(OUTPUTDIR) -mindepth 1 -delete

GNU find has far too many knobs for cross-platform portability, and will break on *BSD and a variety of other platforms.

A better form of this command that would accomplish the same:

find $(OUTPUTDIR) ... -print0 |xargs -0 rm -rf

I don't have access to too many different types of systems - I have held off on a pull request, but can do so with testing on other platforms confirming this as acceptable.

@wking
Copy link
Contributor

wking commented Mar 14, 2013

On Thu, Mar 14, 2013 at 12:05:39PM -0700, illumin-us-r3v0lution wrote:

clean:
    find $(OUTPUTDIR) -mindepth 1 -delete

Looks like a job for rm -rf "$(OUTPUTDIR)" ;). Pelican should
automatically create OUTPUTDIR if it doesn't already exist, but I
haven't checked if this is actually the case.

@davidmarble
Copy link
Contributor

Chiming in for this change.

clean:
[ ! -d $(OUTPUTDIR) ] || find $(OUTPUTDIR) -mindepth 1 -print0 | xargs -0 rm -rf

should be rather cross-platform.

@wking
Copy link
Contributor

wking commented Jun 15, 2013

In the interest of closing this one, here's a brief history of Pelican's clean implementation:

  • 1edbe04 Include the "pelican-quickstart" script (2011-08-11)

    clean:
      rm -r $$(OUTPUTDIR)/*
    
  • 66ca61d Patch the makefile to automaticaly create the output dir… (2011-08-16, landed in GTM Time in ATOM feeds #156)

    clean:
      rm -fr $$(OUTPUTDIR)
      mkdir $$(OUTPUTDIR)
    
  • 764a2cf Add dual dev/publish modes to quickstart script (2012-07-07, landed in Add dual dev/publish modes to quickstart script #403)

    clean:
      find $$(OUTPUTDIR) -mindepth 1 -delete
    
  • 4e4080c Check output directory exists before doing a clean (2013-02-24)

    clean:
      [ ! -d $$(OUTPUTDIR) ] || find $$(OUTPUTDIR) -mindepth 1 -delete
    

There is no discussion motivating 66ca61d or 764a2cf in their commit messages or PRs. I expect @Natim @justinmayer was just cleaning it up in passing. I think all of these proposals are more complicated than rm -rf $$(OUTPUTDIR). I just tested:

$ python -c 'import pelican as P; P.main()' -o /tmp/a/b/c -s samples/pelican.conf.py samples/content/

which successfully created a nested output directory that had previously not existed, so I don't see any problem with a full removal. Anyone else want to chime in before I submit a PR?

@Natim
Copy link
Contributor

Natim commented Jun 15, 2013

Ok for me.

@ghost ghost assigned justinmayer Jun 23, 2013
@justinmayer
Copy link
Member

@wking: Thanks for the detailed analysis. There are several open issues relating to the topic of when and how the output folder is cleaned, and I've spent the better part of the weekend thinking about an efficient solution. These problems are:

  • find isn't very portable, as mentioned in this issue
  • default make html behavior nukes output folder, including VCS data (e.g., .hg and .git directories)
  • default Makefile ignores the user's DELETE_OUTPUT_DIRECTORY setting

I'm working on a series of changes to address all three problems. Those changes boil down to:

  1. Output folder cleaning behavior should, whenever possible, be handled by pelican.util's clean_output_dir function and the DELETE_OUTPUT_DIRECTORY setting.
  2. Modify clean_output_dir function in order to preserve VCS data directories such as .git and .hg
  3. Remove clean task from html in default Makefile: output directory will no longer be cleaned by default when make html is run in development. That said, make clean can of course be run explicitly at any time.
  4. Set DELETE_OUTPUT_DIRECTORY to True in publishconf.py so that spurious files are removed before publishing to the production server environment.
  5. Revert the make clean task to use rm -rf $$(OUTPUTDIR)

I believe that these changes will provide saner default behavior, reduce the likelihood of data loss, and allow users greater flexibility and control regarding when and how the output folder is cleaned. Comments and suggestions are of course most welcome.

@wking
Copy link
Contributor

wking commented Jun 23, 2013

On Sun, Jun 23, 2013 at 11:24:24AM -0700, Justin Mayer wrote:

  • default make html behavior nukes output folder, including VCS
    data (e.g., .hg and .git directories)

Why would those be in the output directory? If they're tracking extra
source content, that content should go in the source directory, to be
copied over as static files, etc.

@justinmayer
Copy link
Member

There are several reasons one might want to version a site's output directory. One of those reasons is to facilitate deployment via Heroku, as noted in #574.

@justinmayer
Copy link
Member

First PR related to above changes submitted.

justinmayer added a commit to justinmayer/pelican that referenced this issue Jun 24, 2013
The change to the "make clean" task in 764a2cf from "rm -rf" to instead
relying on GNU "find" appears to have broken cross-platform portability,
likely causing problems on *BSD and other platforms. This commit reverts
that change back to the previous "rm -rf" behavior.
@justinmayer
Copy link
Member

The genesis of this issue, as well as my other proposed changes, have been addressed in #948.

@justinmayer
Copy link
Member

Just checking in... Any comments or feedback on this before it's merged?

@wking
Copy link
Contributor

wking commented Jun 26, 2013

On Wed, Jun 26, 2013 at 06:49:16AM -0700, Justin Mayer wrote:

Just checking in... Any comments or feedback on this before it's merged?

This looks good to me.

@justinmayer
Copy link
Member

Fix implemented in bf0a508

bport pushed a commit to bport/pelican that referenced this issue Apr 1, 2014
The change to the "make clean" task in 764a2cf from "rm -rf" to instead
relying on GNU "find" appears to have broken cross-platform portability,
likely causing problems on *BSD and other platforms. This commit reverts
that change back to the previous "rm -rf" behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants