helaili / jekyll-action

A GitHub Action to publish Jekyll based content as a GitHub Pages site
MIT License
250 stars 120 forks source link

remote_branch variable is not correctly detected #26

Closed mberlanda closed 4 years ago

mberlanda commented 4 years ago

I am running this action on a repository like organization/organization.github.io When running the action in a pull request, the consequences of this code snippet are potentially destructive

https://github.com/helaili/jekyll-action/blob/af3293c2d6f25c063e2a3448433fe47d4d65b11c/entrypoint.sh#L30-L35

In my repository, the script tried to push to gh-pages branch even if it should not You may need to add a caveat in the documentation

limjh16 commented 4 years ago

Hey! In the workflow file, it only allows this action to run on push, so it shouldn't affect PRs https://github.com/helaili/jekyll-action/blob/d59da9641cc5462003464e6b9ecf7cd7f77116fb/.github/workflows/test.yml#L3-L4 Not sure why it doesn't detect the right repository though, I never tested it on a .github.io repo, maybe the `behind.io"` is not needed? Alternatively, there is an action https://github.com/marketplace/actions/github-pages-action to give the user more control over which branch / repo to push to, whether to force push, which user commits, or to only build and not push the site in a PR, etc.

mberlanda commented 4 years ago

Hey @limjh16 I would not backport all these aspects in this action (I am strongly against copy/pasting code across repos).

Probably the scope of this action could be narrowed down specifying the supported versions and pinning them (also regarding #25 ).

This could be an high-level action for non-rubysts and refer the other action in the doc in a section like what this action does not

What would you think?

mberlanda commented 4 years ago

Regarding the comment about on: push, I don't see a branch restriction, so I assume that if I open a pull request on the branch foobar, the commit will be built and published

limjh16 commented 4 years ago

Maybe the README should suggest adding a branch restriction to master or whichever branch the main code is in... something like this?

on:
  push:
    branches:
      - master

Also, your suggestion sounds good, I think most people won't have an issue with the latest bundler and ruby version anyways, and if they do need a specific version it isn't that difficult to modify this action to suit their own needs

Harrypulvirenti commented 4 years ago

Hey I'm facing one issue that probably could be related to this topic. I'm trying to run the action on my organization repo Harrypulvirenti/harrypulvirenti.github.io but I'm receiving this message Cannot publish on branch master

Using a non-organization repo my setup works without problems.

How can I fix this issue?

helaili commented 4 years ago

It's basically because your source and your target are both set to master, so publishing would overwrite the source.

Move the source to a different branch, and trigger the workflow when a push happens on this branch. master will then contain the generated version of your site, and you should never commit on this branch as it will be overwritten eventually.

Harrypulvirenti commented 4 years ago

@helaili thanks for the answer. Now I'm receiving a different error message fatal: could not read Password for 'https://***@github.com': No such device or address

helaili commented 4 years ago

Can you set a ACTIONS_STEP_DEBUG secret set to true and re-run the workflow?

Harrypulvirenti commented 4 years ago

Done!

this is the result

Jekyll build done
Publishing to Harrypulvirenti/harrypulvirenti.github.io on branch master
##[debug]Pushing to https://***@github.com/Harrypulvirenti/harrypulvirenti.github.io.git
Initialized empty Git repository in /github/workspace/build/.git/
[master (root-commit) f8736b7] jekyll build from Action 1da095a9193cee0eb632e602d0a404946f1b46c4
 9 files changed, 691 insertions(+)
 create mode 100644 .nojekyll
 create mode 100644 404.html
 create mode 100644 about/index.html
 create mode 100644 assets/main.css
 create mode 100644 assets/main.css.map
 create mode 100644 assets/minima-social-icons.svg
 create mode 100644 feed.xml
 create mode 100644 index.html
 create mode 100644 jekyll/update/2020/05/26/welcome-to-jekyll.html
fatal: could not read Password for 'https://***@github.com': No such device or address
##[debug]Docker Action run completed with exit code 128
helaili commented 4 years ago

I've forked your repo and tried, it works perfectly. Only changed I've made is to switch the default branch from master to source and changed the workflow trigger as below. My guess is there is a problem with the personal access token you have generated. Can you try generating again the PAT and updating the JEKYLL_PAT secret?

on:
  push:
    branches-ignore: 
      - master
Harrypulvirenti commented 4 years ago

I regenerated the token and I changed the name of the secret to JEKYLL_PAT and now is working! thanks for the help