Skip to content
New issue

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

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

Already on GitHub? # to your account

Merge stdext and reverse deps into XAPI #5378

Merged
merged 506 commits into from
Jan 31, 2024

Conversation

edwintorok
Copy link
Contributor

Merges as git subtree:

  • xapi-inventory
  • xapi-rrd
  • xapi-stdext*

Opening PR to see whether the CI works.

mseri and others added 30 commits March 15, 2018 15:12
Make safe-string compliant in a backward-compatible way
…, std, threads, unix} 1.1.0

Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
Move quicktest_encodings from xapi into a unit test
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
 really_write: remove deprecation and make robust against EINTR
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
unixext: update interface to mimick the ocaml Unix one (fd-send-recv >= 2.0.0)
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
`Backtrace.is_important` was not called.
We have a `finally` function that does the proper thing wrt to
backtraces, use it.

xapi-stdext-pervasives is a dependency of xapi-stdext-threads already,
so this does not introduce a new dependency.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
We've got a `with_file` in this same file, use it instead of
reimplementing it, so we get the improved backtraces.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
We cannot use finally here, because the socket is only closed on
failure.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
CP-28365: improve backtraces by using finally
…nal one with it

Use the Logs library to report the cleanup exception we caught. This
way, we don't have to depend on xcp-idl. In xcp-idl, a Logs reporter
will be registered, which collects the log messages from Logs and
reports them in the usual way.

Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
CA-292641: Use Logs to log cleanup exn instead of shadowing the origi…
Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
Optionally use rpclib if it's available
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
…a7c07e997c7'

Also move *.opam files to root, update dune install/uninstall rules and quality gate.

git-subtree-dir: ocaml/libs/xapi-rrd
git-subtree-mainline: da5059d
git-subtree-split: 7e7e838
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
…merge-xapi-rrd' and 'private/edvint/merge-inventory'
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
…f1a8522c8e6399'

git-subtree-dir: ocaml/libs/xapi-stdext
git-subtree-mainline: c0ed37e
git-subtree-split: 4cf5cbe
This was referenced Jan 22, 2024
@edwintorok
Copy link
Contributor Author

edwintorok commented Jan 22, 2024

#5383 and #5382 as separate draft PRs if you want to see how xapi-inventory and xapi-rrd got merged.
I tried to keep the merge commits buildable by using 'git commit -a --amend' when I did build fixes like moving opam files around or updating Makefiles/quality gates.
Also includes #5381

@edwintorok
Copy link
Contributor Author

Is it worth it, wouldn't it be easier to drop the dune definiton of the packages of the stdext libs?

I found another way of doing it (by using .opam.template files). In general I'd like to move to generating files from dune-project, because then we can also use tools like 'opam dune lint' that checks the consistency of dependencies between opam files and dune files, and we can also share some metadata (e.g. consistent homepage, build rules and bugreport urls). And it'll also encourage us to have fewer opam files, we have way too many now....

@edwintorok edwintorok force-pushed the private/edvint/merge-stdext2 branch from 68abce8 to dd1bc5e Compare January 22, 2024 15:57
Otherwise it fails because it doesn't about the 'xapi-stdext' packge
(it is not part of xs-opam).

But ocamlformat shouldn't need depexts anyway, so disable them.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
@psafont
Copy link
Member

psafont commented Jan 22, 2024

And it'll also encourage us to have fewer opam files, we have way too many now....

I agree, also at some point dune should support stashing them away in the opam subdir

@edwintorok edwintorok marked this pull request as ready for review January 22, 2024 16:29
@edwintorok
Copy link
Contributor Author

Tests are passing now, and I've also done a successful koji build.
(Haven't tried a chainbuild after removing the packages from xs-opam yet).

@psafont
Copy link
Member

psafont commented Jan 23, 2024

Can the unneeded files in ocaml/libs/xapi-stdext/, ocaml/libs/xapi-inventory/ and ocaml/libs/xapi-rrd/? (like the github workflows or gitignore) be removed

Drop redundant:
* .github
* .ocamlformat
* .travis.yml
* INSTALL
* MAINTAINERS
* .gitattributes
* .gitarchive-info
* Makefile

For consistency I've removed these from other subprojects too, not just recently merged ones.

The only one I didn't touch was vhd-tool, which we may intend to upstream again at some point.

I've kept the README/LICENSE and Changelog files.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
@edwintorok
Copy link
Contributor Author

I've cleaned up the merged subprojects, I've kept .gitignore/ChangeLog/LICENSE/README:

ocaml/libs/xapi-inventory/:
.  ..  .gitignore  ChangeLog  LICENSE  README.md  lib

ocaml/libs/xapi-rrd:
.  ..  .gitignore  ChangeLog  LICENSE  README.md  lib  lib_test  unix

ocaml/libs/xapi-stdext:
.  ..  .gitignore  CHANGES.md  LICENSE  README.md  lib

@psafont
Copy link
Member

psafont commented Jan 30, 2024

I'd like to push this forward, but after the fixed release is done.

Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@cloud.com>
@psafont psafont force-pushed the private/edvint/merge-stdext2 branch from 4bd7423 to 478ec07 Compare January 31, 2024 10:07
Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4ea474b) 45.38% compared to head (478ec07) 45.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5378   +/-   ##
=======================================
  Coverage   45.38%   45.38%           
=======================================
  Files          18       18           
  Lines        2937     2937           
=======================================
  Hits         1333     1333           
  Misses       1604     1604           
Flag Coverage Δ
python2.7 52.09% <ø> (ø)
python3.11 50.15% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@psafont
Copy link
Member

psafont commented Jan 31, 2024

Overrriding the DCO check, all commits are from Xenserver / Citrix employees at the time of committing

@psafont
Copy link
Member

psafont commented Jan 31, 2024

I've checked that the tests in the new packages have been run

@psafont psafont merged commit eff32e4 into xapi-project:master Jan 31, 2024
8 checks passed
Copy link

github-actions bot commented Jan 31, 2024

pytype_reporter extracted 49 problem reports from pytype output.

You can check the results of the job here

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants