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

github_deploy shouldn’t fail if it can’t determine the source commit #2847

Closed
asmeurer opened this Issue Jun 17, 2017 · 17 comments

Comments

Projects
None yet
2 participants
@asmeurer
Contributor

asmeurer commented Jun 17, 2017

github_deploy tries to deploy from the branch specified by GITHUB_SOURCE_BRANCH, which defaults to master.

This doesn't make sense to me. The default behavior that I would expect would be to just deploy the current contents of the repo.

@Kwpolska Kwpolska added the question label Jun 18, 2017

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 18, 2017

Member

github_deploy requires a special branching model. There are two branches, one for page source, and another for the page itself. By default, Nikola automatically commits to the source branch for you, so you can keep your site data in sync between different machines. If you don’t want this, just set GITHUB_COMMIT_SOURCE to False — but then you need to remember to do your own committing and take care of branches yourself.

Member

Kwpolska commented Jun 18, 2017

github_deploy requires a special branching model. There are two branches, one for page source, and another for the page itself. By default, Nikola automatically commits to the source branch for you, so you can keep your site data in sync between different machines. If you don’t want this, just set GITHUB_COMMIT_SOURCE to False — but then you need to remember to do your own committing and take care of branches yourself.

@Kwpolska Kwpolska closed this Jun 18, 2017

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 18, 2017

Contributor

This should at least be documented. nikola github_deploy --help says nothing about this.

Contributor

asmeurer commented Jun 18, 2017

This should at least be documented. nikola github_deploy --help says nothing about this.

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 18, 2017

Contributor

I also really think that the default should be False for that variable. I don't really understand that use-case. To me, the purpose of github_deploy is to add the built pages to the gh-pages branch and push it up, nothing more.

Contributor

asmeurer commented Jun 18, 2017

I also really think that the default should be False for that variable. I don't really understand that use-case. To me, the purpose of github_deploy is to add the built pages to the gh-pages branch and push it up, nothing more.

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 18, 2017

Contributor

OK, I tried setting that (is this correct?), but I still get this, indicating it is trying to do stuff with the "source" branch.

nikola github_deploy
[2017-06-18T22:45:39Z] WARNING: Nikola: The COMMENTS_IN_STORIES option is deprecated, use COMMENTS_IN_PAGES instead.
[2017-06-18T22:45:41Z] WARNING: Nikola: The COMMENTS_IN_STORIES option is deprecated, use COMMENTS_IN_PAGES instead.
fatal: ambiguous argument 'master': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
Scanning posts............done!
.  render_posts:timeline_changes
Scanning posts............done!
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/doit/doit_cmd.py", line 167, in run
    return command.parse_execute(args)
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/doit/cmd_base.py", line 120, in parse_execute
    return self.execute(params, args)
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugin_categories.py", line 134, in execute
    return self._execute(options, args)
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugins/command/github_deploy.py", line 108, in _execute
    self._commit_and_push(options['commit_message'])
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugins/command/github_deploy.py", line 150, in _commit_and_push
    source_commit = uni_check_output(['git', 'rev-parse', source])
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugins/command/github_deploy.py", line 43, in uni_check_output
    o = subprocess.check_output(*args, **kwargs)
  File "/opt/python/3.6.1/lib/python3.6/subprocess.py", line 336, in check_output
    **kwargs).stdout
  File "/opt/python/3.6.1/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['git', 'rev-parse', 'master']' returned non-zero exit status 128.
Note: checking out '671a86bd131578910a821c6c772746bae9d6b5b3'.

The context here is that I am trying to get my blog to deploy to gh-pages from Travis CI, which doesn't have master in this case because it's a different branch name that I'm building from.

Contributor

asmeurer commented Jun 18, 2017

OK, I tried setting that (is this correct?), but I still get this, indicating it is trying to do stuff with the "source" branch.

nikola github_deploy
[2017-06-18T22:45:39Z] WARNING: Nikola: The COMMENTS_IN_STORIES option is deprecated, use COMMENTS_IN_PAGES instead.
[2017-06-18T22:45:41Z] WARNING: Nikola: The COMMENTS_IN_STORIES option is deprecated, use COMMENTS_IN_PAGES instead.
fatal: ambiguous argument 'master': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
Scanning posts............done!
.  render_posts:timeline_changes
Scanning posts............done!
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/doit/doit_cmd.py", line 167, in run
    return command.parse_execute(args)
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/doit/cmd_base.py", line 120, in parse_execute
    return self.execute(params, args)
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugin_categories.py", line 134, in execute
    return self._execute(options, args)
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugins/command/github_deploy.py", line 108, in _execute
    self._commit_and_push(options['commit_message'])
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugins/command/github_deploy.py", line 150, in _commit_and_push
    source_commit = uni_check_output(['git', 'rev-parse', source])
  File "/home/travis/virtualenv/python3.6.1/lib/python3.6/site-packages/nikola/plugins/command/github_deploy.py", line 43, in uni_check_output
    o = subprocess.check_output(*args, **kwargs)
  File "/opt/python/3.6.1/lib/python3.6/subprocess.py", line 336, in check_output
    **kwargs).stdout
  File "/opt/python/3.6.1/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['git', 'rev-parse', 'master']' returned non-zero exit status 128.
