Skip to content

Conversation

@JamesonRGrieve
Copy link

@JamesonRGrieve JamesonRGrieve commented Oct 30, 2025

Scripts wich are clearly AI generated and not further revied by the Author of this PR (in terms of Coding Standards and Script Layout) may be closed without review.

✍️ Description

Implements the calling syntax export BASE_URL=https://raw.githubusercontent.com/USER/REPO/COMMIT && bash -c "$(curl -fsSL "$BASE_URL"/ct/erpnext.sh)" to allow for deployment of forked scripts without the risk of find-and-replace artifacts leaking. The variable is defaulted to the initial values where I implemented it.

Note that I only updated the necessary references for testing development scripts - for full implementation I would recommend similarly updating all references to hard-coded URLs similarly (I noticed they don't all point to the same place, some github, some mirror, etc - intentional?).

I've confirmed this method works while developing the ERPNext container - exact syntax / POSIX compliance / etc at the mercy of maintainers to maintain code standards, will update as needed but this is a HUGE quality of life improvement vs find-and-replace.

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards (as much as it can for a core update).
  • Tested thoroughly – Changes work as expected.
  • No breaking changes – Existing functionality remains intact.
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing functionality in a way that may require updates.
  • 🆕 New script – A fully functional and tested script or script set.
  • 🌍 Website update – Changes to website-related JSON files or metadata.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, AppName.md, CONTRIBUTING.md, or other docs.

🔍 Code & Security Review (X in brackets)

  • Follows Code_Audit.md & CONTRIBUTING.md guidelines (updated to match)
  • Uses correct script structure (AppName.sh, AppName-install.sh, AppName.json)
  • No hardcoded credentials

@JamesonRGrieve JamesonRGrieve marked this pull request as ready for review October 30, 2025 22:37
@JamesonRGrieve JamesonRGrieve requested a review from a team as a code owner October 30, 2025 22:37
@michelroegl-brunner
Copy link
Member

Can you append that to all location where it is needed? we tried that sometime back but it always broke with the subshells. When this is cahnged and working, i will test it furhter. @JamesonRGrieve

@JamesonRGrieve
Copy link
Author

Can you append that to all location where it is needed? we tried that sometime back but it always broke with the subshells. When this is cahnged and working, i will test it furhter. @JamesonRGrieve

It works for development of new containers as-is (I used it for the ERPNext one) - changing other ones would only be necessary for fork development of core functionality. I'm not super familiar with the core scripts so I'm hesitant to touch them but I can give it a go when I have some more time if you wish.

@michelroegl-brunner
Copy link
Member

@MickLesk Do we want the core parts also to be done or work for the moment to allow easy development, where the core (tools.func, core.func ,... ) alsways gets pulled from our github?

@MickLesk
Copy link
Member

Finalize Build.func, i think this week

@SunFlowerOwl
Copy link
Contributor

SunFlowerOwl commented Nov 17, 2025

Thank you for this. I was actually about to open a feature request.
I just spent some time fixing my URL paths after merging main into my test branch, so if I had seen this PR earlier, I would have waited. ^^

In my case, I’m using a fork hosted on my self-hosted Gitea instance (thanks to the community project).
To access it, I need to include this header before the URL:

-H "Authorization: token ************************************"

Have you tested whether this works with authenticated Gitea instances as well?

@michelroegl-brunner
Copy link
Member

Thank you for this. I was actually about to open a feature request. I just spent some time fixing my URL paths after merging main into my test branch, so if I had seen this PR earlier, I would have waited. ^^

In my case, I’m using a fork hosted on my self-hosted Gitea instance (thanks to the community project). To access it, I need to include this header before the URL:

-H "Authorization: token ************************************"

Have you tested whether this works with authenticated Gitea instances as well?

It is not tested and this would need some heavy rework. We need to wait until Mick finishes up with the core changes anyway until we can proceed further with this.
But i dont think we will support authentication out of the box, but this needs further discussion.

@JamesonRGrieve
Copy link
Author

Thank you for this. I was actually about to open a feature request. I just spent some time fixing my URL paths after merging main into my test branch, so if I had seen this PR earlier, I would have waited. ^^

Happy to help. 😄

@JamesonRGrieve
Copy link
Author

It is not tested and this would need some heavy rework. We need to wait until Mick finishes up with the core changes anyway until we can proceed further with this.

I mean I've done two new services using it now and it's worked flawlessly so any necessary tweaks should be minor. The quality of life improvement is definitely worth it.

@AlphaLawless
Copy link
Contributor

It is not tested and this would need some heavy rework. We need to wait until Mick finishes up with the core changes anyway until we can proceed further with this.

I mean I've done two new services using it now and it's worked flawlessly so any necessary tweaks should be minor. The quality of life improvement is definitely worth it.

What you did is easier than what I did. And I even dared to call the script "easily".

If you want to check, I think it can be integrated with what you did, since it checks a few other things.

https://gist.github.com/AlphaLawless/6d30173b3422d7864aa5e22e633f541c

@JamesonRGrieve
Copy link
Author

It is not tested and this would need some heavy rework. We need to wait until Mick finishes up with the core changes anyway until we can proceed further with this.

I mean I've done two new services using it now and it's worked flawlessly so any necessary tweaks should be minor. The quality of life improvement is definitely worth it.

What you did is easier than what I did. And I even dared to call the script "easily".

If you want to check, I think it can be integrated with what you did, since it checks a few other things.

https://gist.github.com/AlphaLawless/6d30173b3422d7864aa5e22e633f541c

The nice thing about the env var method is it's relatively oversight proof - once implemented it's impossible for someone to forget to undo their changes to paths in core files, impossible for a maintainer to miss a change to core files in a code review relating to not reverting paths, etc. Core files never need to be touched to reference forks.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants