Forum OpenACS Development: Core team heads-up. Check for curriculum bar in default master committed to HEAD

I just committed a small change to the default-master, that the core team might want to review.

This is what went into the cvs log:
"I added a check for whether the curriculum package is installed or not to the default-master. If it is installed, the (possibly empty) bar will be included. This won't fail even if there's no instance of curriculum mounted (knock wood)"

If I should undo the change, I bet I'll hear about it 😉

Also, there's now a simple demo of the curriculum package available:
http://oacs4.polyxena.net/curriculum/

Let me know if you want admin privileges on it.

/Ola

After having checked in all the initial files belonging to our package, I checked out a new "working copy" ... However, all the files have become read-only!?

Does anyone know why I see this behavior?

Does the cvs repository have some kind of admin file for file perms?

Also, while I'm at it - could I ask someone on the core team a favor, namely to add a cvs module for "curriculum"? Thanks!

Do we really want package specific stuff in the default-master? I vote no - if this becomes a trend then we'll have a default-master cluttered with 10 different if clauses that no one knows and so complicated that the average openacs user doesn't dare to touch and modify it, and possibly related problems with CVS conflicts on a customized default-master (I just updated my checkout and the curriculum bar conflicts with some modifications I made to that file - it wouldn't be nice if that happens everytime something regarding the package specific parts gets changed in the repository).

What about a little instruction in the docs of curriculum where and how to add the <include> tag for the bar?

developer-support is a bad example, agreed. One could argue that developer-support is somehow special, because it isn't a user application, but I wouldn't have it included in the default-master either.

What do others think about that?

Collapse
8: CVS and file permissions (response to 2)
Posted by Andrew Piskorski on
Ola, AFAIK CVS has no reliable way to preserve file permissions. What you are seeing is more or less normal. I never bothered to track the details down, but presumably your umask is set in such a way that CVS makes the files read only. Or something.

If you figure out or track down a full explanation of just how CVS ends up assigning file permissions to newly checked out or updated files, please let me know. That would help avoid annoyances like the one you just experienced.

But if you need reliable, reproduceable, specific permissions on specific files (in your case you don't), then AFAIK the only way to guarantee that with CVS is to write a little script that bashes permissions on each of the specific files you care about.

Definitely should not be a check in the default-master for any optional package, and if it is required, you wouldn't need to check. Maybe some type of general function that you could register your desires, but how and where? Maybe a package for the content bar to allow packages register what they want.

like the others above my vote is that non necessary package specific stuff doesn't get include in default-master. this seems like the type of thing you should have package specific documentation on that explains what people need to do.
I agree, there should be no package-specific stuff in the master template.  Why not use a package-level master template that sources the system-wide master? *

Or, if this is a positional issue then, along Tom's line of thinking, maybe the master template could use a generic variable similar to @header_stuff@ (call it @body_start@ and @body_end@).  Kludgy, but methinks still better than including package-specific code.

Rob

* Yikes, this embarrassingly relates to a bug which was assigned to me ages ago and for which Vinod has kindly provided a patch:
https://openacs.org/bugtracker/openacs/bug?bug%5fnumber=68

Oh, silly me, it just occurred to me the curriculum bar is something that should appear even if your in another package.  Is that the concept?

Hmmm... well that's a tougher problem to solve.  Ok, I'll shut up now...

Andrew is right in saying that there is no way to sanely preserve file permissions in CVS.  There is an experimental feature to somewhat solve this problem though (for those who like to live on the bleeding edge).  Refer to (URL may wrap):

http://cvsbook.red-bean.com/cvsbook.html#CVS_keeps_changing_file_permissions__why_does_it_do_that_

I'd also favour to take the change out of default-master and leave it package independent. But, why not offer a default-master.adp with the package, that you can copy over the existing default-master if the package is installed.

Or provide a patch for the default-master.adp that will be executed automatically after the installation of the package (I know, probably even more tweaking of the APM, but maybe it is a good option to offer to execute patches while installing a package).

Ola, I also think that having the curriculum bar in the default master is not a good idea. However, I suggest that you write a service contract for toolbars. The service contract should, at least, support a toolbar provider operation, i.e. the curriculum bar could be provided for all labels/context that match the expression "*" (let's call this the toolbar provider expression (TPE)).

Then, a package parameter, say ShowToolbarsP, in acs-templating or acs-kernel could be used to denoted whether to display toolbars or not. A simple extension to default-master would be required in that case, i.e. if the ShowToolbarsP switch is on, then forall TPEs that match "default-master", display the corresponding toolbar. Other toolbars that I can think of are for i18n, ecommerce, etc.

Now, suppose that you need to display a toolbar (not necessarily for curriculum) in all templates of a special type, say that you want to display a user's recent history of product views in templates that are related to the ecommerce package, then you could say assign a TPE of ec-* to your toolbar and the ecommerce package would have a template that requests toolbars with "ec-special1" or "ec-special2" and so on.

The toolbars should be either read-only or both read and write (one further package parameter could be used to denote the type of toolbars to process in each template). The read-only is obvious what it means. An example of a read-write toolbar is the recent-product history (introduced above), i.e. the recent-product-history would update the user's product views history every time it is rendered.

What others think?

Hi,

Why not just set the master template to a different template upon installation, the new APM does support tcl callbacks right?  And under the docs of curriculum just explain why.  Then leave it to the developer to merge the new tool bar to his template.  Normally in my projects I leave the default-template as is.  And on www/templates/foo/ I place the templates, images, css, etc.  So I guess in my case I will have to place that toolbar on my template as well.

I am not sure why service contracts, etc. will have to be done.  Its additional overhead and also additional complication for a newbie.  Maybe just a simple doc can do.  Or like bug-tracker or logger that checks if its not yet setup properly and prompts the user.  Say admin mounts curriculum then goes to /curriculum it then says that subsite template needs blah blah.  Maybe links to the doc to explain further etc.

Anyway just my opinion.

Yun, do you mean the default-master should be changed automatically when the curriculum package is being installed? I don't see how this would be anything else than confusing ...

I'd rather require the developer to insert the include tag manually.

Hi Til,

I am assuming that I am "Yun".  Yes you are right its confusing.  I think the later opinion that I presented is more sensible.  The curriculum package when first visited will have inform that needs to be setup like bug tracker and logger.  This is also the current path that I am taking in my packages.

Okay, I haven't removed the curriculum include nor the developer-support ditto from the default master yet (and someone has contacted me about it). Should I remove those two bits? Or just the curriculum include? Or none of it?

Core team, speak up, please! 😊

(I naively interpreted it as an implicit approval to let it stay when I read the answers to this thread; https://openacs.org/forums/message-view?message_id=105027 )

I was doing some scalability stuff today and noticed that the curriculum bar generates 7 db queries and takes 100ms of the 330ms a simple form page took on my box, and that is for a site with curriculum is installed but not mounted.

At the very least the check to determine whether to include the curriculum bar should be fast and not incur any db overhead, should check that it be mounted and not merely installed, and has be fast (that or we need to remove curriculum from the site-master/default-master and provide instructions on how to include it instead).

I will remove the check and the curriculum include from the template, and it will be documented how to include it. Should I remove the check for developer-support, too, while I'm at it?

I am also going to have to revisit and fix the cache structures in Curriculum - especially curriculum::conn. The current performance is unacceptable!

Thanks for pointing this out, Jeff.

/Ola

Ola, you can use site_node:: procs to find out if curriculum is mounted.  I would prefer developer support not be removed
since the check for it is quite fast.
Right, there is no need to remove the developer support stub since the check is very fast.  If curriculum is not mounted there's no need to do any db checking there, either, and the site node package Jeff refers to nsv's (caches) its info so using it to check to see if curriculum should be very fast, too.

Of course when it is mounted and being used it would be awfully nice if it ran faster but we can address that separately.

Hmm, I don't think what Don and Jeff just said can be interpreted in any other way than that if the curriculum check is fast (which it will be, I'm told) it is preferred to keep it in site-master.tcl ...

So, despite the objections to the principle of having ad hoc checks in the template, I won't remove it after all. (One reason why I don't mind keeping the check is that I think there will be less confusion about what Curriculum offers, and less support calls, too.)

I've hosed my own oacs development copy at the moment as I am in the process of debugging my new caching solution in curriculum::conn, so I'm holding off improving the curriculum check in site-master.tcl until I get things straight - which should be today or tomorrow.

My proposed check (in site-master.tcl) looks like this:
set curriculum_bar_p [llength [site_node::get_children -all -filters { package_key "curriculum" } -node_id $subsite_node_id]]
Comments?