Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On 02/06/18 09:01, Joey Pabalinas wrote:
> On Fri, Jun 01, 2018 at 06:56:13PM -0400, Andrew Gregory wrote:
>> Sort of. The current --root option is a confusing mess that nobody
>> actually understands, so it will go away at some point. The
>> underlying libalpm rootdir setting isn't going anywhere though, and,
>> in the future, will be configured with a new --rootdir option.
>
> Well looking at the code it actually isn't as complicated as I thought it
> would be.
>
> How about something like this:
>
>> case OP_OVERWRITE_FILES:
>> {
>> char *i, *root = config->rootdir, *save = NULL;
>> for(i = strtok_r(optarg, ",", ); i; i = strtok_r(NULL,
>> ",", )) {
>> /* strip rootdir if applicable */
>> if (root && !memcmp(i, root, strlen(root)))
>> i += strlen(root);
>> /* strip remaining leading "/" before adding to option
>> list */
>> i += strspn(i, "/");
>> config->overwrite_files =
>> alpm_list_add(config->overwrite_files, strdup(i));
>> }
>> }
>
> which would strip the rootdir and then the leading "/". Although I am not
> 100% certain config->rootdir would be NULL if no argument is passed; could
> you confirm that part?
>
This will not work - we need to handle this after the config file is
read as the root dir may be set there.
A
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On Fri, Jun 01, 2018 at 06:56:13PM -0400, Andrew Gregory wrote:
> Sort of. The current --root option is a confusing mess that nobody
> actually understands, so it will go away at some point. The
> underlying libalpm rootdir setting isn't going anywhere though, and,
> in the future, will be configured with a new --rootdir option.
Well looking at the code it actually isn't as complicated as I thought it
would be.
How about something like this:
> case OP_OVERWRITE_FILES:
> {
> char *i, *root = config->rootdir, *save = NULL;
> for(i = strtok_r(optarg, ",", ); i; i = strtok_r(NULL,
> ",", )) {
> /* strip rootdir if applicable */
> if (root && !memcmp(i, root, strlen(root)))
> i += strlen(root);
> /* strip remaining leading "/" before adding to option
> list */
> i += strspn(i, "/");
> config->overwrite_files =
> alpm_list_add(config->overwrite_files, strdup(i));
> }
> }
which would strip the rootdir and then the leading "/". Although I am not
100% certain config->rootdir would be NULL if no argument is passed; could
you confirm that part?
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On 06/01/18 at 12:44pm, Joey Pabalinas wrote: > On Fri, Jun 01, 2018 at 05:45:11PM -0400, Andrew Gregory wrote: > > Not the sysroot, rootdir. I don't see how that would be overly > > complex, we do it in other places already. > > Isn't --root deprecated in favor of --sysroot though? (according to the > manpage at least). Sort of. The current --root option is a confusing mess that nobody actually understands, so it will go away at some point. The underlying libalpm rootdir setting isn't going anywhere though, and, in the future, will be configured with a new --rootdir option.
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On Fri, Jun 01, 2018 at 05:45:11PM -0400, Andrew Gregory wrote: > Not the sysroot, rootdir. I don't see how that would be overly > complex, we do it in other places already. Isn't --root deprecated in favor of --sysroot though? (according to the manpage at least). -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On 06/01/18 at 11:41am, Joey Pabalinas wrote: > On Fri, Jun 01, 2018 at 05:13:37PM -0400, Andrew Gregory wrote: > > If we're going to allow absolute paths, should we not be removing the > > full root, not just '/'? > > Did you mean strip out the sysroot in the event it happens to not be "/"? In > my opinion, although doable, the logic you'd need to add would make this far > more complex than just stripping out leading "/", and how scarce that > particular > situation is in reality make this not really seem like a worthwhile trade-off > to me. Not the sysroot, rootdir. I don't see how that would be overly complex, we do it in other places already.
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On Fri, Jun 01, 2018 at 05:13:37PM -0400, Andrew Gregory wrote: > If we're going to allow absolute paths, should we not be removing the > full root, not just '/'? Did you mean strip out the sysroot in the event it happens to not be "/"? In my opinion, although doable, the logic you'd need to add would make this far more complex than just stripping out leading "/", and how scarce that particular situation is in reality make this not really seem like a worthwhile trade-off to me. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On 05/29/18 at 02:28pm, Allan McRae wrote:
> The arguments for the --overwrite option requried the user to strip the
> leading "/" from the path. It is more intuative to provide the whole
> path and have pacman strip the leading "/" before passing to the
> backend.
>
> Signed-off-by: Allan McRae
> ---
> src/pacman/pacman.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index fe54793e..d90a9f6c 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -723,7 +723,16 @@ static int parsearg_upgrade(int opt)
> config->flags |= ALPM_TRANS_FLAG_FORCE;
> break;
> case OP_OVERWRITE_FILES:
> - parsearg_util_addlist(&(config->overwrite_files));
> + {
> + char *i, *save = NULL;
> + for(i = strtok_r(optarg, ",", ); i; i =
> strtok_r(NULL, ",", )) {
> + /* strip leading "/" before adding to
> option list */
> + while(i[0] == '/') {
> + i = i + 1;
> + }
> + config->overwrite_files =
> alpm_list_add(config->overwrite_files, strdup(i));
> + }
> + }
> break;
> case OP_ASDEPS:
> config->flags |= ALPM_TRANS_FLAG_ALLDEPS;
> --
> 2.17.0
If we're going to allow absolute paths, should we not be removing the
full root, not just '/'?
Re: [pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
On Tue May 29 04:28:28 UTC 2018, Allan McRae wrote:
> The arguments for the --overwrite option requried the user to strip the
> leading "/" from the path. It is more intuative to provide the whole
> path and have pacman strip the leading "/" before passing to the
> backend.
>
> ...
>
> + while(i[0] == '/') {
> + i = i + 1;
> + }
Eli pointed out that you had already submitted a patch for this
and mine touched a bit too much, and I agree.
I do like your approach more, but I am still of the opinion that
something like:
> i += strspn(i, "/")
would be much simpler (and maintain equivalent semantics) in place of that loop.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
[pacman-dev] [PATCH] Strip leading "/" from arguments to --overwrite
The arguments for the --overwrite option requried the user to strip the
leading "/" from the path. It is more intuative to provide the whole
path and have pacman strip the leading "/" before passing to the
backend.
Signed-off-by: Allan McRae
---
src/pacman/pacman.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index fe54793e..d90a9f6c 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -723,7 +723,16 @@ static int parsearg_upgrade(int opt)
config->flags |= ALPM_TRANS_FLAG_FORCE;
break;
case OP_OVERWRITE_FILES:
- parsearg_util_addlist(&(config->overwrite_files));
+ {
+ char *i, *save = NULL;
+ for(i = strtok_r(optarg, ",", ); i; i =
strtok_r(NULL, ",", )) {
+ /* strip leading "/" before adding to
option list */
+ while(i[0] == '/') {
+ i = i + 1;
+ }
+ config->overwrite_files =
alpm_list_add(config->overwrite_files, strdup(i));
+ }
+ }
break;
case OP_ASDEPS:
config->flags |= ALPM_TRANS_FLAG_ALLDEPS;
--
2.17.0
