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

Add touch as a common core utility #2410

Merged
merged 1 commit into from
Aug 28, 2021

Conversation

hbina
Copy link
Contributor

@hbina hbina commented Jun 13, 2021

Found out that touch wasn't compiled in what I assume in ordinary Ubuntu
Linux setup.

                          ./+o+-       hbina@komputa
                  yyyyy- -yyyyyy+      OS: Ubuntu 20.04 focal
               ://+//////-yyyyyyo      Kernel: x86_64 Linux 5.12.9-051209-generic
           .++ .:/++++++/-.+sss/`      Uptime: 1h 5m
         .:++o:  /++++++++/:--:/-      Packages: 2241
        o:+o+:++.`..```.-/oo+++++/     Shell: bash 5.0.17
       .:+o:+o/.          `+sssoo+/    Resolution: 1920x1080
  .++/+:+oo+o:`             /sssooo.   DE: GNOME 3.36.5
 /+++//+:`oo+o               /::--:.   WM: Mutter
 \+/+o+++`o++o               ++////.   WM Theme: Adwaita
  .++.o+++oo+:`             /dddhhh.   GTK Theme: Yaru [GTK2/3]
       .+.o+oo:.          `oddhhhh+    Icon Theme: ubuntu-mono-dark
        \+.++o+o``-````.:ohdhhhhh+     Font: Cantarell 11
         `:o+++ `ohhhhhhhhyo++os:      Disk: 142G / 475G (32%)
           .o:`.syhhhhhhh/.oo++o`      CPU: AMD Ryzen 7 4800U with Radeon Graphics @ 16x 1.8GHz
               /osyyyyyyo++ooo+++/     GPU: AMD/ATI
                   ````` +oo+++o\:     RAM: 9370MiB / 31584MiB
                          `oo++.

Signed-off-by: Hanif Bin Ariffin hanif.ariffin.4326@gmail.com

Found out that touch wasn't compiled in what I assume in ordinary Ubuntu
Linux setup.

```
                          ./+o+-       hbina@komputa
                  yyyyy- -yyyyyy+      OS: Ubuntu 20.04 focal
               ://+//////-yyyyyyo      Kernel: x86_64 Linux 5.12.9-051209-generic
           .++ .:/++++++/-.+sss/`      Uptime: 1h 5m
         .:++o:  /++++++++/:--:/-      Packages: 2241
        o:+o+:++.`..```.-/oo+++++/     Shell: bash 5.0.17
       .:+o:+o/.          `+sssoo+/    Resolution: 1920x1080
  .++/+:+oo+o:`             /sssooo.   DE: GNOME 3.36.5
 /+++//+:`oo+o               /::--:.   WM: Mutter
 \+/+o+++`o++o               ++////.   WM Theme: Adwaita
  .++.o+++oo+:`             /dddhhh.   GTK Theme: Yaru [GTK2/3]
       .+.o+oo:.          `oddhhhh+    Icon Theme: ubuntu-mono-dark
        \+.++o+o``-````.:ohdhhhhh+     Font: Cantarell 11
         `:o+++ `ohhhhhhhhyo++os:      Disk: 142G / 475G (32%)
           .o:`.syhhhhhhh/.oo++o`      CPU: AMD Ryzen 7 4800U with Radeon Graphics @ 16x 1.8GHz
               /osyyyyyyo++ooo+++/     GPU: AMD/ATI
                   ````` +oo+++o\:     RAM: 9370MiB / 31584MiB
                          `oo++.

```

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@tertsdiepraam
Copy link
Member

Thanks! I was also surprised to see touch not being in the common core. Note that it is part of the "unix" set of features though. So you could build it with cargo run/test/build --features "unix".

I think it should be in common core if possible, but it currently seems to fail on FreeBSD. If you can get it to work there, I would be happy to accept this.

Finally, it'd be great if you could remove touch from the tier 1 feature list as wel, because the common core is implied there.

Copy link
Contributor

@sylvestre sylvestre left a comment

Choose a reason for hiding this comment

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

Yeah, please remove it from the current declaration and fix freebsd :)

@rivy
Copy link
Member

rivy commented Jun 21, 2021

I believe touch is not in "common_core" because it was troublesome on some of the alternate/Tier2 platforms (specifically, 'fuschia' and 'redox'); @Arcterus, could you weigh in here?

@hbina , cargo build builds the 'common_core' as default to deliver a nice initial experience for anyone who wants to try building from source on most platforms. But, for the full unix suite of tools, cargo build --features unix should be used. More explanation is in the README.

Any suggestions to make this point more obvious in the README for new devs/users?

But, ultimately, I think using cargo build --features unix is the right answer here... moving touch to 'common_core' is a bigger task which I think would involve at least fixing FreeBSD and maybe adding back testing for some of the other Tier2 platforms? I'd love for the increased platform testing to happen, but it was failing even before we shifted to GHA. And I haven't seen any CI pattern that I can use to re-initiate testing.

I'm happy to work on it if someone has a template I can look at to work from...

@hbina
Copy link
Contributor Author

hbina commented Jun 22, 2021

is build.rs not powerful enough to detect the host operating system?

cargo build --features unix

I will keep this mind. On a side note, I need to install all 4 OSses in a VM T.t

@rivy
Copy link
Member

rivy commented Jun 22, 2021

is build.rs not powerful enough to detect the host operating system?

cargo build --features unix

cargo doesn't allow the build script to change features at runtime (see rust-lang/cargo#5499 (comment)).

@sylvestre
Copy link
Contributor

freebsd is still broken

@sylvestre sylvestre merged commit b0becf0 into uutils:master Aug 28, 2021
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.

4 participants