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

Removing quotes from HistIgnore and Histcontrol #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lisavbreugel
Copy link

As they were not working correctly with quotes.

As they were not working correctly with quotes.
@augustohp augustohp self-assigned this Oct 25, 2019
@augustohp augustohp added the bug Something isn't working label Oct 25, 2019
@lisavbreugel
Copy link
Author

The HISTIGNORE wasn’t working, so when we generate the dockerfile the commands that we were supposed to ignore to go to history, were going, so we always got at the end of the file RUN exit

@augustohp
Copy link
Owner

The HISTIGNORE wasn’t working, so when we generate the dockerfile the commands that we were supposed to ignore to go to history, were going, so we always got at the end of the file RUN exit

Hmn, thanks (a lot) for the reply! Can you send me your use case? I want to have a test scenario for this so we don't have regressions, HISTIGNORE was working with my scenarios 🤔

@airtonzanon
Copy link

Hey @augustohp I have the same problem as @lisavbreugel

When I try to generate a Dockerfile with ship

./ship ubuntu
root@4d544e54516b:/# ls
bin  boot  dev	etc  home  lib	lib64  media  mnt  opt	proc  root  run  sbin  srv  sys  tmp  usr  var
root@4d544e54516b:/# cd home
root@4d544e54516b:/home# apt update
Get:1 http://archive.ubuntu.com/ubuntu bionic InRelease [242 kB]
Get:2 http://security.ubuntu.com/ubuntu bionic-security InRelease [88.7 kB]
Get:3 http://archive.ubuntu.com/ubuntu bionic-updates InRelease [88.7 kB]
Get:4 http://archive.ubuntu.com/ubuntu bionic-backports InRelease [74.6 kB]
Get:5 http://security.ubuntu.com/ubuntu bionic-security/main amd64 Packages [700 kB]
Get:6 http://security.ubuntu.com/ubuntu bionic-security/universe amd64 Packages [782 kB]
Get:7 http://archive.ubuntu.com/ubuntu bionic/restricted amd64 Packages [13.5 kB]
Get:8 http://archive.ubuntu.com/ubuntu bionic/universe amd64 Packages [11.3 MB]
Get:9 http://security.ubuntu.com/ubuntu bionic-security/restricted amd64 Packages [12.6 kB]
Get:10 http://security.ubuntu.com/ubuntu bionic-security/multiverse amd64 Packages [5944 B]
Get:11 http://archive.ubuntu.com/ubuntu bionic/multiverse amd64 Packages [186 kB]
Get:12 http://archive.ubuntu.com/ubuntu bionic/main amd64 Packages [1344 kB]
Get:13 http://archive.ubuntu.com/ubuntu bionic-updates/main amd64 Packages [995 kB]
Get:14 http://archive.ubuntu.com/ubuntu bionic-updates/universe amd64 Packages [1303 kB]
Get:15 http://archive.ubuntu.com/ubuntu bionic-updates/multiverse amd64 Packages [9022 B]
Get:16 http://archive.ubuntu.com/ubuntu bionic-updates/restricted amd64 Packages [23.2 kB]
Get:17 http://archive.ubuntu.com/ubuntu bionic-backports/universe amd64 Packages [4234 B]
Get:18 http://archive.ubuntu.com/ubuntu bionic-backports/main amd64 Packages [2496 B]
Fetched 17.2 MB in 3s (6697 kB/s)
Reading package lists... Done
Building dependency tree
Reading state information... Done
1 package can be upgraded. Run 'apt list --upgradable' to see it.
root@4d544e54516b:/home# exit
exit

Dockerfile writeen successfully!

The Dockerfile generated will be

# Generated by https://github.com/augustohp/ship
FROM ubuntu
LABEL maintainer=" <me@airton.dev>"

RUN ls
RUN cd home
RUN apt update
RUN exit

The result of echo $HISTIGNORE inside the container is

root@f33081a6b7a6:/# echo $HISTIGNORE
"&:[ ]*:exit:ls:bg:fg:history:clear"

And if I run history inside the container:

root@f33081a6b7a6:/# history
    1  echo $HISTIGNORE
    2  history

@airtonzanon
Copy link

@augustohp Do you think that this PR still valid?

@augustohp
Copy link
Owner

@airtonzanon I think it is, but I don't actually know what to do about it. I didn't understand what code you've tested (master or this one), but if you tested this branch and it is working with you, I am inclined to merge it. Please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants