Dear Tumbleweed users and hackers,

Another week, in which openQA did block some of the snapshots – and some issues it was unfortunately not able to see. Anyway, during the week 2019/47 we have released three snapshot into the wild (1116, 1118 and 1119), containing those changes:

• Mesa 19.2.4: fixes critical rendering issues from earlier Mesa 19.2.3. As this rendering issue did not happen on all graphics adapters, openQA had no chance of spotting it
• Linux kernel 5.3.11
• KDE Plasma 5.17.3
• Subversion 1.13.0
• binutils 2.33.1

The things being worked on include:

• openssl 1.1.1d: will be part of snapshot 1121 and later)
• rpm 4.15.0: the new python dependency generator is nasty, as it adds much more dependencies that were the before (probably a good thing), exposing new build cycles
• Python 3.8
• Rust 1.39.0: break Firefox and Thunderbird so far
• Perl 5.30.1
• libxml 2.9.10: breaks perl-XML-LibXSLT (on i586 only, invalid free() )

The Contents

• The System Role selection dialog got usability improvements
  • and we added a CustomStatusItemSelector to the widget library in the process.
• Snapper gained machine-readable output
• Storage:
  • remote encrypted devices are activated properly at boot time
  • random and pervasive encryption also supported in AutoYaST
  • improved support for AutoYaST Guided Partitioning

A More User Friendly Role Selector Dialog

Step by step, we continue improving the user experience making use of the newly added widgets to libyui. This sprint was the turn to update the role selection dialog to use the new item selector introduced during the sprint 87. Apart from looking better as it can be seen in the screenshots below, there are two immediate improvements:

• the vertical scroll, when present, is respected after selecting a role (instead of “jumping to the beginning”), and
• the selected role (if any) will be visible when arriving at the dialog even when the list is quite long or the available space too short.

What is more, updating the dialog was also useful for us to realize about some needed improvements for the widget itself, mentioned in the next section. Quite a productive change :smiley:

When one Bit is not Enough: The CustomStatusItemSelector

A few weeks ago, we wrote about the new ItemSelector widget that is finding its way into YaST user interfaces. It turned out that just a simple on/off status is not enough in some cases, so we had to extend that concept. For example, software modules may have dependencies, and we want to show the difference between one that was explicitly selected by the user and one that was auto-selected because some other software module requires it.

This kind of shook the foundations of the underlying classes; all of a sudden a bit is no longer just a bit, but it needs to be broken down into even smaller pieces. Well, we cheated; we now use integer values instead. Most of the class hierarchy still only uses 0 and 1, but the new YCustomStatusItemSelector also supports using higher numbers for application-defined purposes.

For each possible status value, the application defines the name of the icon to be displayed (for graphical UIs like the Qt UI), the text equivalent (for text mode / the NCurses UI), and an optional nextStatus which tells the widget what status to cycle to when the user changes the status of an item with a mouse click or with the keyboard. A value of -1 lets the application handle this.

So this is not a one-trick-pony that is useful only for that one use case (the software modules), but a generic tool that might find good uses in other places all over YaST as well.

Usage examples: C++, Ruby.

Snapper and Machine-readable Output

Most likely you already know snapper, a great tool to work with your filesystem snapshots. Some third-party scripts and tools (e.g., YaST) use the snapper CLI to get some information, but generally, snapper generates an output intended to be human-readable. Sometimes that could cause some troubles in scripts checking the snapper output. Now on, snapper also offers CLI options to generate its output in a machine-readable format, i.e., CSV and JSON. Please, check this post for more information about those new options.

Fix Boot Problems with Remote Encrypted Devices

Since we adopted systemd, the management during system boot of encrypted devices on top of network-based devices like iSCSI or FCoE disks has been less than optimal. But now we are happy to announce that we have put all the pieces together to make the experience as smooth as possible.

One of the main responsibilities of systemd is sorting the actions performed during boot and setting the dependencies between them. For example, if there are encrypted devices, systemd will first ask you for the password and activate the devices to afterwards mount the file system contained in those encrypted devices. Systemd should be able to distinguish when an encrypted device is based on a network-based storage device and, thus, can only be initialized after the network is up. In some cases that detection failed (for example network block device based mounts, such as iSCSI and FCoE disks) and systemd got stuck before initializing the network waiting for the device to be available.

Recently, SLE and openSUSE Leap has incorporated support for specifying the _netdev option in the /etc/crypttab file. With such option, systemd will recognize the encrypted device as network-based and will only try to activate it after setting up the network. That’s analogous to the corresponding _netdev option in /etc/fstab that has been already there for quite some time and that can be used to defer when a device is mounted. For it to work properly, the _netdev option must be present in all the relevant entries of both crypttab and fstab.

And that’s exactly what YaST will do now in openSUSE Tumbleweed and upcoming releases of both SLE and openSUSE Leap. From now on, the _netdev option will be added fstab for all mount points depending (directly or indirectly) on the network. In addition, that option (and also the noauto and nofail ones) will be propagated from fstab to all the corresponding crypttab entries.

This should mark the end of a dark age of encrypted iSCSI and FCoE devices timing out during boot.

AutoYaST Support for Random and Pervasive Encryption

Back in October, we announced that YaST got support for new encryption methods like random or pervasive encryption. At that time, AutoYaST was out of scope because we wanted to have a stable (and tested) API first. Fortunately, the time has come and now AutoYaST supports these encryption mechanisms.

Starting in autoyast2 4.2.17, you can specify the encryption method using a crypt_method element, as shown in the example below. Possible values are luks1 (regular LUKS1 encryption), pervasive_luks2 (pervasive volume encryption), protected_swap (encryption with volatile protected key), secure_swap (encryption with volatile secure key) and random_swap (encryption with volatile random key).

<drive>
 <type config:type="symbol">CT_DISK</type>
 <use>all</use>
 <partitions config:type="list">
  <partition>
   <size>20G</size>
   <mount>/</mount>
   <filesystem config:type="symbol">ext4</filesystem>
   <crypt_method config:type="symbol">luks1</crypt_method> <!-- default method if crypt_key is defined -->
   <crypt_key>S3CR3T</crypt_key>
  </partition>
  <partition>
   <size>1G</size>
   <mount>swap</mount>
   <crypt_method config:type="symbol">random_swap</crypt_method> <!-- set encryption method -->
  </partition>
 </partitions>
</drive>

As we want AutoYaST to be as user-friendly as possible, it will try to help you if you do some mistake setting the encryption configuration as when in the screenshot below.

Finally, the old crypt_fs element is deprecated, although it stills works for backward-compatibility reasons. Basically, it is equivalent to setting crypt_method to luks1.

Improve Support for AutoYaST Guided Partitioning

When it comes to partitioning, we can categorize AutoYaST use cases into three different levels:

• Automatic partitioning: the user does not care about the partitioning and trusts in AutoYaST to do the right thing.
• Guided partitioning: the user would like to set some basic settings (use LVM, set an encryption password, etc.)
• Expert partitioning: the user specifies how the layout should look, although a complete definition is not required.

To some extent, it is like using the regular installer where you can skip the partitioning screen and trust in YaST, use the Guided Proposal, or define the partitioning layout through the Expert Partitioner.

The second level (Guided partitioning) was introduced in AutoYaST with the release of SUSE Linux Enteprise 15 (and Leap 15.0) but it was not documented at all. Additionally, although it was working as designed at first sight, it was far from being that useful.

This sprint with invested quite some time improving the documentation (kudos to our awesome documentation team) and the behaviour. Now, if you want to set up an LVM system without having the specify all the details, you can use the following snippet in your profile:

<general>
  <storage>
    <lvm config:type="boolean">true</lvm>
  </storage>
</general>

If you are interested in the available options, you can check the documentation draft.

ΒΗΜΑ ΠΡΟΣ ΒΗΜΑ

<?xml version="1.0"?>
<snapshot>
  <type>single</type>
  <num>1</num>
  <date>$DATE</date>
  <description>first root filesystem</description>
</snapshot>

When starting a WSL distribution for the first time, a text prompt for user name and password appears:

The code for that is partially in the Windows launcher. The Windows side actually prompts for the user name:
https://github.com/microsoft/WSL-DistroLauncher/blob/master/DistroLauncher/DistroLauncher.cpp#L44

and passes it to ‘adduser’:
https://github.com/microsoft/WSL-DistroLauncher/blob/1f8551f7e2ea22bba2e6fb02f01e7a5f7fb757f3/DistroLauncher/DistributionInfo.cpp#L14

That seems to be a Debian specific tool that also prompts for a password. We don’t have it in openSUSE. When done, the Windows part actually calls into the Linux environment again with ‘id -u’ to get the uid of the added user:
https://github.com/microsoft/WSL-DistroLauncher/blob/1f8551f7e2ea22bba2e6fb02f01e7a5f7fb757f3/DistroLauncher/DistributionInfo.cpp#L44

So in order to also prompt for the password we’d have to write a wrapper like the Debian one or implement another prompt in the launcher. Implementing such a prompt in Windows code seems boring to me. When writing a wrapper, I’d do something dialog based to make it look more fancy. There’s already jeos-firstboot that does something similar already and more. But then the WSL image doesn’t have to be really minimal, which means we have YaST!

So even though WSL doesn’t really boot as it has no systemd it would be still possible to run the YaST firstboot wizard on first start. What modules it launches is configurable via xml file. So leaving out hardware/VM specific things like network configuration it works pretty well:

For the launcher to know the name of the created user a small YaST module was needed to write the name into /run/wsl_firstboot_uid. The launcher fetches it from there.

Using the YaST firstboot wizard also allows to use e.g. the existing registration dialogs on SLE or add other useful configuration steps. One feature I have in mind would be for example is the role selection screen to offer some pre defined package selections for WSL use cases.

Tumbleweed and Leap appx files to test this are available from download.opensuse.org. Keep in mind that one needs to import the certificates used by OBS for signing first.

Ce tutoriel est issu d'un post sur le forum par squid-f qui nous partage ici sa procédure d'installation. Merci à lui !

En fait, il y a beaucoup de tutos pour ubuntu mais très peu pour openSUSE, d'où ce tutoriel pour openSUSE Leap 15.1.

Avant de commencer, voici les sources qui m’ont bien aidé :

• https://www.c-rieger.de/nextcloud-installation-guide-ubuntu-18-04
• https://decatec.de/home-server/nextcloud-auf-ubuntu-server-18-04-lts-mit-nginx-mariadb-php-lets-encrypt-redis-und-fail2ban
• https://www.howtoforge.com/tutorial/owncloud-nginx-opensuse-leap-42.1
• https://nextcloud.com/support

Installation de NGINX et PHP

J’ai choisi d’installer une autre version de php-fpm que celle d’origine de Leap 15.1 car le module password de Nextcloud ne va bientôt plus supporter php-fpm7.2.

La version 7.3 sera installée grâce à (php7-apcu et imagick sont dans le dépôt extensions) :

zypper ar -f http://download.opensuse.org/repositories/devel:/languages:/php/openSUSE_Leap_15.1/ php-devel

zypper addrepo https://download.opensuse.org/repositories/server:php:extensions:php7/php7_openSUSE_Leap_15.1/server:php:extensions:php7.repo

zypper refresh

Installation de nginx

zypper in nginx

Installation de PHP

En disséquant la documentation de Nextcloud, j’ai abouti à la liste et l’installation des modules suivants :

zypper in php7 php7-APCu php7-bz2 php7-ctype php7-curl php7-dom php7-exif php7-fileinfo php7-fpm php7-gd php7-gmp php7-iconv php7-imagick php7-intl php7-json php7-ldap php7-mbstring php7-mysql php7-opcache php7-openssl php7-pcntl php7-pdo php7-posix php7-tokenizer php7-xmlreader php7-xmlwriter php7-zip php7-zlib ffmpeg-3

Tests initiaux NGINX et PHP-FPM

Test de NGINX

• Créer /srv/www/htdocs/index.html avec sudo nano /srv/www/htdocs/index.html et coller le code suivant :

<html>

<body>

<h1> NGINX fonctionne </h1>

</body>

</html>

• Editer /etc/nginx/nginx.conf avec sudo nano /etc/nginx/nginx.conf

Ajouter index.php dans la ligne ci-dessous vers le début du fichier :

index index.html index.htm index.php;

enlever les symboles " # " de commentaire dans le bloc ci-dessous :

