Clone URL on repository landing page not functioning as expected

Issue #336 resolved
Brandon Jones
created an issue

On the landing page for a repository in Kallithea I noticed a few things about the Clone URL.

First, it is showing http, when my instance is hosted with a https URL.

Second, there is no underscore before the actual repo ID (should be /_1780 not /1780)

Third, the show by name/show by id button does not appear to have any effect.

Comments (13)

  1. Brandon Jones reporter

    So the first issue is irrelevant - I am behind a reverse proxy. I was unaware that the clone_uri is configurable from settings.

    However, I think there is a small issue with the code below (summary.py)

            _def_clone_uri = _def_clone_uri_by_id = c.clone_uri_tmpl
            if '{repo}' in _def_clone_uri:
                _def_clone_uri_by_id = _def_clone_uri.replace('{repo}', '_{repoid}')
            elif '{repoid}' in _def_clone_uri:
                _def_clone_uri_by_id = _def_clone_uri.replace('_{repoid}', '{repo}')
    

    It is checking for '{repoid}' but then replacing '_{repoid}', which may not be found. In my case, the template was defaulted to /{repoid} with no underscore before it.

  2. Thomas De Schampheleire

    Yes, I tried reproducing based on a default installation, and failed.

    I don't really understand the 'elif' branch in the code you pasted. It originates from the original RhodeCode codebase. I also don't really see why this template needs to be configurable. The code seems to assume/require the leading underscore for repoid, but does not actually enforce it. Is it to differentiate a repo called '123' from the repo with id '123' ?

    You mentioned "In my case, the template was defaulted to {repoid} with no underscore before it.". What exactly do you mean? I think you must have changed it manually, no? And what was your intention / expectation for changing it?

    @Mads Kiilerich Do you know more about this?

  3. Brandon Jones reporter

    I upgraded from 0.3.2, and did not modify my settings during the upgrade, and it did not have the underscore before it in the settings. So, assuming that the Clone URI existed in 0.3.2, then no, I did not modify that. I would assume that the default value is what I saw, but I could be wrong. Either way that side of things is working now on my end.

  4. Thomas De Schampheleire

    The value of 'DEFAULT_CLONE_URI' and 'DEFAULT_CLONE_URI_BY_ID' has not changed between these two releases (and not in the history of Kallithea) but I will test the same upgrade from 0.3.2 to 0.4.0rc1 and report back.

    Is there anything else of the issues you reported in this ticket that remains? At my side, the button does have effect and toggles between displaying the repo path by name and by id.

  5. Thomas De Schampheleire

    I checked, in Kallithea 0.3.2, starting from an empty database and populating it with 'paster setup-db my.ini', the clone URI is set to the same value as the default in Kallithea 0.4.0rc1: {scheme}://{user}@{netloc}/{repo}.

  6. Brandon Jones reporter

    It must have been carryover then, it's just strange because I know for a fact that there was an underscore there before, but when I upgraded the underscore went away.

    Perhaps in 0.3.2 {repoid} resolved to _1780, and in 0.4.0rc1 {repoid} resolves to just 1780?

  7. Thomas De Schampheleire

    Strange indeed. {repoid} is replaced by the numeric ID in both cases, there is no underscore in that part (see kallithea/model/db.py, function 'clone_url' ).

    If you can reproduce this scenario, perhaps in a test scenario, please let me know.

    Does the button correctly toggle at your end? If not, are you sure you executed 'kallithea-cli front-end-build' ?

  8. Mads Kiilerich

    I guess the code was supposed to allow configuration in one way and let it be used the other way, so it should look like

            _def_clone_uri = _def_clone_uri_by_id = c.clone_uri_tmpl
            if '{repo}' in _def_clone_uri_by_id:
                _def_clone_uri_by_id = _def_clone_uri_by_id.replace('{repo}', '_{repoid}')
            elif '_{repoid}' in _def_clone_uri:
                _def_clone_uri = _def_clone_uri.replace('_{repoid}', '{repo}')
    

    Does that make sense?

  9. Brandon Jones reporter

    Yes that makes sense.

    Since the button is toggling successfully for me now when formatted properly, I suppose the only change here is the one you proposed adding the underscore in front of {repoid}

  10. Thomas De Schampheleire

    But, if someone has set the default clone uri based on {repoid} without leading underscore, then there will not be a valid clone uri by name, right? I think that there may need to be a second replacement of {repoid} by {repo} if the first replacement did not succeed. Or alternatively, enforce the leading underscore.

  11. Mads Kiilerich

    I'm confused about what actual problem we are trying to solve. What was the old Admin » Settings » Visual » Clone URL? Did it really work in 0.3.2, both for name and id URLs?

    I guess it was something like {scheme}://{user}@{netloc}/{repoid} ... and that is wrong; there should be a _ before {repoid}. But in either case, the name URLs must have been broken? It is much better to just leave it at the default {scheme}://{user}@{netloc}/{repo}. Or is there a special use case for specifying repoid?

    The handling of these repo id URLs is very hardcoded in a odd call chain that ends in utils.py get_repo_by_id . I doubt there are good use cases for specifying repoid explicitly. We should probably just simplify the code.

  12. Log in to comment