On 05/03/2025 13:47, Jonathan Wakely wrote: > On Wed, 5 Mar 2025 at 13:37, Richard Earnshaw (lists) > <richard.earns...@arm.com> wrote: >> >> On 05/03/2025 13:10, Jonathan Wakely via Gcc wrote: >>> While onboarding somebody today we noticed an error in the >>> customization script if you use a non-default value for the local >>> prefix. >>> >>> I reproduced it with bash -x to show where it happens. In the output >>> below I entered "jw" as the local prefix, instead of the default "me": >>> >>> + echo 'Setting up tracking for personal namespace redi in remotes/users/jw' >>> Setting up tracking for personal namespace redi in remotes/users/jw >>> + git config remote.users/jw.url git+ssh://r...@gcc.gnu.org/git/gcc.git >>> + '[' x '!=' x ']' >>> + git config --replace-all remote.users/jw.fetch >>> '+refs/users/redi/heads/*:refs/remotes/users/jw/*' >>> refs/users/redi/heads/ >>> + git config --replace-all remote.users/jw.fetch >>> '+refs/users/redi/tags/*:refs/tags/users/jw/*' refs/users/redi/tags/ >>> + git config --replace-all remote.users/jw.push >>> 'refs/heads/jw/*:refs/users/redi/heads/*' refs/users/redi >>> + '[' me '!=' jw -a me '!=' origin ']' >>> + git config --remove-section remote.me >>> fatal: no such section: remote.me >>> + git config --unset-all remote.origin.fetch refs/users/redi/ >>> + git config --unset-all remote.origin.push refs/users/redi/ >>> + git fetch users/jw >>> >>> The script finishes successfully, but because the last line that is >>> printed out is "fatal: no such section: remote.me" it makes it look >>> like it failed and exited. But it's only fatal to the 'git config' >>> sub-process, not the script that the user is actually running. >>> >>> Should we just add a 2>/dev/null redirect to that command, or do we >>> want to check if it exists before trying to remove it? i.e. >>> if git config get --regexp remote.me >/dev/null; then >> >> You'd want to match ^remote.${old_pfx}, for safety. > > Good point! > >> You could also use 'git config --get-regexp', but perhaps that's what you >> mean anyway. > > I think 'get --regexp' works, but it doesn't matter if we just remove > that chunk. >
git config --help only shows the form I mentioned. That doesn't mean yours won't work as well, though. >> >>> git config --remove-section remote.me >>> fi >>> >>> Or should we just remove that part entirely? >>> I doubt anybody used the original version of that script prior to >>> January 2020, and hasn't updated to the new structure yet. See >>> r10-6086-g24b178184f260a which introduced that part. >> >> But this might be easier. I agree, that was very much a transitional bit of >> code as we settled on our new workflows. >> >> I'll trust your decision on this one. > > I'll wait for any other comments, but if nobody disagrees, I think we > should remove it. How much of it is cleaning up the old config, just > this? > > -- a/contrib/gcc-git-customization.sh > +++ b/contrib/gcc-git-customization.sh > @@ -205,12 +205,4 @@ git config --replace-all > "remote.users/${new_pfx}.fetch" "+refs/users/${remote_i > git config --replace-all "remote.users/${new_pfx}.fetch" > "+refs/users/${remote_id}/tags/*:refs/tags/users/${new_pfx}/*" "re > fs/users/${remote_id}/tags/" > git config --replace-all "remote.users/${new_pfx}.push" > "refs/heads/${new_pfx}/*:refs/users/${remote_id}/heads/*" "refs/use > rs/${remote_id}" > > -if [ "$old_pfx" != "$new_pfx" -a "$old_pfx" != "${upstream}" ] > -then > - git config --remove-section "remote.${old_pfx}" > -fi > - > -git config --unset-all "remote.${upstream}.fetch" "refs/users/${remote_id}/" > -git config --unset-all "remote.${upstream}.push" "refs/users/${remote_id}/" > - > git fetch "users/${new_pfx}" > > > While making t hat edit, I noticed that the lines above it have: > > git config --replace-all "remote.users/${new_pfx}.fetch" > "+refs/users/${remote_id}/heads/*:refs/remotes/users/${new_pfx}/*" > "refs/users/${remote_id}/heads/" > git config --replace-all "remote.users/${new_pfx}.fetch" > "+refs/users/${remote_id}/tags/*:refs/tags/users/${new_pfx}/*" > "refs/users/${remote_id}/tags/" > > That doesn't look right, because the second line will replace what was > done by the first. Really? isn't one working on heads and the other on tags? All of this hunk seems related to legacy layouts as well: +# Scan the existing settings to see if there are any we need to rewrite. +vendors=$(git config --get-all "remote.${upstream}.fetch" "refs/vendors/" | sed -r "s:.*refs/vendors/([^/]+)/.*:\1:" | sort | uniq) +url=$(git config --get "remote.${upstream}.url") +pushurl=$(git config --get "remote.${upstream}.pushurl") +for v in $vendors +do + echo "Migrating vendor $v to new remote vendors/$v" + git config --unset-all "remote.${upstream}.fetch" "refs/vendors/$v/" + git config --unset-all "remote.${upstream}.push" "refs/vendors/$v/" + git config "remote.vendors/${v}.url" "${url}" + if [ "x$pushurl" != "x" ] + then + git config "remote.vendors/${v}.pushurl" "${pushurl}" + fi + git config --add "remote.vendors/${v}.fetch" "+refs/vendors/$v/heads/*:refs/remotes/vendors/${v}/*" + git config --add "remote.vendors/${v}.fetch" "+refs/vendors/$v/tags/*:refs/tags/vendors/${v}/*" +done R.