PERFORCE change 137991 for review

Garrett Cooper yanegomi at gmail.com
Tue Mar 18 09:29:37 UTC 2008


On Mar 18, 2008, at 2:01 AM, Garrett Cooper wrote:

> ==== //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/ 
> pkgman/main.c#8 (text+ko) ====
>
> @@ -21,8 +21,9 @@
>
> 	cmd_str = strdup(*(argv+1));
>
> -	optional_opt_str_p = (char**) malloc(sizeof(char*)+1);
> +	optional_opt_str_p = (char**) malloc(sizeof(char*)*argc+1);
>
> +	/* Scan for the initial command */
> 	if (0 == strcmp(cmd_str, ADD)) {
> 		action_type = _pkg_add;
> 	} else if (0 == strcmp(cmd_str, CLEAN)) {
> @@ -70,17 +71,24 @@
> 		/** Don't error out on unknown / non-globally applied flags, yet..  
> **/
> 		opterr = 0;
>
> +		/*
> +		 * @todo: Move this parse block to an arg parser to simplify /  
> modularize the program.
> +		 *
> +		 * Doing this eliminates caveats like dealing with the origins  
> and search
> +		 * items, as well as return codes..
> +		 */
> +
> 		/** Parse in global arguments **/
> 		while ( (opt_char = getopt(argc, argv, GLOBAL_OPT_STRING)) != -1 ) {
>
> 			switch (opt_char) {
> 			/** Force **/
> 			case 'f':
> -				global_settings->iu_opts |= FORCE_FLAG;
> +				settings->iu_opts |= FORCE_FLAG;
> 				break;
> 			/** Interactive **/
> 			case 'i':
> -				global_settings->iu_opts |= INTERACTIVE_FLAG;
> +				settings->iu_opts |= INTERACTIVE_FLAG;
> 				break;
> 			/** No-exec command **/
> 			case 'o':
> @@ -91,37 +99,39 @@
> 					 * @todo: Add tmp_pkg_origin to STAILQ here
> 					 * with origin names..
> 					 *
> -					 * Does this get freed with pkg_freebsd_pkg_new
> -					 * (or whatever the constructor was..)?
> +					 * Don't worry -- this stuff gets cleaned up
> +					 * from within the relevant freebsd pkg
> +					 * constructor.
> 					 */
> 				}
> 				break;
> 			/** Prefix **/
> 			case 'p':
> -				if (optarg != NULL && strlen(optarg)) {
> -					strdup(global_settings->prefix_path_str,
> +				if (optarg != NULL && 0 < strlen(optarg)) {
> +					strdup(settings->prefix_path_str,
> 						optarg);
> 				} else {
> -					
> +					errx("-%c flag usage invalid: requires argument..\n",
> +						opt_char);
> 				}
> 				break;
> 			/** Quiet **/
> 			case 'q':
> -				if (global_settings->opts & VERBOSE_FLAG) {
> +				if (settings->iu_opts & VERBOSE_FLAG) {
> 					errx(QV_ERR_MSG);
> 				}
> -				global_settings->iu_opts |= QUIET_FLAG;
> +				settings->iu_opts |= QUIET_FLAG;
> 				break;
> 			/** Recursive **/
> 			case 'r':
> -				global_settings->misc_opts |= RECURSIVE_FLAG;
> +				settings->misc_opts |= RECURSIVE_FLAG;
> 				break;
> 			/** Verbose **/
> 			case 'v':
> -				if (global_settings->opts & QUIET_FLAG) {
> +				if (settings->iu_opts & QUIET_FLAG) {
> 					errx(QV_ERR_MSG);
> 				}
> -				global_settings->iu_opts |= VERBOSE_FLAG;
> +				settings->iu_opts |= VERBOSE_FLAG;
> 				break;
> 			}
>
> @@ -146,35 +156,52 @@
> 	 * Parse command specific args, once and once only..
> 	 * See pkg_action_parsers for more details.
> 	 */
> -	for (i = 1; i <= _PKG_LAST_COMMAND; i++) {
> +	for (i = _PKG_FIRST_COMMAND; i <= _PKG_LAST_COMMAND; i++) {
>
> 		/*
> -		 * If arguments usage was invalid, print usage message for  
> specific command
> -		 * and exit..
> +		 * If arguments usage was invalid, print usage message
> +		 * for specific command and exit..
> 		 */
> -		if (0 < parsers[action_type]->parse_args(global_settings,
> +		if (0 < parsers[i]->parse_args(i, settings,
> 			optional_opts_ind+1, optional_opt_str_p)
> 		) {
> -			/** Display the main usage arguments **/
> +			/**
> +			 * Display the main usage arguments, if
> +			 * i != _pkg_null..
> +			 **/
> 			if (action_type != _pkg_null) {
> 				parsers[_pkg_null]->print_use();
> 			}
> -			parsers[action_type]->print_use();
> +			parsers[i]->print_use();
> 			exit(1);
> +
> 		}
>
> 	}
>
> 	/*
> +	 * Generate actions here -- this is where the actual event  
> composition
> +	 * occurs -- eliminate dup strings, i.e.
> +	 *
> +	 * misc/screen misc/screen ... etc
> +	 *
> +	 * This will reduce workload on decomposition below.
> +	 *
> +	 * This is a prime opportunity for btrees to shine, as the data sets
> +	 * here are sufficiently small to warrant a simpler structure, which
> +	 * is faster to compute / get / fetch data with.
> +	 */
> +
> +	/*
> 	 * Grab pkg data strings to perform actions on.
> 	 *
> -	 * pkg data strings include:
> -	 * 	1. Pkg names.
> +	 * "pkg data strings" include:
> +	 * 	1. Pkg names. [is this a special subset of 3?]
> 	 * 	2. URI's.
> 	 * 	3. Origins.
> 	 * 	4. Patterns.
> 	 *
> -	 * Questions to one one's self:
> +	 * Questions to ask one's self:
> 	 * 1. What items do we not need pkg data strings for?
> 	 * 	a. How many of the commands do we take only 1 string for?
> 	 * 	b. How many of the commands do we take 1+ strings for?
> @@ -186,18 +213,82 @@
> 	 */
> 	int action_result;
>
> -	/** Foreach action / pkg, repeat necessary set of steps to reach  
> required conclusion... **/
> -	{
> +	/*
> +	 * Set the counter to the last optional opt index, plus one of  
> course
> +	 * for good measure :) (cause we skip to our Lou over the last  
> optional
> +	 * opt, yar!)
> +	 */
> +	i = optional_opts_ind + 1;
> +
> +	/*
> +	 * The method of attack which needs to be done below is that the
> +	 * elements need to be decomposed from their composed states, i.e.
> +	 * duplicate steps need to be eliminated, pathes which overlap
> +	 * virtually should be converged, fbsd_pkg objects should be created
> +	 * as necessary, etc.
> +	 *
> +	 * This would be a REALLY handy spot for using a virtual BDB hash
> +	 * data structure with file backing, in particular for caching
> +	 * purposes.
> +	 */
> +
> +	/*
> +	 * 'Foreach' pkg data string, repeat necessary set of steps to reach
> +	 * required conclusion...
> +	 *
> +	 * Why a do-while? Cause some commands don't need pkg data strings
> +	 * (the clean and version commands for instance).
> +	 */
> +	do {
> +
> +		/** Foreach composed action.. **/
> +		{
> +
> +			/*
> +			 * Try to create the package object, depending on what the action
> +			 * being performed is
> +			 */
> +			switch (action->type) {
> +			
> +				/*
> +				 * Create a fbsd_pkg object for everything.
> +				 *
> +				 * clean, update, and version are special cases as they
> +				 * don't necessary need arguments (and hence can be "*"
> +				 * or "match all" equivalent)
> +				 *
> +				 */
> +
> +
> +
> +
> +			}
> +
> +			/*
> +			 * Generate hash of some kind for each action. See if the hash
> +			 * already exists in the lookup table.
> +			 */
> +
> +			/** Perform the action, per the package **/
> +			if ( (action_result =
> +				action->perform(settings, optional_opts, i))
> +			!= 0 ) {
> +				warnx("Error encountered when processing "
> +					"command: %s\n",
> +					cmd_str);

	Arg. I accidentally submitted this change (pkgman/main.c). I'm  
filling in more blanks with comments and filling in the ideas, so that  
way I can code everything up in the near future.
	I wrote up an Excel document (tables were easier in Excel), which  
list some RFCs for the CLI. Some are non-POLA friendly RFCs, but I was  
hoping to programmatically and functionally improve how commands and  
flags work within the framework, because everything seems to be mish- 
mashed up right now in pkg_install(1).
-Garrett


More information about the p4-projects mailing list