Note: checking out '671a86bd131578910a821c6c772746bae9d6b5b3'.

The context here is that I am trying to get my blog to deploy to gh-pages from Travis CI, which doesn't have master in this case because it's a different branch name that I'm building from.

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 19, 2017

Member

This is documented in the manual: https://getnikola.com/handbook.html#deployment

GITHUB_COMMIT_SOURCE controls whether or not the source branch is automatically committed to and pushed. We recommend setting it to True, unless you are automating builds with Travis CI.

And we have a guide for Travis as well: https://getnikola.com/blog/automating-nikola-rebuilds-with-travis-ci.html

Member

Kwpolska commented Jun 19, 2017

This is documented in the manual: https://getnikola.com/handbook.html#deployment

GITHUB_COMMIT_SOURCE controls whether or not the source branch is automatically committed to and pushed. We recommend setting it to True, unless you are automating builds with Travis CI.

And we have a guide for Travis as well: https://getnikola.com/blog/automating-nikola-rebuilds-with-travis-ci.html

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 19, 2017

Contributor

Well I did set GITHUB_COMMIT_SOURCE to False, and it still wants to commit use the source. So either the docs are wrong or there is a bug. I would prefer the latter interpretation because I would really like the command to be modified so that it doesn't use git for anything other than the gh-pages branch.

And being documented in the manual is a lot less helpful than being documented in the command help. One is easy to find and the other is only found after a core developer points it out to you.

Contributor

asmeurer commented Jun 19, 2017

Well I did set GITHUB_COMMIT_SOURCE to False, and it still wants to commit use the source. So either the docs are wrong or there is a bug. I would prefer the latter interpretation because I would really like the command to be modified so that it doesn't use git for anything other than the gh-pages branch.

And being documented in the manual is a lot less helpful than being documented in the command help. One is easy to find and the other is only found after a core developer points it out to you.

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 19, 2017

Member

Regarding the manual vs command line: documenting software is hard. We expect users to read the manual as well if they want to know more. The manual is also a place where we can describe everything in prose, not tied to config variables or commands.

Could you please show output from a session where GITHUB_COMMIT_SOURCE = False doesn’t work? What commands are displayed?

Member

Kwpolska commented Jun 19, 2017

Regarding the manual vs command line: documenting software is hard. We expect users to read the manual as well if they want to know more. The manual is also a place where we can describe everything in prose, not tied to config variables or commands.

Could you please show output from a session where GITHUB_COMMIT_SOURCE = False doesn’t work? What commands are displayed?

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 19, 2017

Contributor

Regarding the manual vs command line: documenting software is hard. We expect users to read the manual as well if they want to know more. The manual is also a place where we can describe everything in prose, not tied to config variables or commands.

Well you could at least link to the manual from the command line.

Could you please show output from a session where GITHUB_COMMIT_SOURCE = False doesn’t work? What commands are displayed?

See #2847 (comment)

Contributor

asmeurer commented Jun 19, 2017

Regarding the manual vs command line: documenting software is hard. We expect users to read the manual as well if they want to know more. The manual is also a place where we can describe everything in prose, not tied to config variables or commands.

Well you could at least link to the manual from the command line.

Could you please show output from a session where GITHUB_COMMIT_SOURCE = False doesn’t work? What commands are displayed?

See #2847 (comment)

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 19, 2017

Member

Ah, I see. This isn’t creating a new commit, it merely tries to grab the commit hash for the source commit. This should work if you’re running on the source (master) branch, because that’s the branch that’s just been cloned. Travis should be disabled for the deploy (gh-pages) branch.

Member

Kwpolska commented Jun 19, 2017

Ah, I see. This isn’t creating a new commit, it merely tries to grab the commit hash for the source commit. This should work if you’re running on the source (master) branch, because that’s the branch that’s just been cloned. Travis should be disabled for the deploy (gh-pages) branch.

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 19, 2017

Contributor

That was the Travis "push" build. It also ran in the Travis "PR" build, which apparently does have a master, and it tried to run it from master (I know because it failed there, because of a fix I did in my branch for the newest version of Nikola). So the flag doesn't do at all what it says it does.

Contributor

asmeurer commented Jun 19, 2017

That was the Travis "push" build. It also ran in the Travis "PR" build, which apparently does have a master, and it tried to run it from master (I know because it failed there, because of a fix I did in my branch for the newest version of Nikola). So the flag doesn't do at all what it says it does.

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 19, 2017

Member

Yes, it does. With the option enabled, the thing creates a commit with git commit et al (lines 133-147). But regardless of that setting, we use rev-parse to get the latest commit hash to use in the commit message. Your scenario and usage is kinda unusual (using different branches and some third-party tool). Since that source check is not important, we could make failure non-fatal.

Member

Kwpolska commented Jun 19, 2017

