Opened 18 months ago

Closed 14 months ago

Last modified 14 months ago

#5336 closed change (fixed)

Allow additional include, page, and template paths using CMS

Reported by: juliandoucette Assignee: kvas
Priority: P2 Milestone: Websites code sharing
Module: Sitescripts Keywords:
Cc: wspee, kvas, jsonesen, ire, saroyanm, juliandoucette Blocked By:
Blocking: #5414, #5654, #5828 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29555839/

Description (last modified by kvas)

Background

  • We would like to share code across websites
  • If I am able to add dependencies to websites (see #5335)
    • Then I could also add generic includes, pages, and templates to website-defaults
      • If CMS would allow me to load from additional paths

So we need to add a possibility to add additional source paths to the standard paths that the CMS uses (${siteroot}/pages, ${siteroot}/globals, etc).

What to change

Add a section to settings.ini called [paths] with one option in it: additional-paths. The value of this option will be a list of strings, separated by newlines, each of which is an additional root (or source, in CMS internal terms), from where includes, globals and templates can be loaded.

Notes

  • The additional paths should apply to generation and preview equally.
  • Currently generation uses Mercurial a committed revision (last revision of default branch or something else if specified) instead of the directory content as a source. The directories added by additional-paths option would need to refer to the local file system, not to the repository (usually the wouldn't exist in the repository). This further complicates current confusing behavior but should become ok once we uncomplicate that (TODO).

Example

If I specify the following in settings.ini:

[paths]
additional-paths =
    node_modules/common_stuff
    node_modules/rare_stuff

Then I:
<? include example ?>

Then:

  1. CMS should check includes/example
  2. If it doesn't find example there then it should look for includes/example under paths specified in paths/additional-paths, if any.

Change History (21)

comment:1 Changed 18 months ago by juliandoucette

  • Description modified (diff)

I forgot about globals :D


Real world example: If implemented, I could define a standard set of meta data fields inside includes in website-defaults and share these includes across websites (if I am able to add dependencies - #5335) instead of duplicating them in each website repository.

comment:2 follow-up: Changed 18 months ago by kvas

While I agree that code sharing across websites would be a nice feature (and the practice makes sense to me), I feel uneasy about the feature as it is specified now. The problem here is that we would start relying on some mechanisms outside of control of CMS (and the version control system) to get those dependencies into place and this makes the builds not reproducible. To have reliable code sharing we'd need to add something like dependencies between websites (or website depending on repository with website fragments). How to do this without reinventing too many wheels is an interesting question.

One approach would be to rely on #5335 for getting the dependencies into place. If we have build steps, we could add a pre-build step that will clone the repositories that we depend on into a subfolder and then we would know that they will be there and the build is repeatable. This is still a bit fragile for my taste, and it's not completely clear how this would interact with the preview feature of CMS (although preview interaction needs to be solved for #5335 to work anyway). What I like about this approach, though, is that it's flexible for the website developers and doesn't add new concepts to the CMS.

Another option would be to add the notion of dependencies between websites directly to CMS. Then CMS would know that it needs to clone some other repos during the build, put them somewhere and make the globals/includes/pages/templates from them available. This seems a bit more robust and easier to use, but then CMS would need to know even more about Mercurial (or potentially also Git and God knows what in the future) and that seems like too much stuff to bring into the CMS domain.

Does anybody have any other ideas or thoughts?

comment:3 Changed 18 months ago by wspee

Another consideration might be a potential staging system. It makes sense to be able to setup a production like staging system of a future version for testing or similar. Currently this is not possible, which means we cannot test dynamic functionality, which means dynamic stuff is likely to break, see for example #5004.

It seems like it might be good idea to incorporate all this aspects into a joint effort?!

comment:4 in reply to: ↑ 2 ; follow-up: Changed 18 months ago by juliandoucette

Replying to kvas:

Does it make you more comfortable if I remove the fallback requirement (not searching one directory and then another)?

comment:5 in reply to: ↑ 4 Changed 18 months ago by kvas

Replying to juliandoucette:

Replying to kvas:

Does it make you more comfortable if I remove the fallback requirement (not searching one directory and then another)?

Not really. I think the searching actually makes sense this way but we need to ensure that the content of those directories is the same in development, testing, production, etc. without requiring that someone maintains them so. Ideally we would like adding and removing dependencies to a website to be transparent.

comment:6 Changed 17 months ago by ire

  • Cc ire added; iaderinokun removed

comment:7 Changed 17 months ago by kvas

  • Description modified (diff)

comment:8 Changed 17 months ago by kvas

@juliandoucette: what do you think about the way I worded it? Sounds about right?

comment:9 Changed 17 months ago by kvas

  • Description modified (diff)

comment:10 Changed 17 months ago by juliandoucette

  • Cc juliandoucette added

I'd rather:

[extra-paths]
node_modules/website-defaults

Than:

[extra-paths]
includes=
    node_modules/website-defaults/includes
globals=
    node_modules/website-defaults/globals
...

But I'm not against having both options :)


PS: How about just "paths", not "exra-paths".
OR (if you insist) "additional-paths" ("extra" ~implies "unnecessary").

Last edited 17 months ago by juliandoucette (previous) (diff)

comment:11 Changed 17 months ago by kvas

  • Description modified (diff)

comment:12 Changed 17 months ago by kvas

  • Priority changed from Unknown to P3
  • Ready set

comment:13 Changed 17 months ago by kvas

  • Priority changed from P3 to P2

comment:14 Changed 15 months ago by juliandoucette

  • Milestone set to Websites code sharing

comment:15 Changed 15 months ago by juliandoucette

  • Blocking 5654 added

comment:16 Changed 15 months ago by ire

  • Blocking 5414 added

comment:17 Changed 15 months ago by kvas

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:18 Changed 15 months ago by kvas

  • Owner set to kvas

comment:19 Changed 14 months ago by abpbot

A commit referencing this issue has landed:
Fixes 5336 - Allow additional include, page, and template paths

comment:20 Changed 14 months ago by kvas

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:21 Changed 14 months ago by kvas

  • Blocking 5828 added
Note: See TracTickets for help on using tickets.