bugGNU arch -- a revision control system - Bugs: bug #5573, includes in multi-tree projects

 
 

bug #5573: includes in multi-tree projects

Submitter:  Christian Thaeter <cehteh>
Submitted:  Thu 25 Sep 2003 05:17:25 PM UTC
   
 
Category:  tla Severity:  3 - Normal
Item Group:  small feature idea Status:  None
Privacy:  Public Open/Closed:  Open
Release: 
Fixed Release: 
Merge Request?:  yes -- merge from my archive
Your Archive Name:  chth@gmx.net--cehteh
Your Archive Location:  http://cehteh.homeunix.org/arch/cehteh000
Assigned to:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 15 Oct 2003 09:58:08 PM UTC, comment #9: 

I would like to make an end on this debate.
IMO the current config syntax is flawed because the
'LOCATION   REVISION' is quite freeform and not easy to detect/differentiate from other possible commands.

a better proprosal is some syntax like:
'keyword ARGUMENTS...'
where we can use:
'subproject LOCATION REVISION'
'include CONFIGFILE'

with this we are free to add all kinds of extensions with an uniform syntax in future! It is much more pleasant for a human to read and edit too.

A problem is that it breaks the current syntax. Some might consider this bad, but i dont think that this is a showstopper because the configfiles (and tools which generate them) can be easily upgraded (we can provide --oldcfg switch until tla 1.2 or so, too). It does not break archives, and tools/tla can ignore any line which starts with a keyword it doesnt understand.

Next is that I am currently working on my MaLa extension language which supports the same syntax by default (coincidentally, i swear). That means when MaLa is finshed and integrated in tla, it will get scripting support and the config-files become scripts which control multitree projects.

Christian

Christian Thaeter <cehteh>
Tue 14 Oct 2003 12:17:06 PM UTC, comment #8: 

Ok, a few things have come up in my config-manager experiment that have relevance.
1) ./ as a path in a config is very useful. As a config can be an arbitrary file these days, the use of ./ combined with -d to buildcfg can (if allowed to) pull the top level directory as well.
2) rather than having a single field line for an include, I think that having the include specified as
<path> <include trigger><config to include>, where path is the normal ./foo/bar; include trigger is the ,,include or whatever prefix to indicate that an include should occur, and the config to include is, well duh :}. The included config then has the <path> prefixed to it's paths. This allows nested configs to pull in the recipe for a sub project to an arbitrary subdir - more flexability.

Robert Collins <robertc>
Mon 06 Oct 2003 04:46:42 PM UTC, comment #7: 

Finally:


2003-10-06 16:42:58 GMT Christian Thaeter <chth@gmx.net>        patch-4

    Summary:
      fixed the config files include directive
    Revision:
      tla--devo--1.1--patch-4

    now named '/include/' instead of 'include'
   
    fixed coding style
   
    using an assoc_table now
   
    split into 2 functions (arch_read_config and arch_read_config_internal)

    modified files:
     libarch/cmd-cfgcat.c libarch/configs.c libarch/configs.h



Christian

Christian Thaeter <cehteh>
Mon 06 Oct 2003 12:30:24 AM UTC, comment #6: 

I would suggest using `/include' or `/include/', on the theory that directories in configs should always be relative, and that `/' seems more `obviously special' than `,', especially for someone not entirely familiar with arch's naming conventions (and the slash looks nicer too :-).

Miles Bader <miles>
Sun 05 Oct 2003 06:57:14 PM UTC, comment #5: 

Robert Collins made the suggestion to use ',include' since dirnames with preceeding commas are already junk names and not valid. I will implement that now and hopefully that will satisfiy all people

Christian

Christian Thaeter <cehteh>
Sat 04 Oct 2003 05:51:29 PM UTC, comment #4: 

please don't just say "No, thats bad." without a better suggestion :) ... I considered a lot of syntax ideas but i still dont have a better one.

It would be quite simple to add a "%include" where "%" is some arbitiary choosen character, but this could clash if someone names directories in some weird sense, so it would need some escaping rules .. doh!

"#include" ... as #.. are comments so far .. ranged out, theres very likely someone out there who started a comment with that.

My first idea was to treat a line which start with whitespaces as include line (no keyword required) .. but thats also ranged out, not clear enough.

Further in my opinion the parser should not be forgiving on formatting errors .. that makes it harder for other tools to parse such a files correct, but it must generate very precise error messages when sometihng is wrong, so that the user can fix it easily. Btw: currently it is not very good in that area, tried to postfix a configdir with a "/"? If you dont know that it doenst work you get some serious problem in tracking it down.


Christian

Christian Thaeter <cehteh>
Sat 04 Oct 2003 04:02:43 AM UTC, comment #3: 


> think this would NOT clash because all managed
> directories should be relative to './'. I would
> rather add code which rejects configs without a
> leading './' and trailing '/' for subdir names. I
> thought we agreed on this on IRC?
> changing 'include' to '%include' or something else
> shouldn't make a problem but is still ambiguous, the
> './' prefix for subdirs is the best way imo.


No.  That's bad.

For users, ./ is a no-op; as far as I know, it's part of automatically generated configs only because of an arbitrary implementation detail; to make it mandatory would seem arbitrary and annoying.  Remember, configs are a user-interface too, so arch should be forgiving  in parsing it.

Miles Bader <miles>
Sat 04 Oct 2003 03:29:04 AM UTC, comment #2: 

.. back from hollyday

>Could you please make some cleanups to this?


Sure

> a rel_table or assoc_table will do just as well.


I need to check that out, i'm still not familar enough with libhackerlab.
 

>arch_read_config can be a wrapper
>function that calls arch_read_config_internal (a staticly
>declared function) with extra parameters.


Ok, well but:
Currently it semms to be not a problem if libarch is not threadsafe, i had no time to ask you about future directions, so i made the decision on myself to make it threadsafe there (yes my patch is not very elegant there). Other changes would require at least one additional user supplied argument (assoc_table* ?) to keep the state for the current run. Please tell me ur opinion.
 

> The particular include syntax is an interesting idea but
> won't quite cut it, i'm afraid. The problem is: what if
> I have a subdir named "include"? The syntax you chose
> might break some existing configs, and will lead to
> confusing errors in future configs.


I think this would NOT clash because all managed directories  should be relative to './'. I would rather add code which rejects configs without a leading './' and trailing '/' for subdir names. I thought we agreed on this on IRC?
changing 'include' to '%include' or something else shouldn't make a problem but is still ambiguous, the './' prefix for subdirs is the best way imo.

Would you allow absolute subdir paths? i think thats a bad idea.

> Minor formatting issues....


Yeah, was in a hurry before hollydays, and ususally i dont use that style.



Cheers Christian

Christian Thaeter <cehteh>
Sat 27 Sep 2003 10:27:35 PM UTC, comment #1: 

Could you please make some cleanups to this?

In particular:

There is no need for the type `struct arch_file_list': a
rel_table or assoc_table will do just as well.

The parameters to arch_read_config should not need to
change.   Instead, arch_read_config can be a wrapper
function that calls arch_read_config_internal (a staticly
declared function) with extra parameters.

The particular include syntax is an interesting idea but
won't quite cut it, i'm afraid.   The problem is:  what if
I have a subdir named "include"?   The syntax you chose
might break some existing configs, and will lead to
confusing errors in future configs.

Minor formatting issues.... Please:

put a space after `for' but not after the '('. 

put spaces around '=' in assignments.

put a space between the function name and the '(' in
function calls.  Do not put a space after the '('.

put spaces around the '==' in comparisons -- around all
binary operators in general.

put a blank line between local variable declarations and the
rest of the code in the block

don't put a space before ')' in an `if'


Tom Lord <tomlord>
Group Member
Thu 25 Sep 2003 05:17:25 PM UTC, original submission:  

chth@gmx.net--cehteh/tla--devo--1.1--patch-1
    added 'include' directive to multi-tree config files
chth@gmx.net--cehteh/tla--devo--1.1--patch-2
    fixed tracker and somewhat tested

docs at:
http://cehteh.homeunix.org/pipawiki/HowToSetupArch_2fHowToSetupMultiTreeProjects

cya Christian

Christian Thaeter <cehteh>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

Only logged-in users can vote.

 

Follow 2 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2003-10-12 tomlord Open/ClosedFeedback None
2003-09-27 tomlord Open/ClosedOpen None

Back to the top

Powered by Savane 3.13-d3ae.
Corresponding source code