| Message ID | 20190103204026.23512-9-patrickdepinguin@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | support/download: fix scp and reintroduce source-check | expand |
Hello Thomas, On Thu, 3 Jan 2019 21:40:23 +0100, Thomas De Schampheleire wrote: > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > The implementation is the same as originally was present. > It suffers from the disadvantage that an invalid revision on a valid URL > will not be detected. > > However, git does not seem to allow a good way to remotely check the > validity of a revision, without cloning the repository. > > For source-check, we don't want to do such download which can be large. While I understand the limitation, I don't really agree with the conclusion: we should go ahead and download the full thing. Indeed the selling argument for source-check in your cover letter is precisely to verify that the version of the package that has been committed by someone is *really* available. If there's no version check in the git, bzr and cvs source-check implementation, it makes the selling point of the cover letter a bit moot, no? Of course, I realize that your primary interest is in hg, and hg has this capability. But still we should ensure git/bzr/cvs provide the same guarantees, by falling back to the slower but working method of downloading everything. Best regards, Thomas
Thomas, DS, Thomas P, All, On 2019-01-03 21:59 +0100, Thomas Petazzoni spake thusly: > On Thu, 3 Jan 2019 21:40:23 +0100, Thomas De Schampheleire wrote: > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > > > The implementation is the same as originally was present. > > It suffers from the disadvantage that an invalid revision on a valid URL > > will not be detected. > > > > However, git does not seem to allow a good way to remotely check the > > validity of a revision, without cloning the repository. > > > > For source-check, we don't want to do such download which can be large. > > While I understand the limitation, I don't really agree with the > conclusion: we should go ahead and download the full thing. Indeed the > selling argument for source-check in your cover letter is precisely to > verify that the version of the package that has been committed by > someone is *really* available. If there's no version check in the git, > bzr and cvs source-check implementation, it makes the selling point of > the cover letter a bit moot, no? Agreed here. *If* we were to re-add support for source-check, it should really behave as it says on the can: check that the source really exists. If a backend can't do a cheap check, then it has to download everything. > Of course, I realize that your primary interest is in hg, and hg has > this capability. But still we should ensure git/bzr/cvs provide the same > guarantees, by falling back to the slower but working method of > downloading everything. Yup. But this is not an endorsement of re-adding source-check, mind you1 ;-) I'm still unconvinced we need it. Regards, Yann E. MORIN.
El jue., 3 ene. 2019 a las 22:00, Thomas Petazzoni (<thomas.petazzoni@bootlin.com>) escribió: > > Hello Thomas, > > On Thu, 3 Jan 2019 21:40:23 +0100, Thomas De Schampheleire wrote: > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > > > The implementation is the same as originally was present. > > It suffers from the disadvantage that an invalid revision on a valid URL > > will not be detected. > > > > However, git does not seem to allow a good way to remotely check the > > validity of a revision, without cloning the repository. > > > > For source-check, we don't want to do such download which can be large. > > While I understand the limitation, I don't really agree with the > conclusion: we should go ahead and download the full thing. Indeed the > selling argument for source-check in your cover letter is precisely to > verify that the version of the package that has been committed by > someone is *really* available. If there's no version check in the git, > bzr and cvs source-check implementation, it makes the selling point of > the cover letter a bit moot, no? > > Of course, I realize that your primary interest is in hg, and hg has > this capability. But still we should ensure git/bzr/cvs provide the same > guarantees, by falling back to the slower but working method of > downloading everything. Yes, I can agree with this. If you accept adding source-check in the first place, then I'll improve these backends. /Thomas
diff --git a/support/download/git b/support/download/git index 17ca04eb98..54f3494c61 100755 --- a/support/download/git +++ b/support/download/git @@ -7,6 +7,7 @@ set -E # # Options: # -q Be quiet. +# -C Only check that the changeset exists in the remote repository # -r Clone and archive sub-modules. # -o FILE Generate archive in FILE. # -u URI Clone from repository at URI. @@ -48,6 +49,7 @@ recurse=0 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do case "${OPT}" in q) verbose=-q; exec >/dev/null;; + C) checkonly=1;; r) recurse=1;; o) output="${OPTARG}";; u) uri="${OPTARG}";; @@ -61,6 +63,20 @@ done shift $((OPTIND-1)) # Get rid of our options +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_git() { + eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}" +} + +if [ -n "${checkonly}" ]; then + # TODO this check only checks that the remote repository exists, not that + # it actually contains the requested revision. And for checkonly, we want + # to avoid creating a clone first. + _git ls-remote --heads "${@}" "'${uri}'" > /dev/null + exit ${?} +fi + # Create and cd into the directory that will contain the local git cache git_cache="${dl_dir}/git" mkdir -p "${git_cache}" @@ -69,12 +85,6 @@ pushd "${git_cache}" >/dev/null # Any error now should try to recover trap _on_error ERR -# Caller needs to single-quote its arguments to prevent them from -# being expanded a second time (in case there are spaces in them) -_git() { - eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}" -} - # Create a warning file, that the user should not use the git cache. # It's ours. Our precious. cat <<-_EOF_ >"${dl_dir}/git.readme"