DAOS-14410 Update to 2.0 #33

janekmi · 2023-09-19T06:51:31Z

No description provided.

Signed-off-by: Jan Michalski <[email protected]>

brianjmurrell

Wouldn't you want to disable the exclusion of ndctl in this PR similar to what you are doing in #32?

brianjmurrell · 2023-09-19T11:36:21Z

debian/changelog

@@ -1,3 +1,9 @@
+pmdk (2.0.0-1) stable; urgency=medium
+
+  * Upgrade to 2.0.0


Please expand on the resulting packaging differences this release makes (i.e. the changes to debian/control and associated files) and why those are needed (i.e. the changes in pmdk-2.0.0 that are causing them).

brianjmurrell · 2023-09-19T11:37:59Z

pmdk.spec

@@ -652,6 +510,9 @@ cp utils/pmdk.magic %{buildroot}%{_datadir}/pmdk/


 %changelog
+* Tue Sep 19 2023 Jan Michalski <[email protected]> - 2.0.0-1
+- Update to release 2.0.0


Please expand on the resulting packaging differences this release makes (i.e. the changes to the subpackages) and why those are needed (i.e. the changes in pmdk-2.0.0 that are causing them).

grom72 · 2023-09-19T17:33:43Z

Wouldn't you want to disable the exclusion of ndctl in this PR similar to what you are doing in #32?

@brianjmurrell as I see from the initial builds of #32, it will be easier to update first to 2.0.0 and then to enable back the ndctl.

janekmi · 2023-09-20T08:01:50Z

Wouldn't you want to disable the exclusion of ndctl in this PR similar to what you are doing in #32?

@brianjmurrell as I see from the initial builds of #32, it will be easier to update first to 2.0.0 and then to enable back the ndctl.

Agreed. Moreover, the update to 2.0 should be relatively easy whereas reinstantiating the NDCTL use has already proven to be challenging. On top of that, I think doing two big changes at once is too risky. 😉

…gelogs Signed-off-by: Jan Michalski <[email protected]>

pmdk.spec

Signed-off-by: Jan Michalski <[email protected]>

brianjmurrell · 2023-09-25T13:05:09Z

pmdk.spec

-BuildRequires:  glibc-devel
-BuildRequires:  autoconf
-BuildRequires:  automake


Are these no longer needed to build pmdk? Did 2.0.0 change configure/build systems? https://github.com/pmem/pmdk/blob/ba92d6b469d52d16f26279bebaf317bbdbb3822c/INSTALL.md?plain=1#L12-L14 seems to indicate that autoconf is still used.

As for glibc-devel, does pmdk really not use glibc at all?

Removing glibc-devel is one step too far. 😁

But autoconf and automake are not used. Sadly, INSTALL.md requires cleanup.

brianjmurrell · 2023-09-25T13:10:10Z

pmdk.spec

+    NORPATH=1 \
+    BUILD_EXAMPLES=n \
+    BUILD_BENCHMARKS=n


Given that these are common and being repeated 3 times (and as you can see, having to be updated in 3 different places) it might be good to have these defined in a macro once and then referenced in the 3 places they are needed. Then any future updates of these will only be in the one place.

brianjmurrell · 2023-09-25T13:11:28Z

pmdk.spec

+    - removes all pmem2_async operations (and the miniasync dependency).
+- Remove BUILD_RPMEM which was removed in release 1.13 (no change to
+  the resulting packaging).
+- Remove deprecated build requirements.


Is this the removal of the autoconf (and friends) BuildRequires:? It's not clear to me that this should be done. Pending your answer on the previous question.

Yes. This is this little cleanup in BuildRequires. To be clear I just added the list of removed dependencies.

brianjmurrell · 2023-09-26T12:49:56Z

pmdk.spec

 %if %{without ndctl}
-    NDCTL_ENABLE=n \
+    NDCTL_ENABLE=n
 %endif


This could be processed as part of make_common_args to further DRY this out.

brianjmurrell · 2023-09-26T12:52:09Z

What is the intention of #32 in relation to this PR?

janekmi · 2023-09-26T14:56:01Z

What is the intention of #32 in relation to this PR?

This PR is about bumping up the PMDK version to the latest 2.0 release. #32 attempts to restore usage of NDCTL which is critical for PMDK to assure correctness of data read from PMem.

Both of these are required and orthogonal. #32 has to happen in some form no matter whether we decide to upgrade PMDK or not. Upgrading PMDK is not required to re-enable NDCTL. I think why we need to upgrade PMDK is self-explanatory and unrelated to the NDCTL issue.

I do not foresee technical difficulties in landing this PR. Whereas #32 requires solving some technical difficulties as you may recall from DAOS-4183.

Signed-off-by: Jan Michalski <[email protected]>

brianjmurrell · 2023-09-26T17:10:40Z

I do not foresee technical difficulties in landing this PR. Whereas #32 requires solving some technical difficulties as you may recall from DAOS-4183.

But really, if you land this one before #32, you will need to rebase #32 on master again to pick this one up, yes?

If #32 is intended to land first though, should the BuildRequires: you removed here be removed there also? Perhaps there is duplicate make args that can be consolidated as you have done here also?

pmdk.spec

brianjmurrell · 2023-09-26T17:20:17Z

Seems to be failing testing though still. Is this expected?

Signed-off-by: Jan Michalski <[email protected]>

janekmi · 2023-09-26T21:05:02Z

But really, if you land this one before #32, you will need to rebase #32 on master again to pick this one up, yes?

It is true. But I bet @grom72 doesn't mind. Probably #32 should be marked as a draft just not to bother you before it is even ready to be reviewed, am I right @grom72? And this is probably where the confusion comes from. I am not sure what the practice should be in this case.

If #32 is intended to land first though, ...

It is highly improbable. #32 requires first fixes to daos-stack/daos which are far from being ready.

janekmi · 2023-09-28T07:45:37Z

Seems to be failing testing though still. Is this expected?

A funny thing. The latest build combined with this replay constitutes one successful run. @brianjmurrell is it okay or I should further investigate?

brianjmurrell · 2023-10-02T14:03:02Z

Please add @daos-stack/release-engineering as a reviewer if/when you are ready to land this and have this version of PMDK be what is used on master, release/2.4 and release/2.2.

janekmi requested review from brianjmurrell and grom72 September 19, 2023 06:51

janekmi requested a review from a team as a code owner September 19, 2023 06:51

janekmi force-pushed the janekmi__update_2.0 branch from 8957b2d to 9f9408e Compare September 19, 2023 06:56

janekmi changed the title ~~Update to 2.0~~ DAOS-623 Update to 2.0 Sep 19, 2023

DAOS-623 Update to 2.0

9f9408e

Signed-off-by: Jan Michalski <[email protected]>

brianjmurrell requested changes Sep 19, 2023

View reviewed changes

grom72 changed the title ~~DAOS-623 Update to 2.0~~ DAOS-14410 Update to 2.0 Sep 19, 2023

Expand on changes and how they affect the resulting packaging in chan…

1b73abb

…gelogs Signed-off-by: Jan Michalski <[email protected]>

grom72 requested changes Sep 20, 2023

View reviewed changes

pmdk.spec Show resolved Hide resolved

grom72 approved these changes Sep 21, 2023

View reviewed changes

grom72 previously approved these changes Sep 21, 2023

View reviewed changes

Further cleanups

3e358c4

Signed-off-by: Jan Michalski <[email protected]>

janekmi dismissed grom72’s stale review via 7f5d307 September 22, 2023 12:08

Add perl to requirements

7f5d307

Signed-off-by: Jan Michalski <[email protected]>

janekmi requested a review from brianjmurrell September 25, 2023 08:37

brianjmurrell requested changes Sep 25, 2023

View reviewed changes

brianjmurrell reviewed Sep 26, 2023

View reviewed changes

Restore glibc-devel and introduce make_common_argsb

9c7986f

Signed-off-by: Jan Michalski <[email protected]>

brianjmurrell reviewed Sep 26, 2023

View reviewed changes

pmdk.spec Show resolved Hide resolved

Combine all NDCTL_ENABLE=n appending instances

d8796ce

Signed-off-by: Jan Michalski <[email protected]>

janekmi requested a review from brianjmurrell September 29, 2023 07:46

brianjmurrell approved these changes Oct 2, 2023

View reviewed changes

grom72 approved these changes Oct 2, 2023

View reviewed changes

janekmi requested a review from a team October 2, 2023 15:48

brianjmurrell merged commit d741e9d into master Oct 3, 2023
1 of 2 checks passed

brianjmurrell deleted the janekmi__update_2.0 branch October 3, 2023 17:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-14410 Update to 2.0 #33

DAOS-14410 Update to 2.0 #33

janekmi commented Sep 19, 2023

brianjmurrell left a comment

brianjmurrell Sep 19, 2023

janekmi Sep 20, 2023

brianjmurrell Sep 19, 2023

janekmi Sep 20, 2023

grom72 commented Sep 19, 2023

janekmi commented Sep 20, 2023

brianjmurrell Sep 25, 2023

janekmi Sep 26, 2023

brianjmurrell Sep 25, 2023

janekmi Sep 26, 2023

brianjmurrell Sep 25, 2023

janekmi Sep 26, 2023

brianjmurrell Sep 26, 2023

janekmi Sep 26, 2023

brianjmurrell commented Sep 26, 2023

janekmi commented Sep 26, 2023

brianjmurrell commented Sep 26, 2023

brianjmurrell commented Sep 26, 2023

janekmi commented Sep 26, 2023 •

edited

Loading

janekmi commented Sep 28, 2023

brianjmurrell commented Oct 2, 2023

		@@ -1,3 +1,9 @@
		pmdk (2.0.0-1) stable; urgency=medium

		* Upgrade to 2.0.0

DAOS-14410 Update to 2.0 #33

DAOS-14410 Update to 2.0 #33

Conversation

janekmi commented Sep 19, 2023

brianjmurrell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grom72 commented Sep 19, 2023

janekmi commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianjmurrell commented Sep 26, 2023

janekmi commented Sep 26, 2023

brianjmurrell commented Sep 26, 2023

brianjmurrell commented Sep 26, 2023

janekmi commented Sep 26, 2023 • edited Loading

janekmi commented Sep 28, 2023

brianjmurrell commented Oct 2, 2023

janekmi commented Sep 26, 2023 •

edited

Loading