Skip to content

Allow local directory global install with 'bpkg install -g'#170

Open
jwerle wants to merge 3 commits into
masterfrom
allow-local-directory-global-install
Open

Allow local directory global install with 'bpkg install -g'#170
jwerle wants to merge 3 commits into
masterfrom
allow-local-directory-global-install

Conversation

@jwerle
Copy link
Copy Markdown
Member

@jwerle jwerle commented Dec 25, 2023

Fixes #169

This PR also normalizes command strings that may have a leading " or ending "

@thewebmasterp
Copy link
Copy Markdown

thewebmasterp commented Dec 26, 2023

There's another related issue I stumbled upon while playing around with this fix: dependencies.

Say that we have package A depending on B. Both A and B are packages residing in local directories on the system, rather than in remote repositories. So doing:

{
    "name": "script A",
    ...
    "dependencies": {
       "local_dependency_package_B": "master"
    }
}

So how would you specify local_dependency_package_B which would be a path in this case?

@jwerle
Copy link
Copy Markdown
Member Author

jwerle commented Dec 26, 2023

There's another related issue I stumbled upon while playing around with this fix: dependencies.

Say that we have package A depending on B. Both A and B are packages residing in local directories on the system, rather than in remote repositories. So doing:

{
    "name": "script A",
    ...
    "dependencies": {
       "local_dependency_package_B": "master"
    }
}

So how would you specify local_dependency_package_B which would be a path in this case?

We could address this by allowing the branch or version in a dependency to also include a file: protocol to indicate a local file path

@samlikins
Copy link
Copy Markdown
Member

samlikins commented Dec 26, 2023

Firstly, this feature request isn't really related to this MR. This MR is for specifying a local project for the utility at the command line, while this feature request is for specifying in a project file. If this goes any further, turn this conversation into an issue, let's not start tacking feature requests on merge requests...

So I think I'm missing something here in this thought process. A dependency in this situation has to do with defining the source location for installation of required projects of a specific project. You'd not only be installing a list of specified dependency projects, but specified versions of those projects. The way this is being discussed is more like defining how to link projects. That's more of a feature of repositories not projects. In other package management software, you'd update the managers listing of source repositories to include a new local repository.

Repository management is a feature I was working at building out. The BPKG site listing and search is now setup to handle pointing to non-GitHub project repositories. My next feature to add to the utility was the revamp of source handling. This feature is just a natural consequence of that concept. Add a local repository override, that will reduce any resulting complexity.

However, let's stop discussing this feature request on this MR. If we must continue this discussion, create a new issue and link this MR discussion for reference.

Thank you,

Sam

Comment thread lib/install/install.sh
json="$(cat package.json 2>/dev/null)"
else
popd >/dev/null || return 1
bpkg_warn 'Unable to determine bpkg.json'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this else block could be removed, since this error case is covered by the [ -z "$json" ] test below?

Comment thread lib/install/install.sh
return 1
fi

## install bin if needed
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this section be extracted to a new function, along with its analog in bpkg_install_from_remote()?

bpkg/lib/install/install.sh

Lines 388 to 392 in c7f5a46

## install bin if needed
build="$(echo -n "$json" | bpkg-json -b -f='"install"')"
build=${build#\"}
build=${build%\"}
fi

Comment thread lib/run/run.sh
local cmd="$1"
shift

if [ "${cmd:0:1}" = "\"" ]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this code be replaced with something like the quote replacement that was added in a different part of this pr?

https://github.com/bpkg/bpkg/pull/170/files#diff-4ffd36319be99fb7f82206db54bcc36ed2fbc2eb5078a685ab4357eae032fd8dR192

cmd=${cmd#\"}
cmd=${cmd%\"}

Copy link
Copy Markdown
Member

@hyperupcall hyperupcall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy new years! Code looks good to me, I just had a few comments that could simplify a few things

@jwerle
Copy link
Copy Markdown
Member Author

jwerle commented Feb 3, 2024

I have been super busy I am sorry! I will get back to this very soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Can't install a package from a local directory

4 participants