Yes, it does. With the option enabled, the thing creates a commit with git commit et al (lines 133-147). But regardless of that setting, we use rev-parse to get the latest commit hash to use in the commit message. Your scenario and usage is kinda unusual (using different branches and some third-party tool). Since that source check is not important, we could make failure non-fatal.

@Kwpolska Kwpolska reopened this Jun 19, 2017

@Kwpolska Kwpolska changed the title from Why does github_deploy try to deploy from master by default to github_deploy shouldn’t fail if it can’t determine the source commit Jun 19, 2017

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 19, 2017

Contributor

I've ditched using github_deploy for Travis (I'll still use it for deploying locally, if I ever decide to do that). It is trying to be too smart, and all it really needs to do is sync files from output/ to the gh-pages branch, which doctr, the tool I wrote, already does.

I wrote a post on how I am deploy my site. I much prefer it to the method outlined in your docs because doctr handles all the SSH key stuff for you, and also the syncing.

I still think that github_deploy should (by default) just deploy the files as they are, i.e., even if there are uncommitted files they should be built and deployed.

Contributor

asmeurer commented Jun 19, 2017

I've ditched using github_deploy for Travis (I'll still use it for deploying locally, if I ever decide to do that). It is trying to be too smart, and all it really needs to do is sync files from output/ to the gh-pages branch, which doctr, the tool I wrote, already does.

I wrote a post on how I am deploy my site. I much prefer it to the method outlined in your docs because doctr handles all the SSH key stuff for you, and also the syncing.

I still think that github_deploy should (by default) just deploy the files as they are, i.e., even if there are uncommitted files they should be built and deployed.

@Kwpolska Kwpolska closed this in 7e98b0e Jun 20, 2017

Kwpolska added a commit that referenced this issue Jun 20, 2017

Merge pull request #2848 from getnikola/fix-2847
Fix #2847 – non-fatal rev-parse failures in github_deploy
@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 20, 2017

Contributor

@Kwpolska what about the other issues here?

Contributor

asmeurer commented Jun 20, 2017

@Kwpolska what about the other issues here?

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 20, 2017

Member

What issues? github_deploy DOES NOT change branches nor commit anything if the option is disabled. We use ghp-import to do all the heavy lifting, and AFAIK it shouldn’t force any branching either. The last commit should fix the crash you had in #2847 (comment).

Member

Kwpolska commented Jun 20, 2017

What issues? github_deploy DOES NOT change branches nor commit anything if the option is disabled. We use ghp-import to do all the heavy lifting, and AFAIK it shouldn’t force any branching either. The last commit should fix the crash you had in #2847 (comment).

Kwpolska added a commit that referenced this issue Jun 20, 2017

Backport #2847/#2848 and update changelog
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 20, 2017

Contributor

You can tell me if I enabled the option wrong, but that's not what I saw. I'll test it again later.

There's also the documentation issue.

And lastly, I really think the defaults should change here.

Contributor

asmeurer commented Jun 20, 2017

You can tell me if I enabled the option wrong, but that's not what I saw. I'll test it again later.

There's also the documentation issue.

And lastly, I really think the defaults should change here.

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Jun 21, 2017

Member

You can tell me if I enabled the option wrong, but that's not what I saw. I'll test it again later.

You’re confusing two things here. Nikola tries to fetch the commit hash from the source branch. I made failures to do that non-fatal in #2848 . If the auto-commit mode is enabled, you would see something like ==> ['git', 'checkout', 'master'] in output — and that did not appear.

There's also the documentation issue.

I’ll add a link to the manual to nikola help github_deploy. (The online manual is better for showing long docs, plus it’s less documents to keep it in sync.)

And lastly, I really think the defaults should change here.

I disagree. The default is handy for users of github_deploy who use the command manually. And you need to configure github_deploy to get it to work anyway.

Member

Kwpolska commented Jun 21, 2017

You can tell me if I enabled the option wrong, but that's not what I saw. I'll test it again later.

You’re confusing two things here. Nikola tries to fetch the commit hash from the source branch. I made failures to do that non-fatal in #2848 . If the auto-commit mode is enabled, you would see something like ==> ['git', 'checkout', 'master'] in output — and that did not appear.

There's also the documentation issue.

I’ll add a link to the manual to nikola help github_deploy. (The online manual is better for showing long docs, plus it’s less documents to keep it in sync.)

And lastly, I really think the defaults should change here.

I disagree. The default is handy for users of github_deploy who use the command manually. And you need to configure github_deploy to get it to work anyway.

Kwpolska added a commit that referenced this issue Jun 21, 2017

Add handbook link to github_deploy help (#2847)
Signed-off-by: Chris Warrick <kwpolska@gmail.com>

Kwpolska added a commit that referenced this issue Jun 21, 2017

Add handbook link to github_deploy help (#2847)
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 21, 2017

Contributor

OK, I took a deeper look and I think another part of my build was checking out master.

Contributor

asmeurer commented Jun 21, 2017

OK, I took a deeper look and I think another part of my build was checking out master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment