Skip to content

fix using agama.install_url with equal in param#1803

Merged
jreidinger merged 5 commits intomasterfrom
fix_install_url
Dec 3, 2024
Merged

fix using agama.install_url with equal in param#1803
jreidinger merged 5 commits intomasterfrom
fix_install_url

Conversation

@jreidinger
Copy link
Contributor

@jreidinger jreidinger commented Dec 3, 2024

Problem

We get report that using agama with agama.install_url=cd:/?devices=/dev/sr1 to read repository from second dvd does not work.

Solution

Fix parsing cmdline arguments which do too much splitting for each argument.

Testing

  • Added a new unit test
  • Tested manually without VPN and also completely without network

@jreidinger jreidinger marked this pull request as ready for review December 3, 2024 20:33
Comment on lines +40 to +48
it "converts 'true' and 'false' values into booleans" do
args = described_class.read_from(File.join(workdir, "/proc/cmdline"))
expect(args.data["web"]).to eql({ "ssl" => true })
end

it "converts keys that contain another dots after 'agama.' to hash value and using first value as key" do
args = described_class.read_from(File.join(workdir, "/proc/cmdline"))
expect(args.data["web"]).to eq({ "ssl" => true })
end
Copy link
Contributor

Choose a reason for hiding this comment

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

These examples has identical expectations. Is it ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, as both test uses original fixture, so I would like to here emphasize that it do "hashisize" of key and also normalize of value. My plan was to use separate examples with documented ones from https://agama-project.github.io/docs/user/boot_options sadly noone of those options are documented, so I just keep it for testing regressions.

it "properly parse values that contain '='" do
args = described_class.read_from(File.join(workdir, "/proc/cmdline"))
expect(args.data["install_url"]).to eq("cd:/?devices=/dev/sr1")
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a missing example for testing the "auto" value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, for auto it is plain option and it is in top level one at https://github.com/agama-project/agama/pull/1803/files#diff-68c3c88c1cfcfdc461f9d92425b028aa4ed3ca2a8c6239f2325b01fc4b0c8567R32
here it is intentionally using real cd schema that user complain on slack

@coveralls
Copy link

coveralls commented Dec 3, 2024

Pull Request Test Coverage Report for Build 12147701143

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 71.077%

Totals Coverage Status
Change from base Build 12144610593: 0.008%
Covered Lines: 17028
Relevant Lines: 23957

💛 - Coveralls

@jreidinger jreidinger merged commit 6f87318 into master Dec 3, 2024
@jreidinger jreidinger deleted the fix_install_url branch December 3, 2024 20:52
@imobachgs imobachgs mentioned this pull request Jan 10, 2025
imobachgs added a commit that referenced this pull request Jan 13, 2025
Update to release version 11.

* #1495
* #1564
* #1617
* #1618
* #1625
* #1626
* #1627
* #1628
* #1630
* #1631
* #1632
* #1633
* #1634
* #1635
* #1636
* #1639
* #1640
* #1641
* #1642
* #1643
* #1644
* #1645
* #1646
* #1647
* #1648
* #1649
* #1650
* #1651
* #1652
* #1654
* #1655
* #1656
* #1657
* #1660
* #1663
* #1666
* #1667
* #1668
* #1670
* #1671
* #1673
* #1674
* #1675
* #1676
* #1677
* #1681
* #1682
* #1683
* #1684
* #1687
* #1688
* #1689
* #1690
* #1691
* #1692
* #1693
* #1694
* #1695
* #1696
* #1698
* #1699
* #1702
* #1703
* #1704
* #1705
* #1707
* #1708
* #1709
* #1710
* #1711
* #1712
* #1713
* #1714
* #1715
* #1716
* #1717
* #1718
* #1720
* #1721
* #1722
* #1723
* #1727
* #1728
* #1729
* #1731
* #1732
* #1733
* #1734
* #1735
* #1736
* #1737
* #1740
* #1741
* #1743
* #1744
* #1745
* #1746
* #1751
* #1753
* #1754
* #1755
* #1757
* #1762
* #1763
* #1764
* #1765
* #1766
* #1767
* #1769
* #1771
* #1772
* #1773
* #1774
* #1777
* #1778
* #1785
* #1786
* #1787
* #1788
* #1789
* #1790
* #1791
* #1792
* #1793
* #1794
* #1795
* #1796
* #1797
* #1798
* #1799
* #1800
* #1802
* #1803
* #1804
* #1805
* #1807
* #1808
* #1809
* #1810
* #1811
* #1812
* #1814
* #1815
* #1821
* #1822
* #1823
* #1824
* #1825
* #1826
* #1827
* #1828
* #1830
* #1831
* #1832
* #1833
* #1834
* #1835
* #1836
* #1837
* #1838
* #1839
* #1840
* #1841
* #1842
* #1843
* #1844
* #1845
* #1847
* #1848
* #1849
* #1850
* #1851
* #1854
* #1855
* #1856
* #1857
* #1860
* #1861
* #1863
* #1864
* #1865
* #1866
* #1867
* #1871
* #1872
* #1873
* #1875
* #1876
* #1877
* #1878
* #1880
* #1881
* #1882
* #1883
* #1884
* #1885
* #1886
* #1888
* #1889
* #1890
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.

3 participants