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

lvm2: 2.02.177 -> 2.03.01 #51756

Merged
merged 2 commits into from Dec 10, 2018
Merged

lvm2: 2.02.177 -> 2.03.01 #51756

merged 2 commits into from Dec 10, 2018

Conversation

markuskowa
Copy link
Member

@markuskowa markuskowa commented Dec 8, 2018

Motivation for this change

Version bump. Switched the source to the git repository. Should now be fit for the auto updater.

CC @7c6f434c

Things done
  • switch to sources to git
  • add libaio to buildInputs
  • Fix grub (fallout from xfsprogs-4.19 update).
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

@GrahamcOfBorg test installer.lvm

@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

@GrahamcOfBorg biuld nixosTests.installer.lvm

@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

And this is the first bump across what they called a set of large refactorings on the mailing list, so probably we shouldn't hurry with backporting?

@markuskowa
Copy link
Member Author

What do you mean by backporting?
@GrahamcOfBorg build nixosTests.installer.lvm

@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

I meant release-18.09

@markuskowa
Copy link
Member Author

OK, if there is any need for backporting, there is also lvm2-2.02.183.

@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

I have some doubts even about 2.02.183

@pbogdan
Copy link
Member

pbogdan commented Dec 9, 2018

Not directly pertaining to the changes in this PR but just FYI the LVM installer test is likely broken on master anyway for unrelated reasons - #51444 (comment)

@markuskowa
Copy link
Member Author

IIRC there were also other problems with the installer tests, such as randomly timed out tests.

* switch to sources to git
* add libaio to buildInputs
@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

@pbogdan Well, there was also that spice failure breaking all the tests…

@markuskowa
Copy link
Member Author

I just did a rebase. This should eliminate the issue with spice and leave only the xfsprogs/grub-os-prober problem on x86_64-linux.
@GrahamcOfBorg build nixosTests.installer.lvm

@markuskowa
Copy link
Member Author

Now the test times out on x86_64-linux.

@7c6f434c
Copy link
Member

7c6f434c commented Dec 9, 2018

Locally it failed for GRUB reasons without this PR. #51790

@markuskowa
Copy link
Member Author

@GrahamcOfBorg build nixosTests.installer.lvm

@markuskowa
Copy link
Member Author

With the XFS patch the lvm installer test succeeds again (locally, with ofborg it times out).

xfsprogs-4.16 introduced an new on disk format feature
that is not recognized properly by grub. This patch
allows grub to access XFS filesystem created with
xfsprogs >= 4.16.
This upstream patch can be removed for grub-2.03.
@hedning
Copy link
Contributor

hedning commented Dec 10, 2018

Test works locally here too 👍 any reason not to merge? at the very worse we'll break an already broke trunk :)

@markuskowa
Copy link
Member Author

If there are no further comments it can be merged.

@7c6f434c 7c6f434c merged commit 654d324 into NixOS:master Dec 10, 2018
@markuskowa markuskowa deleted the upd-lvm2 branch December 10, 2018 22:04
@hedning
Copy link
Contributor

hedning commented Dec 11, 2018

Hmm, seems like 2.03.00 removed applib, the flag is no longer recognized:

configure: WARNING: unrecognized options: --disable-static, --enable-applib

The lvm2 commit removing it.

osquery breaks due to the removal:

.../block_devices.cpp:24:10: fatal error: lvm2app.h: No such file or directory

osquery master still depends on this, so it's not enough to upgrade osquery.

cc latest osquery maintainer @Ma27

@markuskowa
Copy link
Member Author

The lvm update also breaks udisks1 and udisks-glue. The latest versions of those packages are from 2013 and 2014 and look rather outdated. Can they be removed (CC @pSub)?

@Ma27
Copy link
Member

Ma27 commented Dec 11, 2018

Even if it would be fixed on oquery master, it would take some time until I do the next osquery bump, unfortunately the diff between the patched cmake (I'm partially responsible for) and master's cmake is way too big now.

Until this is fixed upstream and I do the upgrade for nixpkgs, wouldn't it be possible to maintain two versions of the package (as this seems to be a breaking change)?

@7c6f434c
Copy link
Member

Reinstating 2.02.183 seems to be a natural plan…

@markuskowa
Copy link
Member Author

So far it looks like osquery is the only package not ready lvm2-2.03.x. Makes sense to add 2.02.183 as a legacy version. We can keep a legacy version until osquery is updated.

@Ma27
Copy link
Member

Ma27 commented Dec 12, 2018

👍 do you want to file a patch for this? (feel free to ping me then, I can test and merge this for you then :))

@Ma27 Ma27 mentioned this pull request Dec 23, 2018
10 tasks
Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Dec 24, 2018
As discussed in NixOS#51756, recently packaged versions of `lvm2` miss the
`lvm2app.h` header which breaks the osquery build.

Please note that this simply fixes the build and is not an upgrade. The
CMake patches are fairly diverged in constrast to the current upstream
packaging which requires a lot more effort I can't provide ATM.

cc @markuskowa @hedning
@ivan ivan mentioned this pull request Dec 27, 2018
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants