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

CP-43777: Add xapi-expiry-alerts library #647

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

gangj
Copy link
Contributor

@gangj gangj commented Jul 3, 2023

No description provided.

"xapi-types"
"xapi-stdext-date"
]
synopsis: "Library required by xapi"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs a better description.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also run opam lint --normalise opam

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, updated

@psafont
Copy link
Member

psafont commented Jul 3, 2023

This is awkward, adding this library will break until the library is in xen-api's master branch.

I recommend creating a PR against xen-api's master branch with
xapi-project/xen-api#5060 and xapi-project/xen-api#5085 (with the commits cleaned up).

After that we can merge this to make the CI happy and be able to use the library in the proprietary daemons

@gangj
Copy link
Contributor Author

gangj commented Jul 4, 2023

@psafont
Copy link
Member

psafont commented Jul 4, 2023

Please squash all three commits

@gangj gangj requested a review from lindig July 5, 2023 01:52
@gangj
Copy link
Contributor Author

gangj commented Jul 5, 2023

Please squash all three commits

Squashed, please help to review again, thank you~

["dune" "build" "-p" name "-j" jobs]
["dune" "runtest" "-p" name "-j" jobs] {with-test}
]
dev-repo: "git+https://github.com/xapi-project/xen-api.git"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This URL is not recognised or supported by Git - try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I did not find related spec for the dev-repo field, seems it is a string for information purpose only?
https://opam.ocaml.org/doc/Manual.html#File-hierarchies :

dev-repo: <string>: the URL of the package's source repository, which may be useful for developers

This URL is copied from other opam files:

[gangj@xenrt1015818053 xs-opam]$ git grep -wn "git+https://github.com/xapi-project/xen-api.git" packages/xs-extra/
packages/xs-extra/gzip.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/http-lib.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/pciutil.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/rrd-transport.master/opam:5:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/rrd2csv.master/opam:8:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/rrddump.master/opam:11:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/safe-resources.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/sexpr.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/stunnel.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/uuid.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/varstored-guard.master/opam:5:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/vhd-format-lwt.master/opam:35:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/vhd-format.master/opam:30:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/vhd-tool.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/wsproxy.master/opam:7:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-cli-protocol.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-client.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-compression.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-consts.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-datamodel.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-expiry-alerts.master/opam:26:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-log.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-nbd.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-networkd.master/opam:5:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-open-uri.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-rrd-transport-utils.master/opam:5:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-rrd-transport.master/opam:5:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-rrdd.master/opam:38:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-schema.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-storage-cli.master/opam:8:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-tracing.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-types.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-xenopsd-cli.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-xenopsd-simulator.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-xenopsd-xc.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi-xenopsd.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xapi.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xe.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xen-api-client-async.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xen-api-client-lwt.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xen-api-client.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xen-api-sdk.master/opam:9:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/xml-light2.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"
packages/xs-extra/zstd.master/opam:6:dev-repo: "git+https://github.com/xapi-project/xen-api.git"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Use a URL that works with git. Maybe it worked in the past.

Copy link
Member

Choose a reason for hiding this comment

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

I believe this an opam-specific uri, Python's PIP uses similar URIs to denote git repositories that have to be fetched using HTTPS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find a commit which added the "git+" to the dev-repo to use Opam 2.0:

commit 1802d44dd62aa437ee87f3686394c6c2a821c848
Author: Christian Lindig <christian.lindig@citrix.com>
Date:   Tue Oct 2 14:16:35 2018 +0100

    Update meta data to Opam 2 format
    
    Signed-off-by: Christian Lindig <christian.lindig@citrix.com>

diff --git a/packages/xs-extra/gzip.master/opam b/packages/xs-extra/gzip.master/opam
index a4d07e90..cdc7bf0f 100644
--- a/packages/xs-extra/gzip.master/opam
+++ b/packages/xs-extra/gzip.master/opam
@@ -1,16 +1,24 @@
-opam-version: "1.2"
+opam-version: "2.0"
 maintainer: "xen-api@lists.xen.org"
 authors: "xen-api@lists.xen.org"
 homepage: "https://xapi-project.github.io/"
 bug-reports: "https://github.com/xapi-project/xen-api-libs-transitional.git"
-dev-repo: "https://github.com/xapi-project/xen-api-libs-transitional.git"
-
+dev-repo: "git+https://github.com/xapi-project/xen-api-libs-transitional.git"
 build: [[ "jbuilder" "build" "-p" name "-j" jobs ]]
...

Maybe we need to add "git+" for those which missed the "git+" prefix? Like this one:

[gangj@xenrt1015818053 xs-opam]$ cat packages/xs-extra/forkexec.master/opam
opam-version: "2.0"
maintainer: "xen-api@lists.xen.org"
authors: "xen-api@lists.xen.org"
homepage: "https://github.com/xapi-project/"
bug-reports: "https://github.com/xapi-project/xen-api/issues"
dev-repo: "https://github.com/xapi-project/xen-api.git"
tags: [ "org:xapi-project" ]

build: [[ "dune" "build" "-p" name "-j" jobs ]]

depends: [
  "ocaml"
  "dune"
  "base-threads"
  "fd-send-recv"
  "ppx_deriving_rpc"
  "rpclib"
  "uuid"
  "xapi-log"
  "xapi-stdext-pervasives"
  "xapi-stdext-unix"
]
synopsis: "Sub-process control service for xapi"
description:
  "This daemon creates and manages sub-processes on behalf of xapi."
url {
  src: "https://github.com/xapi-project/xen-api/archive/master.tar.gz"
}
[gangj@xenrt1015818053 xs-opam]$ 

Signed-off-by: Gang Ji <gang.ji@citrix.com>
@@ -51,6 +51,7 @@ depends: [
"xapi-cli-protocol"
"xapi-consts"
"xapi-datamodel"
"xapi-expiry-alerts"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@psafont Thank you for helping to add these missing part!
May I have a question: is this opam used for building the xapi binary: https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/dune#L162 ?
And may I know when will the xapi binary be requested through the opam repo?

Copy link
Member

Choose a reason for hiding this comment

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

yes, the main xapi binary is part of the xapi opam package. The certificate alert binary is also part of the xapi opam package:
https://github.com/xapi-project/xen-api/blob/master/ocaml/alerts/certificate/dune#L18

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, then for the xapi opam package, who will request it from the opam repo?
For those libraries, like this new added "xapi-expiry-alerts", I think it will be requested from the opam repo to set up the dev env(inside or outside of koji env), while for the xapi opam package, who is its consumer? I greped for "package xapi)" in xen-api repo, seems the xapi package contains only some exe binaries.

Copy link
Member

Choose a reason for hiding this comment

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

the opam repository is only used to test packages in github's CI and test install packages in the developers' machines. To change the binaries installed in Xenserver xapi.spec must be modified to include the files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, thank you!

@psafont psafont merged commit 17e167e into xapi-project:master Jul 5, 2023
2 checks passed
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.

None yet

3 participants