Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(packages.xqm): iterate over configured repos #17

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

duncdrum
Copy link
Contributor

@duncdrum duncdrum commented Oct 5, 2020

no tests in this source repo, so screenshot it is for proof it is working: 59 packages includes public repo +1 test from mine.
Screenshot 2020-10-05 at 17 13 41

i m not sure how the @default flag on repos is supposed to be interpreted i could not generate any visible changes by switching it to false, my code changes iterate over all repos which have the @active flag set to true irrespective of @default

see brobertson/Lace2#124
see #16

@joewiz
Copy link
Member

joewiz commented Oct 5, 2020

Nice! I'm curious: with your PR, what happens if the same package is available from two repos? Are there two entries or just one?

@duncdrum
Copy link
Contributor Author

duncdrum commented Oct 5, 2020

there are two entries, it would be nice if the package-manager would sport separate tabs for each repo, or somehow visually distinguish which copy comes from which source, but that is code in another repo. First the xquery lib needs to actually support muliple repos, which i guess was the intention since otherwise the wrapper element is kind of superfluous.

modules/packages.xqm Outdated Show resolved Hide resolved
duncdrum added a commit to duncdrum/Lace2 that referenced this pull request Oct 6, 2020
@brobertson
Copy link

Exciting! Does this resolve dependencies across all the repositories? I.e. if packageA is in repository1 one and it has a dependency on packageB, which happens to be in repository2, will the package manager now grab packageB from repository2 in order to resolve the dependency?

@line-o
Copy link
Member

line-o commented Oct 8, 2020

@brobertson This is untested. I have the same requirement.
-- edit --
Ah, I see you answered that :)
@duncdrum I was also asked what happens with duplicate packages (packageA being present in both repos).

@duncdrum
Copy link
Contributor Author

duncdrum commented Oct 8, 2020

duplicate packages appear as duplicate entries in the list of packages see my package manager issue linked above.
i haven't tested cross-repo dependency resolution yet, but it should work as everything ends up in one big list again anyway

Copy link
Member

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

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

I still cannot see how this yields into a sequence of URLs...

image

i still can not see how this would yield into a sequence of URLs

@dizzzz
Copy link
Member

dizzzz commented Oct 10, 2020

please could you post the contents of for $pkgs in $packages:repos

@duncdrum
Copy link
Contributor Author

@dizzzz how could it not be a sequence???

the following configuration.xml:

<settings>
    <repositories>
        <repository active="true" default="true">http://exist-db.org/exist/apps/public-repo</repository>
        <repository active="true" default="false">http://heml.ddns.net:8080/exist/apps/public-repo/</repository>
    </repositories>
    <readonly>
        <package>http://exist-db.org/apps/existdb-packageservice</package>
        <package>http://exist-db.org/apps/packagemanager</package>
    </readonly>
</settings>

Screenshot 2020-10-10 at 23 22 54

@duncdrum
Copy link
Contributor Author

@line-o ok cross-repo dependency resolution is not working:

Screenshot 2020-10-10 at 23 45 31

[load]:298) - Retrieving package from https://www.duncdrum.eu/exist/apps/public-repo//modules/find.xql?name=https%3A%2F%2Fwww.duncdrum.eu%2Fapps%2FcrossRepo&processor=5.3.0-SNAPSHOT&version=1.0.0

10 Oct 2020 21:45:36,915 [qtp1497538476-137] ERROR (Deploy.java [installAndDeploy]:215) - Failed to install dependency from https://www.duncdrum.eu/exist/apps/public-repo//modules/find.xql?name=https%3A%2F%2Fwww.duncdrum.eu%2Fapps%2FcrossRepo&processor=5.3.0-SNAPSHOT&version=1.0.0: https://www.duncdrum.eu/public/cross-repo-dep-test-1.0.0.xar

java.io.IOException: Failed to install dependency from https://www.duncdrum.eu/exist/apps/public-repo//modules/find.xql?name=https%3A%2F%2Fwww.duncdrum.eu%2Fapps%2FcrossRepo&processor=5.3.0-SNAPSHOT&version=1.0.0: https://www.duncdrum.eu/public/cross-repo-dep-test-1.0.0.xar

@dizzzz
Copy link
Member

dizzzz commented Oct 11, 2020

Ok, I had no idea that in let $urls := $pkgs || "sometext" the "sometext" is appended to each of the entries in the $pkgs sequence. I can't see it in the spec though:

https://www.w3.org/TR/xpath-30/#id-string-concat-expr

checking the fn:concat() function https://www.w3.org/TR/xpath-functions/#func-concat ... I still do not see how this should work; at it works on xs:anyAtomicType?

[saxon gives the same results]

but see ...

image

@duncdrum
Copy link
Contributor Author

https://www.w3.org/TR/xquery-31/#id-xquery-let-clause

@line-o
Copy link
Member

line-o commented Oct 11, 2020

@duncdrum @dizzzz $url is just one at a time inside the FLOWR
This does change if group by is used, but is not the point here.

@@ -282,10 +283,12 @@ declare function packages:get-package-meta($app as xs:string, $name as xs:string
(: should be private but there seems to be a bug :)
declare function packages:public-repo-contents($installed as element(repo-app)*) {
try {
let $url := $config:DEFAULT-REPO || "/public/apps.xml?version=" || packages:get-version() ||
for $pkgs in $packages:repos
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should rename this to $pkg as this will be an item() from the tuple stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about $pkg-list I really would like to avoid using identical variable names for different things in the library

"&amp;source=" || util:system-property("product-source")
(: EXPath client module does not work properly. No idea why. :)
let $request :=
let $request := for $url in $urls
Copy link
Contributor

Choose a reason for hiding this comment

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

This for doesn't make any sense to me. As a $pkg (or $pkgs) can only be an item() not a sequence. Therefore I think this for is redundant...

Copy link
Contributor Author

@duncdrum duncdrum Oct 12, 2020

Choose a reason for hiding this comment

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

It is not redundant, without it the sequence of all packages is looked up in both repos, with 1 cannot find package error message for each package from repo a not present in repo b, and vice versa.

The for loop sends the request to the endpoint it should go to, the repo that contains the packages in question.

@duncdrum duncdrum marked this pull request as draft January 12, 2021 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants