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

Compiler config deps #1585

Merged
merged 5 commits into from Jan 17, 2015
Merged

Compiler config deps #1585

merged 5 commits into from Jan 17, 2015

Conversation

@ralsina
Copy link
Member

@ralsina ralsina commented Jan 17, 2015

This should fix Issue #1523

@Kwpolska Kwpolska added this to the v7.3.1 milestone Jan 17, 2015
@Kwpolska Kwpolska self-assigned this Jan 17, 2015
Kwpolska added a commit that referenced this issue Jan 17, 2015
@Kwpolska Kwpolska merged commit b7c6572 into master Jan 17, 2015
2 checks passed
@ralsina ralsina deleted the compiler-config-deps branch Jan 17, 2015
@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jan 17, 2015

I think post.add_dependency_uptodate is not used correctly here. It is more or less directly added (for lists, its elements are added) to doit's uptodate field, which expects lists of False, True, None, callables, or strings (which are executed as shell commands) (see http://pydoit.org/dependencies.html#uptodate). I think tools.config_changed should be used here to (somehow) wrap the data added.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 17, 2015

Ugh. Ok.

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jan 17, 2015

I think doit will try to execute each string as a shell command, and use its return value to determine whether the task changed or not. That's not what you want, I think?

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 17, 2015

Indeed.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 17, 2015

Just out of curiosity, is add_dependency_uptodate used anywhere so I can see how it's done there?

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jan 17, 2015

If you want to update if the list of strings changed (maybe you want to sort the list to avoid list reordering triggering rebuilds), use tools.config_changed({1: your_string_list}, 'some unique identifier for the plugin') or so.

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jan 17, 2015

I found an example in one of my (not yet published) plugins:

post.add_dependency_uptodate(utils.config_changed({1: some_data}, 'nikola.plugins.path.to.comment.and.its.name:' + post.base_path), False, 'page')

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 17, 2015

ok, pushing now.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 17, 2015

The ID you say "unique"... that means unique in the context of that specific task, right? Not unique across all the tasks?

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jan 17, 2015

It should be unique over all posts and tasks, because it could happen that different tasks add something to the uptodate dependency of a post, and many posts are on one index/list/... page, and all those uptodate dependencies are put into the uptodate dependency on the generated index/list/...

I'd use a combination of the task/plugin/... name and the post's source path.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 18, 2015

Ugh again then.

This method is completely unusable based on the documentation there. It's a strange feeling to have no idea how this works. I think we should remove the marketing stuff about how nikola's codebase is easy to understand.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 18, 2015

Last question: it has to be unique across the whole build and stable from one build to another?

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Jan 18, 2015

Yes. (Of course, it should be stable iff the options it contains don't change.)
So if you want to force a rebuilt if a list of strings changes (but its order doesn't depend), sort the list, put it into a config_changed object and make its tag depend on the plugin and the post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants