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

Action does not deploy CNAME or .nojekyll files #354

Closed
LPGhatguy opened this issue Jul 5, 2020 · 11 comments
Closed

Action does not deploy CNAME or .nojekyll files #354

LPGhatguy opened this issue Jul 5, 2020 · 11 comments
Assignees
Labels
feature request ✨ New feature or request planned 🕘 This feature is planned. version 3 Issues related to version 3 of this action.

Comments

@LPGhatguy
Copy link

I recently started a new project using this action to deploy to GitHub Actions. My page generator creates CNAME and .nojekyll files, but I noticed that my gh-pages branch didn't end up with either of those files!

It appears that, although this action preserves those files if they were already there, it does not add them if they weren't there already. I have since manually added those files by hand. I read in this repository's README that CNAME and .nojekyll are special, which leads me to believe there may be a bug.

My use of the workflow is:

      - name: Deploy
        uses: JamesIves/github-pages-deploy-action@releases/v3
        with:
          ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }}
          BRANCH: gh-pages
          FOLDER: dist
          CLEAN: true

Here is a run of the relevant workflow: https://github.com/LPGhatguy/rojo.space-v2/runs/837921047?check_suite_focus=true

This run happened after I manually added CNAME (because I noticed it was missing), but before I manually added .nojekyll. There are no error messages or output to report, just this funny behavior.

@JamesIves
Copy link
Owner

JamesIves commented Jul 5, 2020

You need to manually commit them on the gh-pages branch, they won't be removed by the action afterwards. The CLEAN option provides some behavior that ignores these files otherwise build inconsistencies will cause them to be removed. (For instance if they weren't present in your build files and the action didn't ignore them they would be nuked on each deployment).

I'm going to close this as I don't believe this is a bug as per say, just a side effect of how CLEAN works and an effort to keep the README consistent. You can extend the list of ignored files by CLEAN using the CLEAN_EXCLUDES parameter.

@JamesIves JamesIves added support ❓ Issues related to action support. version 3 Issues related to version 3 of this action. labels Jul 5, 2020
@LPGhatguy
Copy link
Author

Hey, thanks for the response!

The README does not mention that these files need to be manually committed, only that CLEAN won't remove them. It is counter-intuitive to me that the action would not deploy these files. This requires manual intervention from the user, instead of letting my deployment script handle both of those values.

I'm not looking for those files to be cleaned. Instead, I want the files to be created if they're present. This is important for template repositories that use this action. With this bug, a user is required to manually create two files and push them to the repository or else the automated deployment will not work.

Do you think this behavior should be documented? s there a way to use CLEAN_EXCLUDES to force the action to commit these files?

@JamesIves
Copy link
Owner

JamesIves commented Jul 6, 2020

It's mentioned under the Additional Build Files section. The user needs to manually commit them for a few reasons:

  • The value of a CNAME file is specific to a repository/domain.
  • In a typical deployment setting you don't want to override these files or update them once they have been set.
  • If they were not ignored in the deployment process, and you were to remove the CNAME generation from your build script accidentally it's going to break your entire website due to how this option works.

In my opinion intervention from the user to commit these files once is a much better option than creating the potential scenario where a website goes down because of a mis-configured or non existent CNAME file in your distribution folder. Checking the existence of these files and conditionally ignoring them if they don't exist would be an improvement, but I'm cautious to make this change due to the impact it could have on everyone currently using this action, it would likely require a major version bump.

Can you explain the use case of generating a CNAME file via a template?

@JamesIves JamesIves reopened this Jul 6, 2020
@LPGhatguy
Copy link
Author

I'm sorry, I can't find what you're referring to in the docs. The "Additional Build Files" section says this for me right now:

If you're using a custom domain and require a CNAME file, or if you require the use of a .nojekyll file, you can safely commit these files directly into deployment branch without them being overridden after each deployment.

This section tells me that I can manually commit CNAME and .nojekyll files, but this doesn't look like the part of the docs that tells me I need to commit them, or that the tool will not create them.

I agree that the CNAME file being misconfigured is a very bad scenario. It also seems that many site generators do not include it themselves. In the past, I have used a manual step to copy mine to the right place before deploying.

I want to make it clear that I don't think the CLEAN option should remove your CNAME file. That would lead to the scenario you're talking about, which would be very bad! I just want to deploy or update them from my repository if they're present. The folder I am deploying from has these files.

My use case for .nojekyll and CNAME files in a template is as part of a push to make building docs websites easier for users in my community. Many of them are new to Git and GitHub Pages and may struggle debugging the issues that come from missing files like .nojekyll in particular. The template I'm working on should be able to handle these files to try to prevent those issues.

I think the more important file for this case is .nojekyll. It's easy to create a site that will run into issues if this file is not present, and debugging the issue is difficult, especially if you're not sure what to look for.

@JamesIves JamesIves added the feature request ✨ New feature or request label Jul 6, 2020
@JamesIves
Copy link
Owner

JamesIves commented Jul 7, 2020

Got it - Let me do some more thinking on the best way to handle this. I think checking the existence of the files first before selectively ignoring them (in the case of clean) would be the best option. There's an issue I ran into with #310 that I'm working on fixing which will likely pave the way for this to become possible.

I may have some time this weekend so I'll see if I can get a working draft ready by then.

@LPGhatguy
Copy link
Author

Thank you! Let me know if there's anything I can do to help.

@JamesIves
Copy link
Owner

JamesIves commented Jul 12, 2020

@LPGhatguy I've done some work on this and I think I have a reasonable solution for you. Can you try this again but use this branch of the action: uses: JamesIves/github-pages-deploy-action@releases/v3-cname - it should now conditionally ignore CNAME and .nojekyll if they don't exist in your deployment folder. This will prevent a scenario where if you leave them out of your deployment folder they won't be nuked from the deployment branch by the clean option, but if they do exist the action will deploy/update them for you.

I'll need to make a README adjustment if that explains how you should delete these files should the need arise as you'll need to manually remove them from the deployment branch.

Let me know if this works for your case! I'll push it up a release with this Monday if so.

@JamesIves JamesIves added planned 🕘 This feature is planned. and removed support ❓ Issues related to action support. labels Jul 12, 2020
@JamesIves JamesIves self-assigned this Jul 12, 2020
LPGhatguy added a commit to LPGhatguy/parcel-ssg-example that referenced this issue Jul 13, 2020
@LPGhatguy
Copy link
Author

LPGhatguy commented Jul 13, 2020

This fix looks good to me. The commit that GitHub auto-linked just above created this commit: LPGhatguy/parcel-ssg-example@ea542d6

There is no CNAME file for this project, but it successfully created .nojekyll! I assume that the solution you made is the same or similar for both files. Is this a good enough indicator of success?

Thank you!

@JamesIves
Copy link
Owner

JamesIves commented Jul 13, 2020

The same logic applies to both. I'll go ahead and push this up later today!

I'll let you know when it's available in the release.

@JamesIves
Copy link
Owner

Released as part of 3.5.8: https://github.com/JamesIves/github-pages-deploy-action/releases/tag/3.5.8

@LPGhatguy
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request ✨ New feature or request planned 🕘 This feature is planned. version 3 Issues related to version 3 of this action.
Projects
None yet
Development

No branches or pull requests

2 participants