location ~ \.php$ {

root /srv/www/htdocs/;

fastcgi_pass 127.0.0.1:9000;

fastcgi_index index.php;

fastcgi_param SCRIPT_FILENAME /scripts$fastcgi_script_name;

include fastcgi_params;

}

• Tester la syntaxe des modifications avec la commande ci-dessous qui ne doit pas renvoyer d’erreur :

sudo nginx -t

• Démarrer le service

sudo systemctl enable nginx

sudo systemctl start nginx

• Dans votre navigateur internet, taper l’adresse localhost et le message « NGINX fonctionne » doit apparaître

Premier paramétrage et test de PHP-FPM

• Créer /srv/www/htdocs/info.php avec ‘sudo nano /srv/www/htdocs/info.php’ et coller le code suivant :

<?php

phpinfo();

?>

• Initialiser les fichiers de configuration

sudo cp /etc/php7/fpm/php-fpm.conf.default /etc/php7/fpm/php-fpm.conf

sudo cp /etc/php7/fpm/php-fpm/php-fpm.d/www.conf.default /etc/php7/fpm/php-fpm/php-fpm.d/www.conf

• Editer php-fpm.conf ‘sudo nano /etc/php7/fpm/php-fpm.conf’ et décommenter en enlevant le symbole # de la ligne ci-dessous :

error_log = log/php-fpm.log

• Editer www.conf sudo nano /etc/php7/fpm/php-fpm.d/www.conf et modifier comme ci-dessous, en changeant user et groupet en enlevant le symbole de commentaire qui est cette fois ; pour d’autres lignes :

user = nginx
group = nginx
listen.owner = nginx
listen.group = nginx
listen.mode = 0660

Pour améliorer la performance, continuer à modifier www.conf de façon à écouter un socket et non un port. Pour cela, commenter avec ; la ligne listen = 127.0.0.1:9000 et ajouter une ligne pour pointer vers un socket :

;listen = 127.0.0.1:9000
listen = /var/run/php-fpm.sock

Pour être cohérent en PHP-FPM et NGINX quant à l’écoute d’un socket, il faut re-éditer /etc/nginx/nginx.conf et modifier comme suit le bloc location ~ \.php$ en ajoutant la ligne fastcgi_pass pointant le même socket que dans www.conf :

location ~ \.php$ {
    root /srv/www/htdocs/;
    #fastcgi_pass 127.0.0.1:9000;
    fastcgi_pass unix:/var/run/php-fpm.sock;
    fastcgi_index index.php;
    fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
    include fastcgi_params;
}

• Initialiser...

CSS length values have a number and a unit, e.g. 5cm or 6px. Sometimes the unit is a percentage, like 50%, and SVG says that lengths with percentage units should be resolved with respect to a certain rectangle. For example, consider this circle element:

<circle cx="50%" cy="75%" r="4px" fill="black"/>

This means, draw a solid black circle whose center is at 50% of the width and 75% of the height of the current viewport. The circle should have a 4-pixel radius.

The process of converting that kind of units into absolute pixels for the final drawing is called normalization. In SVG, percentage units sometimes need to be normalized with respect to the current viewport (a local coordinate system), or with respect to the size of another object (e.g. when a clipping path is used to cut the current shape in half).

One detail about normalization is that it can be with respect to the horizontal dimension of the current viewport, the vertical dimension, or both. Keep this in mind: at normalization time, we need to be able to distinguish between those three modes.

The original C version

I have talked about the original C code for lengths before; the following is a small summary.

The original C code had this struct to represent lengths:

typedef struct {
    double length;
    char factor;
} RsvgLength;

The parsing code would set the factor field to a character depending on the length's unit: 'p' for percentages, 'i' for inches, etc., and '\0' for the default unit, which is pixels.

Along with that, the normalization code needed to know the direction (horizontal, vertical, both) to which the length in question refers. It did this by taking another character as an argument to the normalization function:

double
_rsvg_css_normalize_length (const RsvgLength * in, RsvgDrawingCtx * ctx, char dir)
{
    if (in->factor == '\0')            /* pixels, no need to normalize */
        return in->length;
    else if (in->factor == 'p') {      /* percentages; need to consider direction */
        if (dir == 'h')                                     /* horizontal */
            return in->length * ctx->vb.rect.width;
        if (dir == 'v')                                     /* vertical */
            return in->length * ctx->vb.rect.height;
        if (dir == 'o')                                     /* both */
            return in->length * rsvg_viewport_percentage (ctx->vb.rect.width,
                                                          ctx->vb.rect.height);
    } else { ... }
}

The original post talks about how I found a couple of bugs with how the directions are identified at normalization time. The function above expects one of 'h'/'v'/'o' for horizontal/vertical/both, and one or two places in the code passed the wrong character.

Making the C version cleaner

Before converting that code to Rust, I removed the pesky characters and made the code use proper enums to identify a length's units.

+typedef enum {
+    LENGTH_UNIT_DEFAULT,
+    LENGTH_UNIT_PERCENT,
+    LENGTH_UNIT_FONT_EM,
+    LENGTH_UNIT_FONT_EX,
+    LENGTH_UNIT_INCH,
+    LENGTH_UNIT_RELATIVE_LARGER,
+    LENGTH_UNIT_RELATIVE_SMALLER
+} LengthUnit;
+
 typedef struct {
     double length;
-    char factor;
+    LengthUnit unit;
 } RsvgLength;

Then, do the same for the normalization function, so it will get the direction in which to normalize as an enum instead of a char.

+typedef enum {
+    LENGTH_DIR_HORIZONTAL,
+    LENGTH_DIR_VERTICAL,
+    LENGTH_DIR_BOTH
+} LengthDir;

 double
-_rsvg_css_normalize_length (const RsvgLength * in, RsvgDrawingCtx * ctx, char dir)
+_rsvg_css_normalize_length (const RsvgLength * in, RsvgDrawingCtx * ctx, LengthDir dir)

Making the C version easier to get right

While doing the last change above, I found a place in the code that used the wrong direction by mistake, probably due to a cut&paste error. Part of the problem here is that the code was specifying the direction at normalization time.

I decided to change it so that each direction value carried its own direction since initialization, so that subsequent code wouldn't have to worry about that. Hopefully, initializing a width field should make it obvious that it needed LENGTH_DIR_HORIZONTAL.

 typedef struct {
     double length;
     LengthUnit unit;
+    LengthDir dir;
 } RsvgLength;

That is, so that instead of

  /* at initialization time */
  foo.width = _rsvg_css_parse_length (str);

  ...

  /* at rendering time */
  double final_width = _rsvg_css_normalize_length (&foo.width, ctx, LENGTH_DIR_HORIZONTAL);

we would instead do this:

  /* at initialization time */
  foo.width = _rsvg_css_parse_length (str, LENGTH_DIR_HORIZONTAL);

  ...

  /* at rendering time */
  double final_width = _rsvg_css_normalize_length (&foo.width, ctx);

This made the drawing code, which deals with a lot of coordinates at the same time, a lot less noisy.

Initial port to Rust

To recap, this was the state of the structs after the initial refactoring in C:

typedef enum {
    LENGTH_UNIT_DEFAULT,
    LENGTH_UNIT_PERCENT,
    LENGTH_UNIT_FONT_EM,
    LENGTH_UNIT_FONT_EX,
    LENGTH_UNIT_INCH,
    LENGTH_UNIT_RELATIVE_LARGER,
    LENGTH_UNIT_RELATIVE_SMALLER
} LengthUnit;

typedef enum {
    LENGTH_DIR_HORIZONTAL,
    LENGTH_DIR_VERTICAL,
    LENGTH_DIR_BOTH
} LengthDir;

typedef struct {
    double length;
    LengthUnit unit;
    LengthDir dir;
} RsvgLength;

This ported to Rust in a straightforward fashion:

pub enum LengthUnit {
    Default,
    Percent,
    FontEm,
    FontEx,
    Inch,
    RelativeLarger,
    RelativeSmaller
}

pub enum LengthDir {
    Horizontal,
    Vertical,
    Both
}

pub struct RsvgLength {
    length: f64,
    unit: LengthUnit,
    dir: LengthDir
}

It got a similar constructor that took the direction and produced an RsvgLength:

impl RsvgLength {
    pub fn parse (string: &str, dir: LengthDir) -> RsvgLength { ... }
}

(This was before using Result; remember that the original C code did very little error checking!)

The initial Parse trait

It was at that point that it seemed convenient to introduce a Parse trait, which all CSS value types would implement to parse themselves from string.

However, parsing an RsvgLength also needed an extra piece of data, the LengthDir. My initial version of the Parse trait had an associated called Data, through which one could pass an extra piece of data during parsing/initialization:

pub trait Parse: Sized {
    type Data;
    type Err;

    fn parse (s: &str, data: Self::Data) -> Result<Self, Self::Err>;
}

This was explicitly to be able to pass a LengthDir to the parser for RsvgLength:

impl Parse for RsvgLength {
    type Data = LengthDir;
    type Err = AttributeError;

    fn parse (string: &str, dir: LengthDir) -> Result <RsvgLength, AttributeError> { ... }
}

This was okay for lengths, but very noisy for everything else that didn't require an extra bit of data. In the rest of the code, the helper type was Data = () and there was a pair of extra parentheses () in every place that parse() was called.

Removing the helper Data type

Introducing one type per direction

Over a year later, that () bit of data everywhere was driving me nuts. I started refactoring the Length module to remove it.

First, I introduced three newtypes to wrap Length, and indicate their direction at the same time:

pub struct LengthHorizontal(Length);
pub struct LengthVertical(Length);
pub struct LengthBoth(Length);

This was done with a macro because now each wrapper type needed to know the relevant LengthDir.

Now, for example, the declaration for the <circle> element looked like this:

pub struct NodeCircle {
    cx: Cell<LengthHorizontal>,
    cy: Cell<LengthVertical>,
    r: Cell<LengthBoth>,
}

(Ignore the Cell everywhere; we got rid of that later.)

Removing the dir field

Since now the information about the length's direction is embodied in the LengthHorizontal/LengthVertical/LengthBoth types, this made it possible to remove the dir field from the inner Length struct.

 pub struct RsvgLength {
     length: f64,
     unit: LengthUnit,
-    dir: LengthDir
 }

+pub struct LengthHorizontal(Length);
+pub struct LengthVertical(Length);
+pub struct LengthBoth(Length);
+
+define_length_type!(LengthHorizontal, LengthDir::Horizontal);
+define_length_type!(LengthVertical, LengthDir::Vertical);
+define_length_type!(LengthBoth, LengthDir::Both);

Note the use of a define_length_type! macro to generate code for those three newtypes.

Removing the Data associated type

And finally, this made it possible to remove the Data associated type from the Parse trait.

 pub trait Parse: Sized {
-    type Data;
     type Err;

-    fn parse(parser: &mut Parser<'_, '_>, data: Self::Data) -> Result<Self, Self::Err>;
+    fn parse(parser: &mut Parser<'_, '_>) -> Result<Self, Self::Err>;
 }

The resulting mega-commit removed a bunch of stray parentheses () from all calls to parse(), and the code ended up a lot easier to read.

Removing the newtypes

This was fine for a while. Recently, however, I figured out that it would be possible to embody the information for a length's direction in a different way.

But to get there, I first needed a temporary refactor.

Replacing the macro with a trait with a default implementation

Deep in the guts of length.rs, the key function that does something different based on LengthDir is its scaling_factor method:

enum LengthDir {
    Horizontal,
    Vertical,
    Both,
}

impl LengthDir {
    fn scaling_factor(self, x: f64, y: f64) -> f64 {
        match self {
            LengthDir::Horizontal => x,
            LengthDir::Vertical => y,
            LengthDir::Both => viewport_percentage(x, y),
        }
    }
}

That method gets passed, for example, the width/height of the current viewport for the x/y arguments. The method decides whether to use the width, height, or a combination of both.

And of course, the interesting part of the define_length_type! macro was to generate code for calling LengthDir::Horizontal::scaling_factor()/etc. as appropriate depending on the LengthDir in question.

First I made a trait called Orientation with a scaling_factor method, and three zero-sized types that implement that trait. Note how each of these three implementations corresponds to one of the match arms above:

pub trait Orientation {
    fn scaling_factor(x: f64, y: f64) -> f64;
}

pub struct Horizontal;
pub struct Vertical;
pub struct Both;

impl Orientation for Horizontal {
    fn scaling_factor(x: f64, _y: f64) -> f64 {
        x
    }
}

impl Orientation for Vertical {
    fn scaling_factor(_x: f64, y: f64) -> f64 {
        y
    }
}

impl Orientation for Both {
    fn scaling_factor(x: f64, y: f64) -> f64 {
        viewport_percentage(x, y)
    }
}

Now most of the contents of the define_length_type! macro can go in the default implementation of a new trait LengthTrait. Crucially, this trait has an Orientation associated type, which it uses to call into the Orientation trait:

pub trait LengthTrait: Sized {
    type Orientation: Orientation;

    ...

    fn normalize(&self, values: &ComputedValues, params: &ViewParams) -> f64 {
        match self.unit() {
            LengthUnit::Px => self.length(),

            LengthUnit::Percent => {
                self.length() *
                <Self::Orientation>::scaling_factor(params.view_box_width, params.view_box_height)
            }

            ...
    }
}

Note that the incantation is <Self::Orientation>::scaling_factor(...) to call that method on the associated type.

Now the define_length_type! macro is shrunk a lot, with the interesting part being just this:

macro_rules! define_length_type {
    {$name:ident, $orient:ty} => {
        pub struct $name(Length);

        impl LengthTrait for $name {
            type Orientation = $orient;
        }
    }
}

define_length_type! { LengthHorizontal, Horizontal }
define_length_type! { LengthVertical, Vertical }
define_length_type! { LengthBoth, Both }

We moved from having three newtypes of length-with-LengthDir to three newtypes with dir-as-associated-type.

Removing the newtypes and the macro

After that temporary refactoring, we had the Orientation trait and the three zero-sized types Horizontal, Vertical, Both.

I figured out that one can use PhantomData as a way to carry around the type that Length needs to normalize itself, instead of using an associated type in an extra LengthTrait. Behold!

pub struct Length<O: Orientation> {
    pub length: f64,
    pub unit: LengthUnit,
    orientation: PhantomData<O>,
}

impl<O: Orientation> Length<O> {
    pub fn normalize(&self, values: &ComputedValues, params: &ViewParams) -> f64 {
        match self.unit {
            LengthUnit::Px => self.length,

            LengthUnit::Percent => {
                self.length 
                    * <O as Orientation>::scaling_factor(params.view_box_width, params.view_box_height)
            }

            ...
        }
    }
}

Now the incantation is <O as Orientation>::scaling_factor() to call the method on the generic type; it is no longer an associated type in a trait.

With that, users of lengths look like this; here, our <circle> element from before:

pub struct Circle {
    cx: Length<Horizontal>,
    cy: Length<Vertical>,
    r: Length<Both>,
}

I'm very happy with the readability of all the code now. I used to think of PhantomData as a way to deal with wrapping pointers from C, but it turns out that it is also useful to keep a generic type around should one need it.

The final Length struct is this:

pub struct Length<O: Orientation> {
    pub length: f64,
    pub unit: LengthUnit,
    orientation: PhantomData<O>,
}

And it only takes up as much space as its length and unit fields; PhantomData is zero-sized after all.

(Later, we renamed Orientation to Normalize, but the code's structure remained the same.)

Summary

Over a couple of years, librsvg's type that represents CSS lengths went from a C representation along the lines of "all data in the world is an int", to a Rust representation that uses some interesting type trickery:

• C struct with char for units.

• C struct with a LengthUnits enum.

• C struct without an embodied direction; each place that needs to normalize needs to get the orientation right.

• C struct with a built-in direction as an extra field, done at initialization time.

• Same struct but in Rust.

• An ugly but workable Parse trait so that the direction can be set at parse/initialization time.

• Three newtypes LengthHorizontal, LengthVertical, LengthBoth with a common core. A cleaned-up Parse trait. A macro to generate those newtypes.

• Replace the LengthDir enum with an Orientation trait, and three zero-sized types Horizontal/Vertical/Both that implement the trait.

• Replace most of the macro with a helper trait LengthTrait that has an Orientation associated type.

• Replace the helper trait with a single Length<T: Orientation> type, which puts the orientation as a generic parameter. The macro disappears and there is a single implementation for everything.

Refactoring never ends!